-
Notifications
You must be signed in to change notification settings - Fork 305
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
Use FrankenPHP's built-in file watcher #971
base: 2.x
Are you sure you want to change the base?
Conversation
@@ -103,7 +103,9 @@ public function handle(ServerProcessInspector $inspector, ServerStateFile $serve | |||
'CADDY_SERVER_LOGGER' => 'json', | |||
'CADDY_SERVER_SERVER_NAME' => $serverName, | |||
'CADDY_SERVER_WORKER_COUNT' => $this->workerCount() ?: '', | |||
'CADDY_SERVER_WORKER_DIRECTIVE' => $this->workerCount() ? "num {$this->workerCount()}" : '', |
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.
Having CADDY_SERVER_WORKER_DIRECTIVE
in addition to the previous CADDY_SERVER_WORKER_COUNT
allows for backwards compatibility if users have their own previous version of the Caddyfile.
The new variable is needed because the num
directive must have a value provided.
Hey, awesome PR :) By any chance, have you tested Franken's watcher in Docker on MacOS? Chokidar has served us very good in that regard, so I'm wondering whether e-watcher used by Franken is as good. |
@oprypkhantc Thanks! The reason it's still marked as a draft because I have not finished doing testing with Docker. I know that a lot of people use macOS, but I do about 98% of my development work on Windows and Linux, so I have to borrow a Mac at work to finish my testing. |
Got it. I would gladly help you test it, but I really lack the time for it right now :( |
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.
LGTM
Since version 1.3, FrankenPHP now has a native file watcher.
This PR switches to use that watcher, instead of needing a separate NodeJS process that restarts the entire server every time a file changes.
For example. this will allow running watch mode in a Docker container that does not have NodeJS.
It might also help fix the same issue as #932 without needing to disable worker mode, though I have not tested that myself.