-
Notifications
You must be signed in to change notification settings - Fork 253
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
feat: Add watcher option to generate command and allow for options to be specified multiple times #522
Conversation
23d6118
to
e6e28d0
Compare
"require": { | ||
"ext-json": "*", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was missing and is required to use json_encode / json_decode in php, even though this extension is usually installed by default in php
@@ -33,6 +33,9 @@ You can run PHP tests with `vendor/bin/phpunit` and JavaScript tests with `npm t | |||
|
|||
If you need any help with this please don't hesitate to ask. | |||
|
|||
> :warning: If your filesystem uses CRLF instead of LF you may run into some issues when running the tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added this because I've been struggling for a good hour with this in a WSL2 install while running tests locally
@Tofandel I appreciate the work you put into this but I'm going to pass for now. This feature has been suggested before but it's simpler to just run |
Hmm I strongly disagree, the mix hook doesn't have any watchers, without this command modification it has no point at all because the watchers watch the js files and not the php files, so you'd have to restart the server every time you make a route change, I'm already using this and it works really perfectly It's really not simpler to run a command when you're developing with mix watch, especially when you have multiple files to generate at the same time, with this, you just run I really can't comprehend why you would turn down a nice feature that eases developer experience. Especially if this was suggested before and I'm serving the PR on a platter with no breaking change and a full set of tests and documentation |
Happy to explain further. I don't want to reach into Laravel to modify internal properties, reload configurations, reboot the app, etc. It's not a great idea to do stuff like that in general, but it's particularly risky in a package like Ziggy because app developers may not know that that's happening and it could interact with their app and with other packages in unexpected ways. That would have to be removed from this PR. Even without those parts though, it adds a lot of complexity that we'll have to maintain, and I don't think it's worth it for this feature considering that there are other ways to accomplish the same thing. It's an interesting idea, and I really do appreciate the time you put in to contribute! Here's an example from an earlier discussion of how to add a watcher to Mix to recompile when a route file changes: #321 (comment). It could probably be even simpler though: const { exec } = require('child_process');
const { resolve } = require('path');
const glob = require('glob');
mix.webpackConfig({
plugins: [
{
apply: (compiler) => {
compiler.hooks.compilation.tap('WatchRoutesPlugin', (compilation) => {
glob.sync('./routes/*.php').map(file => compilation.fileDependencies.add(resolve(file)));
});
compiler.hooks.afterDone.tap('ZiggyGeneratePlugin', () => {
exec('php artisan ziggy:generate');
});
}
},
],
}); |
Okay sorry I misunderstood this code I could modify this PR then, and actually provide a webpack plugin that gives the feature like proposed as I still really would like the multiple arguments part and the not rewriting file if it wasn't modified (as this triggers a useless full recompilation) |
This is a very nice backward compatible improvement to the ziggy:generate command
It allows the command to wait until Enter is pressed meanwhile watching for file change to regenerate them, this is very usefull in conjonction with the watch option of webpack or laravel mix as it allows for a development process not requiring manually running commands
Here is an example of usage
php artisan ziggy:generate ./resources/js/Routes/admin.js --group=admin ./resources/js/Routes/public.js --watch