tests: fix loading wasm when targeting external deps

[mochaaP]

• tests: fix loading wasm when targeting external deps
• Revert "fix: cannot resolve path when build with EXTERNAL_PATH"

Follow up of #91 and #93.

[mochaaP]

I'm sincerely sorry for the noise this caused 😅

[guybedford]

Actually I'm just very confused about what is going on here now - EXTERNAL_PATH is no longer being used as a runtime variable, but instead is being used as a define variable that is a string that is only used as a boolean check and not as a string.

But it is both a build environment variable and a define variable by name.

And in the build, we inline that path as an absolute path, while in the runtime the assumption is that lib/lexer.wasm is available relative to the build file?

And then we also wrap the entry point for the package for some kind of lazy wrapping as well as altering the main distribution file?

This is all just very very very convoluted, and I trusted that it would work but it clearly doesn't.

I've put together a revert PR in #95, to remove these build changes entirely.

[mochaaP]

EXTERNAL_PATH is no longer being used as a runtime variable, but instead is being used as a define variable that is a string that is only used as a boolean check and not as a string.

It was never a runtime variable. The purpose of this is to enable externalized build, and build dist/lexer.js as a loader of EXTERNAL_PATH/dist/lexer.mjs, since node could not resolve filesystem paths with external builtins. It was not exported by package.json nor imported by tests. It's only accessed when referenced from node's build process.

while in the runtime the assumption is that lib/lexer.wasm is available relative to the build file?

The relative path change was done to make sure a single external build can handle the reading of lexer.wasm both in-tree and installed to EXTERNAL_PATH.

I understand the efforts you put into this, and the necessity to make things simpler. But based on the reasons above, #95 is, unfortunately, not a choice here.

[guybedford]

The relative path change was done to make sure a single external build can handle the reading of lexer.wasm both in-tree and installed to EXTERNAL_PATH.

There is no variable in https://github.com/nodejs/cjs-module-lexer/pull/94/files#diff-7d867ca4df223e0c07d997702e598243e0644cb647e57d8363a6652a8888cf1fR97, so how is this the case in the code?

[mochaaP]

A step by step breakdown:

• when required in-tree (e.g., from test/_unit.js)

  • await import('../dist/lexer.mjs');
  • (in dist/lexer.mjs) import.meta.resolve('.') == "file:///path/to/your/repo/dist/lexer.mjs"
  • import.meta.resolve('../lib/lexer.wasm')
  • "file:///path/to/your/repo/lib/lexer.wasm"

• when required as an externalized builtin (from node)

  • require('internal/deps/cjs-module-lexer/dist/lexer');
  • (in EXTERNAL_PATH/dist/lexer.js) node:module magic

    ℹ️ to let dist/lexer.js believe it's loaded as EXTERNAL_PATH/dist/lexer.js instead of as an external, in order not to lose the path context. [this is the only difference]

  • import('./lexer.mjs') (import.meta is already setup correctly here, so we can do relative path stuff safely)
  • import.meta.resolve('../lib/lexer.wasm')
  • "file:///EXTERNAL_PATH/lib/lexer.wasm"

[guybedford]

Thanks for clarifying, if you're doing internal module hooks to alter internal resolution rules, then this pattern needs to be clearly identified and described as part of this work.