fix: Support resolving module.register dependencies

[timfish]

Closes #428

module.register(specifier[, parentURL][, options])

[styfle]

Thanks for the PR!

Can you change the integration test into a unit test instead?

That way we can see see the input and output to make sure these remain the same instead of relying on implementation details of a 3rd party package.

[timfish]

Looks like I need to pull my Windows laptop out and work out why those tests are failing!

[timfish]

So the failing unit test (zeromq-node-gyp) passes in isolation on my Windows machine but fails when all the tests are run.

One thing I don't understand is how it's passing at all, even on other platforms.

The expected output is:

nft/test/unit/zeromq-node-gyp/output.js

Lines 1 to 11 in 31ab417

	[
	"node_modules/@aminya/node-gyp-build/index.js",
	"node_modules/@aminya/node-gyp-build/package.json",
	"node_modules/zeromq/lib/draft.js",
	"node_modules/zeromq/lib/index.js",
	"node_modules/zeromq/lib/native.js",
	"node_modules/zeromq/lib/util.js",
	"node_modules/zeromq/package.json",
	"package.json",
	"test/unit/zeromq-node-gyp/input.js"
	]

But it's failing with an extra file included:

        "node_modules\\@aminya\\node-gyp-build\\index.js",
    +   "node_modules\\@aminya\\node-gyp-build\\node-gyp-build.js",
        "node_modules\\@aminya\\node-gyp-build\\package.json",
        "node_modules\\zeromq\\lib\\draft.js",
        "node_modules\\zeromq\\lib\\index.js",
        "node_modules\\zeromq\\lib\\native.js",
        "node_modules\\zeromq\\lib\\util.js",

And the reason it's being included:

      'node_modules\\@aminya\\node-gyp-build\\node-gyp-build.js' => {
        type: [ 'dependency' ],
        ignored: false,
        parents: Set(1) { 'node_modules\\@aminya\\node-gyp-build\\index.js' }
      }

And then when you look at the actual source of the parent, it's obvious why this is being included:

} else { // else use the runtime version here
  module.exports = require('./node-gyp-build.js')
}

So how is this passing on every other platform but failing on Windows when the full test suite is run?

[styfle]

This might fix it: #432

I merged it in to see if tests pass now 🤞

[timfish]

Unfortunatly not 😭

I'm more confused as to how this test passes on any platform. The additional dependency in the failing case looks like it should be included!

[styfle]

Looks like the only failing test is the new one now. Its missing hook.mjs, hook2.mjs and hook3.mjs.

 ● should correctly trace module-register from cwd

    expect(received).toEqual(expected) // deep equality

    - Expected  - 3
    + Received  + 0

      Array [
        "package.json",
    -   "test\\unit\\module-register\\hook.mjs",
    -   "test\\unit\\module-register\\hook2.mjs",
    -   "test\\unit\\module-register\\hook3.mjs",
        "test\\unit\\module-register\\input-esm.mjs",
        "test\\unit\\module-register\\input.js",
        "test\\unit\\module-register\\node_modules\\test-pkg\\index.mjs",
        "test\\unit\\module-register\\node_modules\\test-pkg\\package.json",
      ]

https://github.com/vercel/nft/actions/runs/9785332243/job/27018410588?pr=429#step:9:1063

[timfish]

So I reverted the path.sep change and the tests now all pass on Windows.

Looking at the resulting deps and imports and on Windows for ALL the tests, these all contain forwards slashes /.

[s1gr1d]

@styfle can this be merged?

[EndangeredMassa]

Looks good, but there's one case we should add.

Thanks!

[styfle]

LGTM, thanks!

@EndangeredMassa Want to give it another look?

[github-actions]

🎉 This PR is included in version 0.27.4 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀