Closed nicolo-ribaudo closed 1 year ago
This has been fixed.
I'm reopening this because I noticed that this change caused an assertion in HTML to be invalid (step 9 of https://html.spec.whatwg.org/#creating-a-javascript-module-script).
Right now, ignoring that assertion, when loading a module this is what happens:
type
attribute, if any (this might throw)We have two options:
I'm implementing this in chromium, and the complexity of doing 1 vs 2 is the same. I have a slight preference for duplicating the check, to align it with modules URL resolution. For dynamic import, currently the check for invalid attribute keys happens before trying to resolve the URL (because it happens in ECMA-262 before calling in HTML).
cc @domenic
Doing (2) also has the benefit of aligning static and dynamic imports.
Given this code:
await Promise.all([
import("https://example.com/x", { with: { type: "foo" } }).catch(e => e.message),
import("x", { with: { type: "foo" } }).catch(e => e.message),
import("https://example.com/x", { with: { invalid: "foo" } }).catch(e => e.message),
import("x", { with: { invalid: "foo" } }).catch(e => e.message),
import('data:text/javascript, import "https://example.com/x" with { type: "foo" }').catch(e => e.message),
import('data:text/javascript, import "x" with { type: "foo" }').catch(e => e.message),
import('data:text/javascript, import "https://example.com/x" with { invalid: "foo" }').catch(e => e.message),
import('data:text/javascript, import "x" with { invalid: "foo" }').catch(e => e.message),
]);
with (1) it would print
[
"\"foo\" is not a valid module type.",
"Failed to resolve module specifier 'x'",
"Invalid attribute key \"invalid\".",
"Invalid attribute key \"invalid\".",
"\"foo\" is not a valid module type.",
"Failed to resolve module specifier \"x\".",
"Invalid attribute key \"invalid\".",
"Failed to resolve module specifier \"x\".",
]
while with (2) it would print
[
"\"foo\" is not a valid module type.",
"Failed to resolve module specifier 'x'",
"Invalid attribute key \"invalid\".",
"Invalid attribute key \"invalid\".",
"\"foo\" is not a valid module type.",
"Failed to resolve module specifier \"x\".",
"Invalid attribute key \"invalid\".",
"Invalid attribute key \"invalid\"."
]
I don't think I have strong feelings here but all else being equal, aligning dynamic and static imports seems nice.
The assert in step 9 does seem pretty inconsequential to me so removing it from HTML doesn't seem like a big deal.
Should parse successfully, and fail trying to load the imported module.
This is not currently observable within ECMA-262, but it affects how errors are reported for code that contains an actual syntax error:
Reporting the parsing error at the attribute rather than at the missing semicolon after
a
is very awkward.