From cd5c8da19f32e25f9f032b0bebc659c5476f5e50 Mon Sep 17 00:00:00 2001 From: Domenic Denicola Date: Wed, 21 Oct 2020 18:17:29 -0400 Subject: [PATCH] Only apply /-prefix handling to special URL-like specifiers Closes #166. --- README.md | 2 ++ .../__tests__/json/url-specifiers.json | 27 ++++++++++++++++++- reference-implementation/lib/resolver.js | 12 +++++---- reference-implementation/lib/utils.js | 4 +++ spec.bs | 14 +++++++--- 5 files changed, 49 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 01566d0..bf7e1d4 100644 --- a/README.md +++ b/README.md @@ -219,6 +219,8 @@ This version ensures that import statements for specifiers that start with the s In general, the point is that the remapping works the same for URL-like imports as for bare imports. Our previous examples changed the resolution of specifiers like `"lodash"`, and thus changed the meaning of `import "lodash"`. Here we're changing the resolution of specifiers like `"/app/helpers.mjs"`, and thus changing the meaning of `import "/app/helpers.mjs"`. +Note that this trailing-slash variant of URL-like specifier mapping only works if the URL-like specifier has a [special scheme](https://url.spec.whatwg.org/#special-scheme): e.g., a mapping of `"data:text/": "/foo"` will not impact the meaning of `import "data:text/javascript,console.log('test')"`, but instead will only impact `import "data:text/"`. + #### Mapping away hashes in script filenames Script files are often given a unique hash in their filename, to improve cachability. See [this general discussion of the technique](https://jakearchibald.com/2016/caching-best-practices/), or [this more JavaScript- and webpack-focused discussion](https://developers.google.com/web/fundamentals/performance/webpack/use-long-term-caching). diff --git a/reference-implementation/__tests__/json/url-specifiers.json b/reference-implementation/__tests__/json/url-specifiers.json index aff55c4..8793b2c 100644 --- a/reference-implementation/__tests__/json/url-specifiers.json +++ b/reference-implementation/__tests__/json/url-specifiers.json @@ -9,7 +9,14 @@ "/test/": "/lib/url-trailing-slash/", "./test/": "/lib/url-trailing-slash-dot/", "/test": "/lib/test1.mjs", - "../test": "/lib/test2.mjs" + "../test": "/lib/test2.mjs", + "data:text/": "/lib/test-data/", + "about:text/": "/lib/test-about/", + "blob:text/": "/lib/test-blob/", + "blah:text/": "/lib/test-blah/", + "file:text/": "/lib/test-file/", + "ftp:text/": "/lib/test-ftp/", + "ws:text/": "/lib/test-ws/" } }, "importMapBaseURL": "https://example.com/app/index.html", @@ -47,6 +54,24 @@ "expectedResults": { "/test": "https://example.com/lib/test2.mjs" } + }, + "Non-special vs. special schemes": { + "expectedResults": { + "data:text/javascript,console.log('foo')": "data:text/javascript,console.log('foo')", + "data:text/": "https://example.com/lib/test-data/", + "about:text/foo": "about:text/foo", + "about:text/": "https://example.com/lib/test-about/", + "blob:text/foo": "blob:text/foo", + "blob:text/": "https://example.com/lib/test-blob/", + "blah:text/foo": "blah:text/foo", + "blah:text/": "https://example.com/lib/test-blah/", + "ftp:text/foo": "https://example.com/lib/test-ftp/foo", + "ftp:text/": "https://example.com/lib/test-ftp/", + "file:text/foo": "https://example.com/lib/test-file/foo", + "file:text/": "https://example.com/lib/test-file/", + "ws:text/foo": "https://example.com/lib/test-ws/foo", + "ws:text/": "https://example.com/lib/test-ws/" + } } } } diff --git a/reference-implementation/lib/resolver.js b/reference-implementation/lib/resolver.js index 81b635c..ac113c6 100644 --- a/reference-implementation/lib/resolver.js +++ b/reference-implementation/lib/resolver.js @@ -1,6 +1,6 @@ 'use strict'; const assert = require('assert'); -const { tryURLLikeSpecifierParse, tryURLParse } = require('./utils.js'); +const { tryURLLikeSpecifierParse, tryURLParse, isSpecial } = require('./utils.js'); exports.resolve = (specifier, parsedImportMap, scriptURL) => { const asURL = tryURLLikeSpecifierParse(specifier, scriptURL); @@ -10,14 +10,14 @@ exports.resolve = (specifier, parsedImportMap, scriptURL) => { for (const [scopePrefix, scopeImports] of Object.entries(parsedImportMap.scopes)) { if (scopePrefix === scriptURLString || (scopePrefix.endsWith('/') && scriptURLString.startsWith(scopePrefix))) { - const scopeImportsMatch = resolveImportsMatch(normalizedSpecifier, scopeImports); + const scopeImportsMatch = resolveImportsMatch(normalizedSpecifier, asURL, scopeImports); if (scopeImportsMatch !== null) { return scopeImportsMatch; } } } - const topLevelImportsMatch = resolveImportsMatch(normalizedSpecifier, parsedImportMap.imports); + const topLevelImportsMatch = resolveImportsMatch(normalizedSpecifier, asURL, parsedImportMap.imports); if (topLevelImportsMatch !== null) { return topLevelImportsMatch; } @@ -30,7 +30,7 @@ exports.resolve = (specifier, parsedImportMap, scriptURL) => { throw new TypeError(`Unmapped bare specifier "${specifier}"`); }; -function resolveImportsMatch(normalizedSpecifier, specifierMap) { +function resolveImportsMatch(normalizedSpecifier, asURL, specifierMap) { for (const [specifierKey, resolutionResult] of Object.entries(specifierMap)) { // Exact-match case if (specifierKey === normalizedSpecifier) { @@ -44,7 +44,9 @@ function resolveImportsMatch(normalizedSpecifier, specifierMap) { } // Package prefix-match case - if (specifierKey.endsWith('/') && normalizedSpecifier.startsWith(specifierKey)) { + if (specifierKey.endsWith('/') && + normalizedSpecifier.startsWith(specifierKey) && + (!asURL || isSpecial(asURL))) { if (resolutionResult === null) { throw new TypeError(`Blocked by a null entry for "${specifierKey}"`); } diff --git a/reference-implementation/lib/utils.js b/reference-implementation/lib/utils.js index 6f04e5c..515395c 100644 --- a/reference-implementation/lib/utils.js +++ b/reference-implementation/lib/utils.js @@ -17,3 +17,7 @@ exports.tryURLLikeSpecifierParse = (specifier, baseURL) => { const url = exports.tryURLParse(specifier); return url; }; + +// https://url.spec.whatwg.org/#special-scheme +const specialProtocols = new Set(["ftp:", "file:", "http:", "https:", "ws:", "wss:"]); +exports.isSpecial = url => specialProtocols.has(url.protocol); diff --git a/spec.bs b/spec.bs index 770c902..c15c680 100644 --- a/spec.bs +++ b/spec.bs @@ -398,9 +398,9 @@ To register an import map given an {{HTMLScriptElement}} |element|: 1. Let |normalizedSpecifier| be the [=URL serializer|serialization=] of |asURL|, if |asURL| is non-null; otherwise, |specifier|. 1. [=map/For each=] |scopePrefix| → |scopeImports| of |importMap|'s [=import map/scopes=], 1. If |scopePrefix| is |baseURLString|, or if |scopePrefix| ends with U+002F (/) and |baseURLString| [=string/starts with=] |scopePrefix|, then: - 1. Let |scopeImportsMatch| be the result of [=resolving an imports match=] given |normalizedSpecifier| and |scopeImports|. + 1. Let |scopeImportsMatch| be the result of [=resolving an imports match=] given |normalizedSpecifier|, |asURL|, and |scopeImports|. 1. If |scopeImportsMatch| is not null, then return |scopeImportsMatch|. - 1. Let |topLevelImportsMatch| be the result of [=resolving an imports match=] given |normalizedSpecifier| and |importMap|'s [=import map/imports=]. + 1. Let |topLevelImportsMatch| be the result of [=resolving an imports match=] given |normalizedSpecifier|, |asURL|, and |importMap|'s [=import map/imports=]. 1. If |topLevelImportsMatch| is not null, then return |topLevelImportsMatch|. 1.

At this point, the specifier was able to be turned in to a URL, but it wasn't remapped to anything by |importMap|.

If |asURL| is not null, then return |asURL|. @@ -408,7 +408,7 @@ To register an import map given an {{HTMLScriptElement}} |element|:
- To resolve an imports match, given a [=string=] |normalizedSpecifier| and a [=specifier map=] |specifierMap|: + To resolve an imports match, given a [=string=] |normalizedSpecifier|, a [=URL=] or null |asURL|, and a [=specifier map=] |specifierMap|: 1. For each |specifierKey| → |resolutionResult| of |specifierMap|, 1. If |specifierKey| is |normalizedSpecifier|, then: @@ -416,7 +416,13 @@ To register an import map given an {{HTMLScriptElement}} |element|:

This will terminate the entire [=resolve a module specifier=] algorithm, without any further fallbacks.

1. Assert: |resolutionResult| is a [=URL=]. 1. Return |resolutionResult|. - 1. If |specifierKey| ends with U+002F (/) and |normalizedSpecifier| [=string/starts with=] |specifierKey|, then: + 1. If all of the following are true: + + * |specifierKey| ends with U+002F (/), + * |normalizedSpecifier| [=string/starts with=] |specifierKey|, and + * either |asURL| is null, or |asURL| [=is special=] + + then: 1. If |resolutionResult| is null, then throw a {{TypeError}} indicating that resolution of |specifierKey| was blocked by a null entry.

This will terminate the entire [=resolve a module specifier=] algorithm, without any further fallbacks.

1. Assert: |resolutionResult| is a [=URL=].