Skip to content
New issue

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

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

Already on GitHub? # to your account

File watching in development environments #458

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

XbNz
Copy link
Contributor

@XbNz XbNz commented Jan 1, 2025

Note

Accompanying PR on the Electron side should be merged together with this

I was torn as to whether this should be something we even handle. It's sort of a grey area, and I still don't know if it's appropriate for us to provide this functionality. Perhaps we can provide the event only, and allow the user to add their own pipeline steps if they are so inclined.

Feel free to throw out parts of it – or even the whole thing.

  • Implement event to fire when file changes
  • Implement listener to capture aformentioned event and invoke pipeline
  • Implement 1 default pipeline step: restarting queue workers
  • Test case asserting pipeline is invoked
  • Test case asserting queue restart initiated

- Implement listener to capture aformentioned event and invoke pipeline
- Implement 1 default pipeline step: restarting queue workers
- Test case asserting pipeline is invoked
- Test case asserting queue restart initiated
@SRWieZ
Copy link
Member

SRWieZ commented Jan 2, 2025

If this is about queues, a simpler change would be to use queue:listen instead of queue:work when in dev mode.

@simonhamp
Copy link
Member

Agreed. Great suggestion @SRWieZ

I do like the idea of a file watcher as a general-purpose tool tho, that has come up before as a feature some have wanted or looked to implement in other ways...

@SRWieZ
Copy link
Member

SRWieZ commented Jan 2, 2025

I like the idea of a file watcher, especially for production applications, not just for development.

It could be facade, we can provide a directory and a custom event that is triggered with $filename and $eventType.

Although I'm not for adding dependencies, I believe that https://github.com/paulmillr/chokidar is a better option than fs.watch, as explained in their README. It is a well-known and widely used library too.


Currently, I use chokidar with spatie/file-system-watcher, but the built version of my app requires the user to have Node installed on their system. I was planning to add ChildProcess::node() to create my own chokidar instance, but a built-in one may be a better choice.

@XbNz
Copy link
Contributor Author

XbNz commented Jan 2, 2025

If this is about queues, a simpler change would be to use queue:listen instead of queue:work when in dev mode.

The principle applies to all child processes, not just queue workers. Restarting queue workers was just one functionality I exposed as a demonstration.

queue:listen would give us what we need for queue workers, yes. I don't think it'd be prudent to use it in production though. From the docs:

When using the queue:listen command, you don't have to manually restart the worker when you want to reload your updated code or reset the application state; however, this command is significantly less efficient than the queue:work command

So even with this, we'd probably have to split it up so it uses queue:work in prod and queue:listen in development. I don't love that. There is also the additional problem that it still does not solve the code refreshing issue for other types of long-running child processes, not just queue workers.

Although I'm not for adding dependencies, I believe that https://github.com/paulmillr/chokidar is a better option than fs.watch, as explained in their README. It is a well-known and widely used library too.

Yep, I wanted to use chokidar too but avoided it so as not to add more dependencies.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants