feat: Add support for import assertions and JSON modules

[bartlomieju]

Closes #7623

This commit adds proper support for import assertions and JSON modules.

Implementation of "core/modules.rs" was changed to account for multiple possible
module types, instead of always assuming that the code is an "ES module". In
effect "ModuleMap" now has knowledge about each modules' type (stored via
"ModuleType" enum). Module loading pipeline now stores information about
expected module type for each request and validates that expected type matches
discovered module type based on file's "MediaType".

Relevant tests were added to "core/modules.rs" and integration tests,
additionally multiple WPT tests were enabled.

There are still some rough edges in the implementation and not all WPT were
enabled, due to:
a) unclear BOM handling in source code by "FileFetcher"
b) design limitation of Deno's "FileFetcher" that doesn't download the same
module multiple times in a single run

[dsherret]

Probably will need an swc upgrade once this is fixed swc-project/swc#2802

Edit: Not relevant

[bartlomieju]

Blocked by #12989

[lucacasonato]

I can not get this to work:

import x from "./.dlint.json" asserts { "type": "json" };

$ cargo run --example fs_module_loader test.js
Run test.js
cargo:rerun-if-changed=/mnt/starship/Projects/github.com/denoland/deno/core/00_primordials.js
cargo:rerun-if-changed=/mnt/starship/Projects/github.com/denoland/deno/core/01_core.js
cargo:rerun-if-changed=/mnt/starship/Projects/github.com/denoland/deno/core/02_error.js
Error: Uncaught SyntaxError: Unexpected identifier
    at file:///mnt/starship/Projects/github.com/denoland/deno/test.js:1:30

[bartlomieju]

I have trouble getting

"UTF-16BE BOM should result in parse error in JSON module script",
"UTF-16LE BOM should result in parse error in JSON module script"

to pass.

When I test it locally the proper error is returned, but somehow during WPT it breaks on some utility like dprint and surfaces TypeError instead of SyntaxError.

To be precise I'm getting:

promise_rejects_js: Expected parse error from UTF-16BE BOM function "function() { throw e }" threw object "TypeError: invalid utf-8 sequence of 1 bytes from index 0" ("TypeError") expected instance of function "function SyntaxError() { [native code] }" ("SyntaxError")
    at async Test.<anonymous> (http://web-platform.test:8000/html/semantics/scripting-1/the-script-element/json-module/charset-bom.any.js:10:5)

However running a local file that imports JSON with BOM I get:

error: Uncaught (in promise) SyntaxError: Unexpected token � in JSON at position 0
const result = await import("./data.json", { assert: { type: "json" } });
               ^
    at async file:///Users/biwanczuk/dev/deno/test.js:1:16

[lucacasonato]

Looking good!

[andreubotella]

The UTF-16 TypeError seems to be a std::string::FromUtf8Error thrown when get_source_from_bytes() is called with None as charset. If called with Some("utf-8"), the conversion would replace invalid UTF-8 with replacement characters, which would give the correct SyntaxError.

Edit: That last sentence is wrong, I completely misread the code 😅

[dsherret]

Looks really good! Will be really nice to have this.

[dsherret]

Nitpick: Seems like something that could be extracted out into a function to help reduce the detail here.

[bartlomieju]

This is only used in a single place, and the whole function seems like a code smell, I'd rather address this in separate PR that would cleanup modules implementation

[dsherret]

Is this something to be fixed in this review or later?

[bartlomieju]

Later, it's not clear how to fix it

[bartlomieju]

The UTF-16 TypeError seems to be a std::string::FromUtf8Error thrown when get_source_from_bytes() is called with None as charset. If called with Some("utf-8"), the conversion would replace invalid UTF-8 with replacement characters, which would give the correct SyntaxError.

Edit: That last sentence is wrong, I completely misread the code 😅

Thanks for pointing this out, it's not clear to me what would need to change to fix it. @dsherret any suggestions?

[bartlomieju]

Due to the fact that Deno caches HTTP responses on first use, I believe we can't enable "Two modules of different type with the same specifier can load if the server changes its responses" test; as it changes the response's content type on second fetch.

[andreubotella]

So according to the HTML spec, the charset declared on the MIME type, or the file's BOM, don't really matter. Instead, all modules –JS, JSON or CSS– are decoded with the Encoding standard's "UTF-8 decode" algorithm (see "fetch a single module script", step 13), which is equivalent to:

let bytes = if bytes.starts_with(&[0xEF, 0xBB, 0xBF]) {
    &bytes[3..]
} else {
    bytes
};
String::from_utf8_lossy(bytes)

I suspect windows-1252 and perhaps UTF-16 might be important for local files in Windows, so we might need to go against the spec there, but the WPT tests expect the JSON parsing to fail, not the UTF-8 decoding.

[bartlomieju]

@andreubotella seems like a niche case - we still fail as expected, just with wrong error type. I'd prefer to address this in a follow up PR.

[andreubotella]

@andreubotella seems like a niche case - we still fail as expected, just with wrong error type. I'd prefer to address this in a follow up PR.

Fair enough

[dsherret]

LGTM