Commit 6806b2f9 authored by Hiroshige Hayashizaki's avatar Hiroshige Hayashizaki Committed by Commit Bot

[Import Maps] Implement scoping

This CL implements scoping of import maps:

- Parsing in ImportMap::Parse(), and
- Resolving in ImportMap::Resolve().

To use ResolveImportsMatch() and MatchPrefix() for resolving using scopes,
this CL makes them take |SpecifierMap| arguments instead of using |imports_|.

Sorting order of scope keys is fixed by
https://github.com/WICG/import-maps/pull/182
and new tests (which will pass after this CL) are added by
https://github.com/WICG/import-maps/pull/183

Bug: 927181
Change-Id: I778a74424ad0eded8af8bfcf8443764b52abfbd2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1838233Reviewed-by: default avatarKouhei Ueno <kouhei@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703402}
parent c5c35e62
......@@ -148,7 +148,8 @@ ImportMap* ImportMap::Parse(const Modulator& modulator,
if (!parsed) {
*error_to_rethrow =
modulator.CreateSyntaxError("Failed to parse import map: invalid JSON");
return MakeGarbageCollected<ImportMap>(modulator, SpecifierMap());
return MakeGarbageCollected<ImportMap>(modulator, SpecifierMap(),
ScopeType());
}
// <spec step="2">If parsed is not a map, then throw a TypeError indicating
......@@ -157,7 +158,8 @@ ImportMap* ImportMap::Parse(const Modulator& modulator,
if (!parsed_map) {
*error_to_rethrow =
modulator.CreateTypeError("Failed to parse import map: not an object");
return MakeGarbageCollected<ImportMap>(modulator, SpecifierMap());
return MakeGarbageCollected<ImportMap>(modulator, SpecifierMap(),
ScopeType());
}
// <spec step="3">Let sortedAndNormalizedImports be an empty map.</spec>
......@@ -173,7 +175,8 @@ ImportMap* ImportMap::Parse(const Modulator& modulator,
*error_to_rethrow = modulator.CreateTypeError(
"Failed to parse import map: \"imports\" "
"top-level key must be a JSON object.");
return MakeGarbageCollected<ImportMap>(modulator, SpecifierMap());
return MakeGarbageCollected<ImportMap>(modulator, SpecifierMap(),
ScopeType());
}
// <spec step="4.2">Set sortedAndNormalizedImports to the result of sorting
......@@ -183,15 +186,95 @@ ImportMap* ImportMap::Parse(const Modulator& modulator,
imports, base_url, support_builtin_modules, logger);
}
// TODO(crbug.com/927181): Process "scopes" entry (Steps 5 and 6).
// <spec step="5">Let sortedAndNormalizedScopes be an empty map.</spec>
ScopeType sorted_and_normalized_scopes;
// <spec step="6">If parsed["scopes"] exists, then:</spec>
if (parsed_map->Get("scopes")) {
// <spec step="6.1">If parsed["scopes"] is not a map, then throw a TypeError
// indicating that the "scopes" top-level key must be a JSON object.</spec>
JSONObject* scopes = parsed_map->GetJSONObject("scopes");
if (!scopes) {
*error_to_rethrow = modulator.CreateTypeError(
"Failed to parse import map: \"scopes\" "
"top-level key must be a JSON object.");
return MakeGarbageCollected<ImportMap>(modulator, SpecifierMap(),
ScopeType());
}
// <spec step="6.2">Set sortedAndNormalizedScopes to the result of sorting
// and normalizing scopes given parsed["scopes"] and baseURL.</spec>
// <specdef
// href="https://wicg.github.io/import-maps/#sort-and-normalize-scopes">
// <spec step="1">Let normalized be an empty map.</spec>
ScopeType normalized;
// <spec step="2">For each scopePrefix -> potentialSpecifierMap of
// originalMap,</spec>
for (wtf_size_t i = 0; i < scopes->size(); ++i) {
const JSONObject::Entry& entry = scopes->at(i);
JSONObject* specifier_map = scopes->GetJSONObject(entry.first);
if (!specifier_map) {
// <spec step="2.1">If potentialSpecifierMap is not a map, then throw a
// TypeError indicating that the value of the scope with prefix
// scopePrefix must be a JSON object.</spec>
*error_to_rethrow = modulator.CreateTypeError(
"Failed to parse import map: the value of the scope with prefix "
"\"" +
entry.first + "\" must be a JSON object.");
return MakeGarbageCollected<ImportMap>(modulator, SpecifierMap(),
ScopeType());
}
// <spec step="2.2">Let scopePrefixURL be the result of parsing
// scopePrefix with baseURL as the base URL.</spec>
const KURL prefix_url(base_url, entry.first);
// <spec step="2.3">If scopePrefixURL is failure, then:</spec>
if (!prefix_url.IsValid()) {
// <spec step="2.3.1">Report a warning to the console that the scope
// prefix URL was not parseable.</spec>
logger.AddConsoleMessage(
mojom::ConsoleMessageSource::kOther,
mojom::ConsoleMessageLevel::kWarning,
"Ignored scope \"" + entry.first + "\": not parsable as a URL.");
// <spec step="2.3.2">Continue.</spec>
continue;
}
// <spec step="2.5">Let normalizedScopePrefix be the serialization of
// scopePrefixURL.</spec>
//
// <spec step="2.6">Set normalized[normalizedScopePrefix] to the result of
// sorting and normalizing a specifier map given potentialSpecifierMap and
// baseURL.</spec>
sorted_and_normalized_scopes.push_back(std::make_pair(
prefix_url.GetString(),
SortAndNormalizeSpecifierMap(specifier_map, base_url,
support_builtin_modules, logger)));
}
// <spec step="3">Return the result of sorting normalized, with an entry a
// being less than an entry b if b's key is code unit less than a's
// key.</spec>
std::sort(sorted_and_normalized_scopes.begin(),
sorted_and_normalized_scopes.end(),
[](const ScopeEntryType& a, const ScopeEntryType& b) {
return CodeUnitCompareLessThan(b.first, a.first);
});
}
// TODO(hiroshige): Implement Step 7.
// <spec step="8">Return the import map whose imports are
// sortedAndNormalizedImports and whose scopes scopes are
// sortedAndNormalizedScopes.</spec>
return MakeGarbageCollected<ImportMap>(modulator,
sorted_and_normalized_imports);
return MakeGarbageCollected<ImportMap>(
modulator, std::move(sorted_and_normalized_imports),
std::move(sorted_and_normalized_scopes));
}
// <specdef
......@@ -321,7 +404,8 @@ ImportMap::SpecifierMap ImportMap::SortAndNormalizeSpecifierMap(
// <specdef href="https://wicg.github.io/import-maps/#resolve-an-imports-match">
base::Optional<ImportMap::MatchResult> ImportMap::MatchPrefix(
const ParsedSpecifier& parsed_specifier) const {
const ParsedSpecifier& parsed_specifier,
const SpecifierMap& specifier_map) const {
// Do not perform prefix match for non-bare specifiers.
if (parsed_specifier.GetType() != ParsedSpecifier::Type::kBare)
return base::nullopt;
......@@ -341,7 +425,7 @@ base::Optional<ImportMap::MatchResult> ImportMap::MatchPrefix(
base::Optional<MatchResult> best_match;
// <spec step="1">For each specifierKey → addresses of specifierMap,</spec>
for (auto it = imports_.begin(); it != imports_.end(); ++it) {
for (auto it = specifier_map.begin(); it != specifier_map.end(); ++it) {
// <spec step="1.2">If specifierKey ends with U+002F (/) and
// normalizedSpecifier starts with specifierKey, then:</spec>
if (!it->key.EndsWith('/'))
......@@ -362,25 +446,63 @@ base::Optional<ImportMap::MatchResult> ImportMap::MatchPrefix(
}
ImportMap::ImportMap(const Modulator& modulator_for_built_in_modules,
const HashMap<String, Vector<KURL>>& imports)
: imports_(imports),
SpecifierMap&& imports,
ScopeType&& scopes)
: imports_(std::move(imports)),
scopes_(std::move(scopes)),
modulator_for_built_in_modules_(&modulator_for_built_in_modules) {}
// <specdef
// href="https://wicg.github.io/import-maps/#resolve-a-module-specifier">
base::Optional<KURL> ImportMap::Resolve(const ParsedSpecifier& parsed_specifier,
const KURL& base_url,
String* debug_message) const {
DCHECK(debug_message);
// <spec step="8">For each scopePrefix -> scopeImports of importMap's
// scopes,</spec>
for (const auto& entry : scopes_) {
// <spec step="8.1">If scopePrefix is baseURLString, or if scopePrefix ends
// with U+002F (/) and baseURLString starts with scopePrefix, then:</spec>
if (entry.first == base_url.GetString() ||
(entry.first.EndsWith("/") &&
base_url.GetString().StartsWith(entry.first))) {
// <spec step="8.1.1">Let scopeImportsMatch be the result of resolving an
// imports match given normalizedSpecifier and scopeImports.</spec>
base::Optional<KURL> scope_match =
ResolveImportsMatch(parsed_specifier, entry.second, debug_message);
// <spec step="8.1.2">If scopeImportsMatch is not null, then return
// scopeImportsMatch.</spec>
if (scope_match)
return scope_match;
}
}
// <spec step="9">Let topLevelImportsMatch be the result of resolving an
// imports match given normalizedSpecifier and importMap’s imports.</spec>
//
// <spec step="10">If topLevelImportsMatch is not null, then return
// topLevelImportsMatch.</spec>
return ResolveImportsMatch(parsed_specifier, imports_, debug_message);
}
// <specdef href="https://wicg.github.io/import-maps/#resolve-an-imports-match">
base::Optional<KURL> ImportMap::ResolveImportsMatch(
const ParsedSpecifier& parsed_specifier,
const SpecifierMap& specifier_map,
String* debug_message) const {
DCHECK(debug_message);
const String key = parsed_specifier.GetImportMapKeyString();
// <spec step="1.1">If specifierKey is normalizedSpecifier, then:</spec>
MatchResult exact = imports_.find(key);
if (exact != imports_.end()) {
MatchResult exact = specifier_map.find(key);
if (exact != specifier_map.end()) {
return ResolveImportsMatchInternal(key, exact, debug_message);
}
// Step 1.2.
if (auto prefix_match = MatchPrefix(parsed_specifier)) {
if (auto prefix_match = MatchPrefix(parsed_specifier, specifier_map)) {
return ResolveImportsMatchInternal(key, *prefix_match, debug_message);
}
......
......@@ -32,20 +32,6 @@ class CORE_EXPORT ImportMap final : public GarbageCollected<ImportMap> {
ConsoleLogger& logger,
ScriptValue* error_to_rethrow);
ImportMap(const Modulator&, const HashMap<String, Vector<KURL>>& imports);
// https://wicg.github.io/import-maps/#resolve-an-imports-match
// Returns nullopt when not mapped by |this| import map (i.e. the import map
// doesn't have corresponding keys).
// Returns a null URL when resolution fails.
base::Optional<KURL> ResolveImportsMatch(const ParsedSpecifier&,
String* debug_message) const;
String ToString() const;
void Trace(Visitor*);
private:
// <spec href="https://wicg.github.io/import-maps/#specifier-map">A specifier
// map is an ordered map from strings to lists of URLs.</spec>
//
......@@ -53,9 +39,33 @@ class CORE_EXPORT ImportMap final : public GarbageCollected<ImportMap> {
// are implemented differently from the spec.
using SpecifierMap = HashMap<String, Vector<KURL>>;
// <spec href="https://wicg.github.io/import-maps/#import-map-scopes">an
// ordered map of URLs to specifier maps.</spec>
using ScopeEntryType = std::pair<String, SpecifierMap>;
using ScopeType = Vector<ScopeEntryType>;
ImportMap(const Modulator&, SpecifierMap&& imports, ScopeType&& scopes);
base::Optional<KURL> Resolve(const ParsedSpecifier&,
const KURL& base_url,
String* debug_message) const;
String ToString() const;
void Trace(Visitor*);
private:
using MatchResult = SpecifierMap::const_iterator;
base::Optional<MatchResult> MatchPrefix(const ParsedSpecifier&) const;
// https://wicg.github.io/import-maps/#resolve-an-imports-match
// Returns nullopt when not mapped by |this| import map (i.e. the import map
// doesn't have corresponding keys).
// Returns a null URL when resolution fails.
base::Optional<KURL> ResolveImportsMatch(const ParsedSpecifier&,
const SpecifierMap&,
String* debug_message) const;
base::Optional<MatchResult> MatchPrefix(const ParsedSpecifier&,
const SpecifierMap&) const;
static SpecifierMap SortAndNormalizeSpecifierMap(const JSONObject* imports,
const KURL& base_url,
bool support_builtin_modules,
......@@ -69,8 +79,8 @@ class CORE_EXPORT ImportMap final : public GarbageCollected<ImportMap> {
// https://wicg.github.io/import-maps/#import-map-imports
SpecifierMap imports_;
// TODO(crbug.com/927181): Implement
// https://wicg.github.io/import-maps/#import-map-scopes.
ScopeType scopes_;
Member<const Modulator> modulator_for_built_in_modules_;
};
......
......@@ -192,8 +192,8 @@ KURL ModulatorImplBase::ResolveModuleSpecifier(const String& specifier,
base::Optional<KURL> mapped_url;
if (import_map_) {
String import_map_debug_message;
mapped_url = import_map_->ResolveImportsMatch(parsed_specifier,
&import_map_debug_message);
mapped_url = import_map_->Resolve(parsed_specifier, base_url,
&import_map_debug_message);
// Output the resolution log. This is too verbose to be always shown, but
// will be helpful for Web developers (and also Chromium developers) for
......
......@@ -2,12 +2,12 @@ This is a testharness.js-based test.
PASS Invalid JSON
PASS Mismatching the top-level schema / should throw for top-level non-objects
PASS Mismatching the top-level schema / should throw if imports is a non-object
FAIL Mismatching the top-level schema / should throw if scopes is a non-object assert_throws: function "() => parseFromString(input, baseURL)" did not throw
PASS Mismatching the top-level schema / should throw if scopes is a non-object
FAIL Mismatching the top-level schema / should ignore unspecified top-level entries assert_object_equals: expected property "0" missing
PASS Mismatching the specifier map schema / should ignore entries where the address is not a string, array, or null
PASS Mismatching the specifier map schema / should ignore entries where the specifier key is an empty string
PASS Mismatching the specifier map schema / should ignore members of an address array that are not strings
FAIL Mismatching the specifier map schema / should throw if a scope's value is not an object assert_throws: function "() => parseFromString(input, baseURL)" did not throw
PASS Mismatching the specifier map schema / should throw if a scope's value is not an object
PASS Normalization / should normalize empty import maps to have imports and scopes keys
PASS Normalization / should normalize an import map without imports to have imports
PASS Normalization / should normalize an import map without scopes to have scopes
......
This is a testharness.js-based test.
PASS Mapped using scope instead of "imports" / should fail when the mapping is to an empty array
FAIL Mapped using scope instead of "imports" / Exact vs. prefix based matching / should match correctly when both are in the map Failed to resolve module specifier moment: Relative references must start with either "/", "./", or "../".
FAIL Mapped using scope instead of "imports" / Exact vs. prefix based matching / should match correctly when only an exact match is in the map Failed to resolve module specifier moment: Relative references must start with either "/", "./", or "../".
FAIL Mapped using scope instead of "imports" / Exact vs. prefix based matching / should match correctly when only a prefix match is in the map Failed to resolve module specifier moment: Relative references must start with either "/", "./", or "../".
FAIL Mapped using scope instead of "imports" / Package-like scenarios / should resolve scoped assert_equals: expected "https://example.com/app/node_modules_2/lodash-es/lodash.js" but got "https://example.com/app/node_modules/lodash-es/lodash.js"
FAIL Mapped using scope instead of "imports" / Package-like scenarios / should apply best scope match assert_equals: expected "https://example.com/node_modules_3/moment/src/moment.js" but got "https://example.com/node_modules/moment/src/moment.js"
PASS Mapped using scope instead of "imports" / Package-like scenarios / should fallback to "imports"
PASS Mapped using scope instead of "imports" / Package-like scenarios / should still fail for package-like specifiers that are not declared
PASS Mapped using scope instead of "imports" / The scope inheritance example from the README / should fall back to "imports" when none match
FAIL Mapped using scope instead of "imports" / The scope inheritance example from the README / should use a direct scope override assert_equals: expected "https://example.com/a-2.mjs" but got "https://example.com/a-1.mjs"
FAIL Mapped using scope instead of "imports" / The scope inheritance example from the README / should use an indirect scope override assert_equals: expected "https://example.com/a-2.mjs" but got "https://example.com/a-1.mjs"
FAIL Mapped using scope instead of "imports" / Relative URL scope keys / should resolve an empty string scope using the import map URL assert_equals: expected "https://example.com/a-empty-string.mjs" but got "https://example.com/a-1.mjs"
FAIL Mapped using scope instead of "imports" / Relative URL scope keys / should resolve a ./ scope using the import map URL's directory assert_equals: expected "https://example.com/b-dot-slash.mjs" but got "https://example.com/b-1.mjs"
FAIL Mapped using scope instead of "imports" / Relative URL scope keys / should resolve a ../ scope using the import map URL's directory assert_equals: expected "https://example.com/c-dot-dot-slash.mjs" but got "https://example.com/c-1.mjs"
Harness: the test ran to completion.
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment