Skip to content

fix(watch): dispatch unload and process exit events on restart#32664

Open
bartlomieju wants to merge 1 commit intodenoland:mainfrom
bartlomieju:fix/watch-unload-event
Open

fix(watch): dispatch unload and process exit events on restart#32664
bartlomieju wants to merge 1 commit intodenoland:mainfrom
bartlomieju:fix/watch-unload-event

Conversation

@bartlomieju
Copy link
Member

Summary

  • Fix unload event not firing when watch mode restarts a script blocked on a top-level await
  • Fix Node.js process.on("exit") handlers not firing on watch restart

What changed

In FileWatcherModuleExecutor::execute(), pending_unload was previously set
after execute_main_module() and dispatch_load_event(). This meant that if
the operation future was cancelled during a top-level await (the module never
finished evaluating), pending_unload stayed false and the Drop impl never
dispatched the unload event.

Now pending_unload is set before execute_main_module() and reset to
false only on module evaluation error (syntax errors, import failures). When
the future is cancelled (dropped), pending_unload remains true and Drop
correctly dispatches both the unload event and the process exit event.

The Drop impl also now calls dispatch_process_exit_event() alongside
dispatch_unload_event(), so Node.js process.on("exit") handlers fire on
restart — matching the normal shutdown path.

Towards #30912

Test plan

  • Added run_watch_unload_on_restart integration test — verifies addEventListener("unload", ...) fires on file change restart
  • Added run_watch_process_exit_on_restart integration test — verifies process.on("exit", ...) fires on file change restart
  • Manually verified: unload fires during blocking top-level await
  • Manually verified: both unload and process exit fire on Ctrl+C (after grace period)
  • Manually verified: unload does NOT fire on module evaluation errors (syntax errors)

🤖 Generated with Claude Code

Previously, `pending_unload` was only set after both module execution
and the load event, so cancellation during a top-level await meant the
unload event never fired. Now `pending_unload` is set before module
execution and reset on error, ensuring the unload event fires even when
the future is cancelled mid-evaluation.

Also adds `dispatch_process_exit_event()` to the Drop impl so that
Node.js `process.on("exit")` handlers fire on watch restart, matching
the normal shutdown path.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Contributor

@marvinhagemeister marvinhagemeister left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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.

2 participants