Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only apply /-prefix handling to special URL-like specifiers #227

Merged
merged 2 commits into from
Oct 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
{
"importMap": {
"imports": {
"data:text/": "/lib/test-data/",
"about:text/": "/lib/test-about/",
"blob:text/": "/lib/test-blob/",
"blah:text/": "/lib/test-blah/",
"http:text/": "/lib/test-http/",
"https:text/": "/lib/test-https/",
"file:text/": "/lib/test-file/",
"ftp:text/": "/lib/test-ftp/",
"ws:text/": "/lib/test-ws/",
"wss:text/": "/lib/test-wss/"
}
},
"importMapBaseURL": "https://example.com/app/index.html",
"baseURL": "https://example.com/js/app.mjs",
"name": "URL-like specifiers",
"tests": {
"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/",
"http:text/foo": "https://example.com/lib/test-http/foo",
"http:text/": "https://example.com/lib/test-http/",
"https:text/foo": "https://example.com/lib/test-https/foo",
"https:text/": "https://example.com/lib/test-https/",
"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/",
"wss:text/foo": "https://example.com/lib/test-wss/foo",
"wss:text/": "https://example.com/lib/test-wss/"
}
}
}
}
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);
14 changes: 10 additions & 4 deletions spec.bs
Original file line number Diff line number Diff line change
Expand Up @@ -398,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=]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When would asURL be null here if the invariant is that it is already normalized?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

asURL is null if normalizedSpecifier is something like "moment".


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