Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTPS imports fail for relative URLs #42098

Closed
yavorg opened this issue Feb 18, 2022 · 16 comments
Closed

HTTPS imports fail for relative URLs #42098

yavorg opened this issue Feb 18, 2022 · 16 comments
Labels
confirmed-bug Issues with confirmed bugs. esm Issues and PRs related to the ECMAScript Modules implementation. module Issues and PRs related to the module subsystem.

Comments

@yavorg
Copy link

yavorg commented Feb 18, 2022

Details

Part of the goal of the network import feature for ECM is that I should be able to load a module hosted via any HTTPS location, like for example common CDNs for hosting such modules such as https://esm.run or https://unpkg.com.

For example, the popular uuid package claims it's an ESM:
image

It is hosted in the various CDNs:

Yet, none of these work when trying to load with the new network imports, see snippet.

Node.js version

17.5 with --experimental-network-imports

Example code

Example:

import { v4 as uuidv4 } from 'https://cdn.jsdelivr.net/npm/[email protected]/wrapper.mjs';
console.log(uuidv4());

Output:

> node --experimental-network-imports index.js
node:internal/errors:465
    ErrorCaptureStackTrace(err);
    ^

TypeError [ERR_INVALID_URL]: Invalid URL
    at new NodeError (node:internal/errors:372:5)
    at onParseError (node:internal/url:563:9)
    at new URL (node:internal/url:643:5)
    at ESMLoader.resolve (node:internal/modules/esm/loader:578:5)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async ESMLoader.getModuleJob (node:internal/modules/esm/loader:250:7)
    at async ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:81:21)
    at async Promise.all (index 0)
    at async link (node:internal/modules/esm/module_job:86:9) {
  input: './dist/index.js',
  code: 'ERR_INVALID_URL'
}

Node.js v18.0.0-nightly20220216dace5a804e

Another example

import { v4 as uuidv4 } from 'https://unpkg.com/browse/[email protected]/wrapper.mjs'

console.log(uuidv4());

Output:

> node --experimental-network-imports index.js
node:internal/errors:465
    ErrorCaptureStackTrace(err);
    ^

RangeError [ERR_UNKNOWN_MODULE_FORMAT]: Unknown module format: null for URL https://unpkg.com/browse/[email protected]/wrapper.mjs
    at new NodeError (node:internal/errors:372:5)
    at ESMLoader.load (node:internal/modules/esm/loader:385:13)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async ESMLoader.moduleProvider (node:internal/modules/esm/loader:282:11) {
  code: 'ERR_UNKNOWN_MODULE_FORMAT'
}

Node.js v18.0.0-nightly20220216dace5a804e

Operating system

MacOS 12.1

Scope

Runtimne

Module and version

Not applicable.

@MartinKolarik
Copy link

MartinKolarik commented Feb 18, 2022

TypeError [ERR_INVALID_URL]: Invalid URL

From this error, it seems the loader doesn't support relative URL references yet, which would mean it won't work with... almost anything 🤔 Or maybe it's the .js extension that's the issue?

@ljharb
Copy link
Member

ljharb commented Feb 23, 2022

@nodejs/modules

@GeoffreyBooth GeoffreyBooth transferred this issue from nodejs/help Feb 23, 2022
@GeoffreyBooth GeoffreyBooth changed the title How do I use new network imports with ESM CDNs HTTPS imports fail for relative URLs Feb 23, 2022
@GeoffreyBooth GeoffreyBooth added esm Issues and PRs related to the ECMAScript Modules implementation. module Issues and PRs related to the module subsystem. labels Feb 23, 2022
@GeoffreyBooth
Copy link
Member

This looks like a bug with --experimental-network-imports, which is very new (shipped about a week ago). The ESM support that Node has had for the last few years hasn’t included support for http: or https: URLs, until --experimental-network-imports, so the docs around ESM are generally referring to local file URLs.

@JakobJingleheimer
Copy link
Member

JakobJingleheimer commented Feb 23, 2022

I'm working on a fix (for https modules that contain relative imports)

@Mesteery Mesteery added the confirmed-bug Issues with confirmed bugs. label Feb 24, 2022
@giltayar
Copy link
Contributor

This may be obvious, but putting it here in case it isn't: even if relative imports worked, uuid-s wrapper.mjs won't work because ./dist/index.js is a CJS module.

@targos
Copy link
Member

targos commented Feb 24, 2022

This may be obvious, but putting it here in case it isn't: even if relative imports worked, uuid-s wrapper.mjs won't work because ./dist/index.js is a CJS module.

import 'https://ga.jspm.io/npm:[email protected]/wrapper.mjs' should work. jspm transforms CJS to ESM.

@MartinKolarik
Copy link

So does jsDelivr, import { v4 as uuidv4 } from 'https://cdn.jsdelivr.net/npm/[email protected]/wrapper.mjs/+esm'; should work once the paths issue is fixed.

@benjamingr
Copy link
Member

benjamingr commented Feb 24, 2022

A fix would be something like:

index 61e609d9ad..d0579e5e5b 100644
--- a/lib/internal/modules/esm/loader.js
+++ b/lib/internal/modules/esm/loader.js
@@ -574,11 +574,12 @@ class ESMLoader {
       );
     }
 
-    new URL(url); // Intentionally trigger error if `url` is invalid
+    // Error early and resolve base urls for https imports
+    const withParent = new URL(url, parentURL);
 
     return {
       format,
-      url,
+      url:  withParent.href,
     };
   

Note @MartinKolarik that https://cdn.jsdelivr.net/npm/[email protected]/wrapper.mjs/+esm returns a 404 so I wasn't able to test that with the patch.

(Edit: note a real patch would also include fixing local imports that is fixing checkIfDisallowedImport )

@MartinKolarik
Copy link

MartinKolarik commented Feb 24, 2022

Note @MartinKolarik that https://cdn.jsdelivr.net/npm/[email protected]/wrapper.mjs/+esm returns a 404 so I wasn't able to test that with the patch.

Thanks for the ping, I forgot it now enforces the exports field entry points, so it would be just https://cdn.jsdelivr.net/npm/[email protected]/+esm but that's the browser version 🤔 I suppose restricting the entry points on a CDN is not a good idea, we'll change it.

@benjamingr
Copy link
Member

but that's the browser version 🤔

Yeah, that returns:

Error: crypto.getRandomValues() not supported. See https://github.com/uuidjs/uuid#getrandomvalues-not-supported

With the patch but at least it loads the package.

@MartinKolarik
Copy link

With the patch but at least it loads the package.

👍 we'll tweak the jsDelivr behavior to support the first link in the next few days.

@MartinKolarik
Copy link

@bmeck
Copy link
Member

bmeck commented Mar 10, 2022

can we get confirmation to close this? It should be fixed now that the PR is closed and 17.7.0 is released. I'd note that the unpkg URL still fails due to referencing a URL that is a 404 though, node can't fix that one.

@yavorg
Copy link
Author

yavorg commented Mar 11, 2022

Confirmed fixed on 17.7.1. It will still fail due to the crypto.getRandomValues() issue, but module loading now works.

import { v4 as uuidv4 } from 'https://cdn.jsdelivr.net/npm/[email protected]/wrapper.mjs/+esm';
console.log(uuidv4());

@yavorg yavorg closed this as completed Mar 11, 2022
@sheunaluko
Copy link

Hi this is slightly unrelated but figured I would post it here in case anyone else finds this thread like I did. In my case I was getting the same error Unknown module format: null for URL for a completely different reason. The file I was importing had another file with the same name but different extension in the directory, so I guess node was confused which one to actually import. Deleting the other file solved the issue. This makes sense since I was importing the file without an extension in the import line -- thus it was ambiguous.

@brandonros
Copy link

import fetch from 'https://cdn.jsdelivr.net/npm/node-fetch/+esm'

const run = async () => {
...
}

run()
$ node --experimental-network-imports index.mjs 
(node:45992) ExperimentalWarning: Network Imports is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
node:internal/errors:490
    ErrorCaptureStackTrace(err);
    ^

Error [ERR_NETWORK_IMPORT_DISALLOWED]: import of 'node:http' by https://cdn.jsdelivr.net/npm/node-fetch/+esm is not supported: only relative and absolute specifiers are supported.
    at new NodeError (node:internal/errors:399:5)
    at checkIfDisallowedImport (node:internal/modules/esm/resolve:1026:13)
    at defaultResolve (node:internal/modules/esm/resolve:1124:23)
    at nextResolve (node:internal/modules/esm/loader:163:28)
    at ESMLoader.resolve (node:internal/modules/esm/loader:838:30)
    at ESMLoader.getModuleJob (node:internal/modules/esm/loader:424:18)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:77:40)
    at link (node:internal/modules/esm/module_job:76:36)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5) {
  code: 'ERR_NETWORK_IMPORT_DISALLOWED'
}

Slightly related.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. esm Issues and PRs related to the ECMAScript Modules implementation. module Issues and PRs related to the module subsystem.
Projects
None yet
Development

No branches or pull requests