fix: handle node: prefix

[Rich-Harris]

This appears to be enough to fix #284, but I'm not sure where a test would go

[ijjk]

We could add a unit fixture that uses node: imports/requires here https://github.com/vercel/nft/tree/main/test/unit would be good to ensure fs/path usage via node:fs and node:path catches the files it's reading correctly

[codecov]

Codecov Report

Merging #285 (085e4ae) into main (4a3725f) will increase coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head 085e4ae differs from pull request most recent head 6391685. Consider uploading reports for the commit 6391685 to get more accurate results

@@            Coverage Diff             @@
##             main     #285      +/-   ##
==========================================
+ Coverage   80.43%   80.44%   +0.01%     
==========================================
  Files          13       13              
  Lines        1482     1483       +1     
  Branches      552      555       +3     
==========================================
+ Hits         1192     1193       +1     
  Misses        120      120              
  Partials      170      170              

Impacted Files	Coverage Δ
src/analyze.ts	86.58% <100.00%> (ø)
src/resolve-dependency.ts	70.00% <100.00%> (+0.18%)	⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

[Rich-Harris]

Looks like that didn't fix it after all 😬

[guybedford]

@Rich-Harris let me know if I can assist with any NFT stuff anytime - it seems like the unit test just needs to be updated? The logging isn't ideal, may make sense to log the error before the warning to ensure the error can be seen at that call site in https://github.com/vercel/nft/blob/main/test/unit.test.js#L175.

[Rich-Harris]

@guybedford if you are able to get this working, that would be terrific — unfortunately I'm maxed out at the moment otherwise I'd spend more time digging into it. Right now I'm just working around it:

	traced.warnings.forEach((error) => {
		// pending https://github.com/vercel/nft/issues/284
		if (error.message.startsWith('Failed to resolve dependency node:')) return;
		console.error(error);
	});

[guybedford]

I don't have time to clone and PR right now, especially since I'm on Windows here. But I'd just suggest the following change in analyze.ts:

Line 305 change:

const m = staticModules[specifier.startsWith('node:') ? specifier.slice(5) : specifier];

line 345 change:

const staticModule = staticModules[source.startsWith('node:') ? source.slice(5) : source];

[ijjk]

Thanks!

[styfle]

Should this test be reverted so we cover both cases?

[ijjk]

Yeah we can revert this one since we have the separate fixtures now, updated in #287