Worker relative path resolution fails with nested folders

[jacogr]

• Version: 10.6
• Platform: Darwin Kernel Version 17.3.0
• Subsystem: workers

Relative path resolution in worker_threads does not quite do what is expected. I created a simple repo that simulates a yarn workspace environment where the issue is manifesting. However while the packages/*/ structure is there, there is no actual references between workspace packages, rather just 3 files that reference each other.

https://github.com/jacogr/node-worker-path

To Reproduce just run npm run test. Basically where packages/worker/src/index.js references the worker (which is in the same directory), the creation fails - Node is looking for the file in the root.

File does a const worker = new Worker('./worker.js'), where worker.js and the index.js is right alongside each other.

events.js:167
      throw er; // Unhandled 'error' event
      ^
Error: Cannot find module '/Users/jacogreeff/Projects/polkadot/node-worker-path/worker.js'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:581:15)
    at Function.Module._load (internal/modules/cjs/loader.js:507:25)
    at Function.Module.runMain (internal/modules/cjs/loader.js:742:12)
    at MessagePort.port.on (internal/worker.js:425:27)
    at MessagePort.emit (events.js:182:13)
    at MessagePort.onmessage (internal/worker.js:66:8)
Emitted 'error' event at:
    at Worker.[kOnErrorMessage] (internal/worker.js:296:10)
    at Worker.[kOnMessage] (internal/worker.js:306:37)
    at MessagePort.Worker.(anonymous function).on (internal/worker.js:243:57)
    at MessagePort.emit (events.js:182:13)
    at MessagePort.onmessage (internal/worker.js:66:8)
error Command failed with exit code 1.

It looks for Cannot find module '...node-worker-path/worker.js while the correct path in this case would be ....node-worker-path/packages/src/worker.js

[addaleax]

Fwiw, there’s some prior discussion around this in #21407 …

I agree with the behaviour that you’d expect, but that’s not quite easy to implement. I’d personally recommend always using absolute paths; path.resolve(__dirname, 'worker.js') should be enough here.

/cc @nodejs/workers

[benjamingr]

cc @itaysabato

[jacogr]

Yes, I did swap to absolute paths in the real code and it works as expected.

So there is a work-around, which is great - I actually started with absolutes in 10.5, then swapped to relatives with 10.6 and then back to absolutes when that didn't quite work as expected... ;)

I however still logged this, since the behavior is not quite what anybody would expect here. I appreciate the complexity of doing it consistently.

[itaysabato]

@jacogr I respectfully disagree that it is not what "anybody" would expect. I think resloving relative to CWD is actually what most people would expect (I am not the only one who thinks this, though I understand there may not be a consensus).

For example, child_process.fork() starts a module in a separate process similarly to how new Worker() starts a module in separate thread - and, by default, it resolves relative to CWD.

[jacogr]

@itaysabato Sure. There are inconsistencies in this -

In the same file, if I would do a require('./worker.js') it would load it from the directory the requester is in. So in my group of "anybody's" the expectation would be for new Worker('./worker.js') to point to the same file since that is the behaviour accustomed to. (If they had to do a fork, well, they would be surprised, maybe.)

So definitely not "anybody" & "everybody" - sadly there are surprises around every corner.

[Trott]

Is there anything to do here? It's not clear to me from the conversation if there's an improvement to be made for relative paths or not.

[Trott]

Is there anything to do here? It's not clear to me from the conversation if there's an improvement to be made for relative paths or not.

@nodejs/workers ^^^^^^

[guybedford]

So far as ES modules are concerned, contextual loading is a primary area of spec work at the moment, with various proposals and solutions available for these exact use cases, applying to more than just workers. The directions here are still unclear with many possibilities, but it does seem pretty certain that in an ES modules context this problem will be solvable using the contextual approaches that emerge, provided we are open to these directions.

[targos]

I'm going to close this as "working as intended". While I agree that it would be more intuitive to resolve relative to the file that instantiated the worker, relative to cwd has always been the documented behavior and it's possible to get the other behavior more easily from inside the calling module (with __dirname in CJS or import.meta.url in ESM).