-
-
Notifications
You must be signed in to change notification settings - Fork 244
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
Conditionally use pcntl
extension in inertia:start-ssr
command
#492
Conversation
It might be better to just include it in the suggestions. Not everyone uses SSR. |
@darthsoup Yeah you're right. I moved it to |
Is pcntl actually required to run SSR or is it only required for graceful shutdowns? |
@rojtjo depends on the distribution and the operating system. pcntl does not work under Windows |
I know it's not available on Windows, but was just wondering if it's even required to start the SSR server. Looking at the code it's only used to register a stop handler which kills the SSR process, but as far as I know the SSR process will also be automatically killed when the parent process ( |
That's probably right, could also just wrap the
Yeah, the artisan command isn't required in the first place, it's only added for error reporting I believe. I've added this to my own implementation of the SSR gateway. I personally won't use this command, as I find the setup strange (a PHP process to start a node process). But, as the code is right now, it requires pcntl. Don't know if it's desirable to rewrite the code to call these methods optionally |
Hey thanks for this @RobertBoes! I've just tweaked it a little to conditionally use the I think this should generally be fine, because even using the |
pcntl
extension in inertia:start-ssr
command
v0.6.6 introduced artisan commands to start the SSR server. Later this was updated (#487), but the code introduced there uses pcntl, which isn't currently part of the requirements. This adds
ext-pcntl
to the requirements. Since the docs now only provide information to start the SSR server through this command I feel like this requirement should be added to Inertia.One side-effect of this is that's it's impossible to use this feature on systems where the extension isn't available, like Windows.