Skip to content

Commit

Permalink
Only apply /-prefix handling to special URL-like specifiers
Browse files Browse the repository at this point in the history
Closes #166.
  • Loading branch information
domenic committed Oct 21, 2020
1 parent 04428ae commit 86b63aa
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 11 deletions.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
27 changes: 26 additions & 1 deletion reference-implementation/__tests__/json/url-specifiers.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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/"
}
}
}
}
12 changes: 7 additions & 5 deletions reference-implementation/lib/resolver.js
Original file line number Diff line number Diff line change
@@ -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);
Expand All @@ -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;
}
Expand All @@ -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) {
Expand All @@ -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}"`);
}
Expand Down
4 changes: 4 additions & 0 deletions reference-implementation/lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
17 changes: 12 additions & 5 deletions spec.bs
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,12 @@ Editor: Domenic Denicola, Google https://www.google.com/, d@domenic.me, https://
Abstract: Import maps allow web pages to control the behavior of JavaScript imports.
!Participate: <a href="https://github.com/WICG/import-maps">GitHub WICG/import-maps</a> (<a href="https://github.com/WICG/import-maps/issues/new">new issue</a>, <a href="https://github.com/WICG/import-maps/issues?state=open">open issues</a>)
!Commits: <a href="https://github.com/WICG/import-maps/commits/master/spec.bs">GitHub spec.bs commits</a>
Complain About: accidental-2119 yes, missing-example-ids yes
Complain About: accidental-2119 yes
Indent: 2
Default Biblio Status: current
Markup Shorthands: markdown yes
</pre>
<!-- TODO: add back `Complain About: missing-example-ids yes` when https://github.com/tabatkins/bikeshed/issues/1804 gets fixed. -->
<pre class="link-defaults">
spec: infra; type: dfn
text: string
Expand Down Expand Up @@ -397,25 +398,31 @@ To <dfn>register an import map</dfn> 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. <p class="note">At this point, the specifier was able to be turned in to a URL, but it wasn't remapped to anything by |importMap|.</p>
If |asURL| is not null, then return |asURL|.
1. Throw a {{TypeError}} indicating that |specifier| was a bare specifier, but was not remapped to anything by |importMap|.
</div>

<div algorithm>
To <dfn lt="resolve an imports match|resolving an imports match">resolve an imports match</dfn>, given a [=string=] |normalizedSpecifier| and a [=specifier map=] |specifierMap|:
To <dfn lt="resolve an imports match|resolving an imports match">resolve an imports match</dfn>, 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:
1. If |resolutionResult| is null, then throw a {{TypeError}} indicating that resolution of |specifierKey| was blocked by a null entry.
<p class="note">This will terminate the entire [=resolve a module specifier=] algorithm, without any further fallbacks.</p>
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.
<p class="note">This will terminate the entire [=resolve a module specifier=] algorithm, without any further fallbacks.</p>
1. Assert: |resolutionResult| is a [=URL=].
Expand Down

0 comments on commit 86b63aa

Please sign in to comment.