laravel-ide-helper icon indicating copy to clipboard operation
laravel-ide-helper copied to clipboard

Fix issue where \Eloquent is not included when using write_mixin

Open Jefemy opened this issue 2 years ago • 6 comments

Summary

If you write your models using the --write-mixin option the included docblock generated on the model will include a mixin. Previously the code replaced here would ignore the @mixin \Eloquent addition if any mixin existed on the model. This would break the code suggestions as on first write the Eloquent mixin would be added but after that it wouldn't be generated as its detected as 'already existing'. Info including the lines and commits where I found this are discussed in #1299

This fix changes it to a loop which removes any mixins that matches the Eloquent class before appending it.

Also fixes #1342, fixes #1343, fixes #1299

Type of change

  • [x] Bug fix (non-breaking change which fixes an issue)

Checklist

  • [x] Existing tests have been adapted and/or new tests have been added
  • [x] Add a CHANGELOG.md entry
  • [ ] Update the README.md
  • [x] Code style has been fixed via composer fix-style

Jefemy avatar May 22 '22 18:05 Jefemy

Will this end up writing @mixin \Eloquent to the model class, or to the generated _ide_helper_models.php classes?

FatBoyXPC avatar May 25 '22 14:05 FatBoyXPC

Will this end up writing @mixin \Eloquent to the model class, or to the generated _ide_helper_models.php classes?

It will put it in the generated _ide_helper_models class. This pull request changes no base functionality in the package, it just fixes an issue where on a second call of the ide-helper:models --write-mixin it will create a different output.

To temporarily work around the issue while this isn't merged you can delete the docblock written to your main models that contains the @mixin IdeHelperModel and rerun the generate command. You will see that @mixin \Eloquent is properly included in _ide_helper_models for those models.

Jefemy avatar May 25 '22 14:05 Jefemy

For the temporary work around, do you mean delete the @mixin IdeHelperFoo from the Foo model?

FatBoyXPC avatar May 25 '22 15:05 FatBoyXPC

For the temporary work around, do you mean delete the @mixin IdeHelperFoo from the Foo model?

Yes. In the current version any mixins existing on the original model will cause \Eloquent to be excluded from the generated class.

Jefemy avatar May 25 '22 15:05 Jefemy

Wow - so this also doesn't add it when you add a brand new model (it'll add the mixin to the model but not to the generated class). Certainly frustrating. Thanks for this PR!

FatBoyXPC avatar May 25 '22 15:05 FatBoyXPC

@barryvdh any plans to merge this pull request?

dnmarques avatar Aug 01 '22 19:08 dnmarques