Skip to content

Conversation

nojaf
Copy link
Member

@nojaf nojaf commented Sep 5, 2025

Replaced execFileSync with spawn to prevent blocking the event loop, allowing for proper signal forwarding and cleanup on termination. This change enhances the user experience during watch mode by ensuring that the Rust watcher can exit cleanly and that the parent process does not terminate prematurely. Added signal handling for SIGINT, SIGTERM, SIGHUP, and SIGQUIT to manage child process termination effectively.

This is to prevent the terminal being blocked after sending ctrl + c during watch mode.

Kapture 2025-09-05 at 17 16 14

Problem goes away after this PR:

Kapture 2025-09-05 at 17 17 55

Copy link

pkg-pr-new bot commented Sep 5, 2025

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript-lang/rescript@7844

@rescript/darwin-arm64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-arm64@7844

@rescript/darwin-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-x64@7844

@rescript/linux-arm64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-arm64@7844

@rescript/linux-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-x64@7844

@rescript/runtime

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/runtime@7844

@rescript/win32-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/win32-x64@7844

commit: ba50294

@nojaf nojaf marked this pull request as ready for review September 5, 2025 15:39
@nojaf
Copy link
Member Author

nojaf commented Sep 5, 2025

Worked on Windows as well.

@zth zth requested a review from cknitt September 5, 2025 15:55
@zth
Copy link
Member

zth commented Sep 5, 2025

👍 from me, great stuff! But would be good if @cknitt had a look.

@cknitt
Copy link
Member

cknitt commented Sep 5, 2025

Thanks! I wonder if this logic shouldn't be extracted to some helper function and used in other places, too, like bsc.js or in the rescript legacy code.

Actually we already have lib_dev/process.js with similar utility functions, so it could go there.

@nojaf
Copy link
Member Author

nojaf commented Sep 5, 2025

Hmm, I'm not sure really, I’d prefer keeping this in the CLI runtime rather than lib_dev/process.js (which is dev/test-only and has different behavior).

bsc.js runs a single-shot compiler invocation without a watch loop. There’s no long-lived child to gracefully tear down on Ctrl+C, so execFileSync is appropriate and simpler. The problem we fixed only shows up in rescript.js because rewatch runs indefinitely and needs orderly signal forwarding/cleanup.

The user problem is with rescript.js, fixing it there seems fine to me.

@nojaf nojaf merged commit 535a8bd into rescript-lang:master Sep 5, 2025
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants