Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.x] Implement IDE Helper #5562

Closed
wants to merge 1 commit into from
Closed

[8.x] Implement IDE Helper #5562

wants to merge 1 commit into from

Conversation

driesvints
Copy link
Member

@driesvints driesvints commented Mar 12, 2021

This implements Laravel IDE Helper Generator which can replace the existing DocBocks in the framework.

Goes along with laravel/framework#36585

@driesvints
Copy link
Member Author

Ping @barryvdh

@barryvdh
Copy link
Contributor

Well obviously I like the idea of adding it by default. Not 100% about just removing the phpdocs though.

@deleugpn
Copy link

I don't like how it makes PHPStorm identify two classes for everything

@SjorsO
Copy link
Contributor

SjorsO commented Mar 12, 2021

The composer script seems to be missing @php artisan ide-helper:models -N, this one is also very helpful.

Also, maybe you can add a composer script to easily regenerate the helpers. The helpers need to be regenerated after making database changes or when adding a model accessor. I use the following in my projects:

"post-update-cmd": [
    "Illuminate\\Foundation\\ComposerScripts::postUpdate",
    "@composer ide"
],
"ide": [
    "@php artisan ide-helper:generate",
    "@php artisan ide-helper:meta",
    "@php artisan ide-helper:models -N"
]

@mfn
Copy link
Contributor

mfn commented Mar 12, 2021

The helpers need to be regenerated after making database changes or when adding model accessor. I use the following in my projects:

Btw, there's barryvdh/laravel-ide-helper#1163 to achieve this

],
"post-update-cmd": [
"Illuminate\\Foundation\\ComposerScripts::postUpdate",
"@php artisan ide-helper:generate",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will fail no-dev environments, I suggest using:

            "[ $COMPOSER_DEV_MODE -eq 0 ] || $PHP_BINARY artisan ide-helper:generate",
            "[ $COMPOSER_DEV_MODE -eq 0 ] || $PHP_BINARY artisan ide-helper:meta"

image

I also suggest adding it to the post-install-cmd too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When do you run composer update in a no-dev environment?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, that's a good point, I have it in my post-install-cmd too and that's what broke for me.

Having it in post install are pretty much a must have if you are planning to have collaborators on the project.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@barryvdh can you confirm this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically composer fails because it tries to execute a nonexistent command, there are two ways to prevent this:

  1. appending || true which will make the command look like it was successful but still showing the error message
  2. using the command I sent which checks and only runs the code in dev mode, basically a bash test

The 3rd solution would be to implement it inside the framework, I thought about it some time ago:

  • Add a PackagesDiscovered event to the package:discover command (this could be useful for other packages too)
  • And add a listener for it in ide helper like in my PR for the migrations
    Thus allowing automatically generating helpers when packages are discovered

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, turns none of my first two solution works under Windows

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I usually add the file to my git, so it is included when sharing the project. Only needs to refresh on updates in that case.
You don't want to run it on production.

@taylorotwell
Copy link
Member

I guess we'll just table this for now since the other PR is closed as well.

@taylorotwell taylorotwell deleted the ide-helper branch March 17, 2021 13:16
@barryvdh
Copy link
Contributor

I'll see if we can help/generate these phpdocs automatically, so it will make it easier to update the phpdocs in the core, if that helps?

@driesvints
Copy link
Member Author

@barryvdh I think that could be useful but I wonder how we'd automate it. Plus keep track of the nuances like other annotations.

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.

7 participants