watch: track worker thread entry files in --watch mode

[SudhansuBandha]

Currently, --watch mode only tracks dependencies from the main module graph (require/import). Worker thread entry points created via new Worker() are not included, so changes to worker files do not trigger restarts.

This change hooks into Worker initialization and registers the worker entry file with the watch mode, ensuring restarts when worker files change.

Fixes: #62275

[clemyan]

This solves the issue for the worker module itself, but not its dependencies. I.e. if a worker module imports/requires another module, modifying the latter still does not trigger re-run.

[SudhansuBandha]

I have updated changes to watch dependencies for worker as well, so changes in imported/required modules will now trigger a re-run.
@clemyan

[clemyan]

This is the wrong way to solve this, for a myriad of reasons: (based on a cursory glance of the code. Did not bother to actual test it)

1. It is impossible to properly parse JS with regex. This approach cannot even distinguish if the "import" is part of a string or a comment
2. It only works for direct dependencies of the worker module, not transitively. (I.e. it does not watch dependencies of dependencies, and their dependencies, etc.)
3. It only works for literals specifiers that are relative paths, not for absolute paths require('/path/to/file.js'), packages require('my-pkg/sub/path'), dynamic specifier require('./messages/'+language+'/string.json'), or paths that are resolved by customization hooks
4. What about the worker module itself? If it is loaded by customization hooks, there isn't necessarily a file on disk to readSync from.

Any solution must be integrated into Node's loading pipeline

[SudhansuBandha]

I will revert back these updates and work on the suggestions
@clemyan

[SudhansuBandha]

@clemyan
I have updated handling of dependencies from worker thread. For this I have also reverted my changes from the initial commit as well f9beab0
I have implemented the solution for both cjs and esm files.
Do let me know your feedback on the proposed solution and whether it makes sense to proceed further in this direction.

[aduh95]

The added test already passes on main, that means either the code change is not necessary, or that it's not actually testing what you're trying to achieve

[SudhansuBandha]

Sure!! @aduh95 I will update the tests for this. Thanks for your feedback!!

[SudhansuBandha]

@aduh95 Please do review the updated tests for the proposed changes based on behaviour of worker file and it's dependencies in --watch mode for both cjs and esm modules.

[aduh95]

Can you not put the tests in test/sequential/test-watch-mode.mjs? That file is enormous, which is becoming a maintenance issue/

[SudhansuBandha]

@aduh95 As suggested I have removed tests from test/sequential/test-watch-mode.mjs to a new file and made few lint fixes.
Do let me know if any further changes are required

[codecov]

Codecov Report

❌ Patch coverage is 94.54545% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.66%. Comparing base (4e3a873) to head (8fa637a).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/modules/cjs/loader.js	87.50%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #62368      +/-   ##
==========================================
- Coverage   91.47%   89.66%   -1.82%     
==========================================
  Files         357      707     +350     
  Lines      150883   219563   +68680     
  Branches    23778    42096   +18318     
==========================================
+ Hits       138026   196875   +58849     
- Misses      12582    14585    +2003     
- Partials      275     8103    +7828     

Files with missing lines	Coverage Δ
lib/internal/modules/esm/loader.js	99.69% <100.00%> (+38.37%)	⬆️
lib/internal/worker.js	96.74% <100.00%> (+0.22%)	⬆️
lib/internal/modules/cjs/loader.js	98.25% <87.50%> (+18.64%)	⬆️

... and 467 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[aduh95]

/cc @nodejs/fs

[SudhansuBandha]

Thank you @aduh95 for your support so far in making necessary updates to the PR and running the CI pipeline.
I have made necessary updates as per latest feedback. Do let me know if any further changes are needed.

[SudhansuBandha]

Can we add label fs to the PR?
@nodejs-github-bot

[clemyan]

I can confirm this solves my issue, but this does cause additional userland messages to be sent on the parentPort of workers run under watch mode. I'll leave deciding whether this is an issue to the Node team.

[SudhansuBandha]

@MoLow I have updated the event relays in worker to handle RACE conditions for specific scenarios. Kindly review it and provide any feedback.

[SudhansuBandha]

@MoLow can we proceed with next round of CI run since preliminary checks are successful?

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/72979/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/72984/

[SudhansuBandha]

Is it good to be merged with main branch? @MoLow

[SudhansuBandha]

Thank you @MoLow, @aduh95 and @clemyan for your guidance and support on this!!
This is my first ever PR in Nodejs and Openspource even
:)

[nodejs-github-bot]

Landed in 7466249