Skip to content

Conversation

@willrowe
Copy link
Contributor

This PR updates the typehints for the make method on the 'App' facade to match those on the make method in the Illuminate\Foundation\Application class that it calls. The app helper function already has this, which is great, but sometimes I'm using the facade and want the assigned variable to resolve to the correct type in my editor.

@willrowe willrowe changed the base branch from 12.x to master September 11, 2025 17:11
@willrowe willrowe marked this pull request as draft September 11, 2025 17:13
@willrowe willrowe marked this pull request as ready for review September 11, 2025 17:14
@willrowe willrowe closed this Sep 11, 2025
@willrowe willrowe reopened this Sep 11, 2025
@cosmastech
Copy link
Contributor

I believe this will be overwritten by https://github.com/laravel/facade-documenter which runs on merge

@willrowe
Copy link
Contributor Author

@cosmastech good catch, checking it now.

@willrowe
Copy link
Contributor Author

I just ran a test with https://github.com/laravel/facade-documenter/blob/main/facade.php locally and it is pulling out the wrong typehints for some reason. Looking into it further now.

@willrowe
Copy link
Contributor Author

So it appears to be an issue in the facade documenter itself. Any idea why it's rewriting the docblocks instead of just copying them?

@cosmastech
Copy link
Contributor

So it appears to be an issue in the facade documenter itself. Any idea why it's rewriting the docblocks instead of just copying them?

I am not sure if this is was an oversight or intentional. Maybe @timacdonald can shed some light.

It's been something that has been on my "open-source todos" list for a while, but never attempted.

@willrowe
Copy link
Contributor Author

It's been something that has been on my "open-source todos" list for a while, but never attempted.

What specifically? What this PR is trying to do or something else?

@cosmastech
Copy link
Contributor

It's been something that has been on my "open-source todos" list for a while, but never attempted.

What specifically? What this PR is trying to do or something else?

Modifying the facade documenter to keep the typehints that have been written and so painstakingly reviewed by Taylor 😆

@willrowe
Copy link
Contributor Author

@cosmastech understood, I agree that it is expected that they would be preserved. I'm glad to submit a PR if @timacdonald can provide some context as to why it is working this way currently.

@willrowe willrowe closed this Sep 11, 2025
@willrowe
Copy link
Contributor Author

I think it has something to do with how syntax supported in @return may not necessarily be valid in @method.

@willrowe willrowe deleted the feature/match-make-typehints branch September 11, 2025 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants