Review requested:

• @nodejs/loaders

Thank you for looking into this. This is definitely something that we should improve.

For reference, here are what error message other runtimes emit when doing import('data:text/javascript,export default import.meta.resolve("bare-package-name")').catch(console.error):

All of the other runtime error messages you cite mention relative URLs. I think a better comparison would be what messages they throw for your example code when an import map is registered that defines that bare import. Do any of them resolve it?

All of the other runtime error messages you cite mention relative URLs.

The Safari one does not.

I think a better comparison would be what messages they throw for your example code when an import map is registered that defines that bare import. Do any of them resolve it?

Import maps are only supported by Chromium and Deno, and yes they do resolve them. oops my testing was flawed, they do support it, let me recheck

If the import map maps bare-package-name to an absolute URL, it resolves it. If the import map maps it to a relative URL, it throws the same error as before.

If the import map maps bare-package-name to an absolute URL, it resolves it.

Then I think we should do the same, per our own resolution algorithm. That was my expectation when I tried that code; it doesn’t really make sense to me as a user that it should error. Like, import of a bare specifier within a data: URL works, so therefore so should import.meta.resolve.

I don't understand what do you mean. Could you lay down what would be the expected behavior vs the actual behavior?

For this code:

import('data:text/javascript,export default import.meta.resolve("pkg")').catch(console.error)

Currently this code always errors. Assuming that import('pkg') would resolve, then the above code should no longer error, and import.meta.resolve("pkg") should return the file URL to 'pkg'.

What would it resolve to?

should return the file URL to 'pkg'.

You're talking about the file URL, but we're in a data URL. How could I link the two?

What would it resolve to?

I’m not sure, but as a user I expect it to work. Like I assume this works?

import('data:text/javascript,import("pkg")')

(Assuming that pkg itself exists, that import('pkg') would work in normal non-data: URL code.)

If the above works, then we’re already resolving that import somehow, so import.meta.resolve would use the same approach.

It doesn't work, and it cannot work. It's documented in

It doesn’t work, and it cannot work.

Well it cannot work as we’ve currently defined data: URLs, but it can work as we can see browsers doing it via import maps. And maybe that’s the answer for us too, that the current filesystem-based resolution algorithm cannot resolve imports in data: URLs but when we add support for import maps then this begins to work when an import map is loaded.

I guess the alternative would be to define the parentURL for data: URLs to be pathToFileURL(process.cwd()), but I’m sure you’ll tell me why that’s a bad idea 😄

Anyway thanks for improving the error messaging, that’s a good improvement for now.

I guess the alternative would be to define the parentURL for data: URLs to be pathToFileURL(process.cwd()), but I’m sure you’ll tell me why that’s a bad idea 😄

The parentURL for data: modules is whatever module loaded that data: module, so changing it wouldn't help – and yes it would break things.

Well it cannot work as we’ve currently defined data: URLs, but it can work as we can see browsers doing it via import maps. And maybe that’s the answer for us too, that the current filesystem-based resolution algorithm cannot resolve imports in data: URLs but when we add support for import maps then this begins to work when an import map is loaded.

Not really, because we don't have import map support nor anything that looks like it. Worth noting that the same data: URL can be imported from different context (different folder, but even different protocol, e.g. the same data: module can be imported by an HTTP module and a file one). Consider the following:

// /root/file.mjs
export {default} from 'data:text/javascript,export {default} from "pkg"';

// /root/node_modules/pkg/index.cjs
module.exports=1;

// /home/user/file.js
export {default} from 'data:text/javascript,export {default} from "pkg"';

// /home/user/node_modules/pkg/index.cjs
module.exports=2;

and then you run node --input-type=module -e 'import One from 'file:///root/file.js;import OneAsWell from 'file:///home/user/file.js', it would be very confusing.

I don't really like the name of the error (ERR_UNSUPPORTED_RESOLVE_REQUEST), nor am I happy with the documentation text. Open to suggestions.

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

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

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

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

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

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

Landed in 3fbe157