-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
v2.9.1: artisan ide-helper:models
stopped working after update
#1188
Comments
Figured. It was because the model contained |
Looking at https://github.com/barryvdh/laravel-ide-helper/releases/tag/v2.9.1 , seems it this one:
|
I'm having this problem too. I had to downgrade to v2.9.0 to fix it. |
Can confirm 2.9.1 introduced this. Seems to be a pretty major BC, no? |
Can anyone experiencing this issue bisect which change it really was? I don't use that feature myself so can't debug 🤷♀️ also re-opening: it seems OP meant his fix was to downgrade (which I wouldn't consider a fix) |
Also pinging @ahmed-aliraqi maybe has an idea if it's related to your PR? |
No, the fix was to remove |
Well, to be fair, downgrading to 2.9.0 (since 2.9.1 has #1074) or removing HasFactory "fixes" it. Ha. |
artisan ide-helper:models
stopped working after update to v2.9.1artisan ide-helper:models
stopped working after update
I added this line to the release, does it sound technically correct? https://github.com/barryvdh/laravel-ide-helper/releases/tag/v2.9.1
Not sure if we should do more here (upgrading guide for this single thing? 🤔) I've pinned the issue and edited the title slightly to make it quickly discoverable 🤞 |
I mean... it makes sense (because it's needed obviously ha), but it seems... strange to have to remove something included by default in every single model that artisan would make. Wouldn't a better approach be to just fail gracefully if there's no actual factory for the model, if that's possible? Hell, at that point I'd even be willing to say the artisan command shouldn't even include the HasFactory if you didn't pass the factory creation flag but that's another point. |
Good points. I really don't have any experience with this feature, so if someone wants to propose a more graceful handling, by all means please suggest a PR! |
So can we revert the PR? |
The way I understand it, the change points out broken constellations but is by itself technically correct. |
Yeah maybe we should make the error less intimidating though. |
I think a bit better documentation or a flag in ide-helper.php would be a solid solution. For example, I would honestly have thought I suppose the biggest hangup for me is how something that's default (HasFactory) in Laravel/Artisan needing to be removed for every single model that doesn't have factories, to me, makes it seem like the PR is a BC. Is it sad I've literally thought to myself "well, instead of removing all of these HasFactory traits all the time, I'll just pass the -f flag when creating models"? Ha. Keep up the amazing work on this package, I literally wouldn't use Laravel without it. :P |
Versions:
Description:
Upd.
Solution:
Be sure to remove any
use HasFactory;
on models not having factories to avoid crashes in ide-helperThe text was updated successfully, but these errors were encountered: