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.

Sign up for GitHub

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

Make any errors (invalid URLs etc) throw errors in resolution #205

Merged
merged 9 commits into from
Feb 7, 2020
Merged

Make any errors (invalid URLs etc) throw errors in resolution #205

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
884b023
Make any errors (invalid URLs etc) fail the whole resolution
hiroshige-g Jan 29, 2020
e522f3d
Use null instead of Block for consistency with parsing tests.
hiroshige-g Jan 29, 2020
26d9748
Update reference impelementation and add/update tests.
hiroshige-g Jan 28, 2020
6842e2c
Fix lint errors
hiroshige-g Jan 29, 2020
0df6a90
Tweaks
domenic Jan 31, 2020
248cfcc
Remove no-longer-needed comment
domenic Jan 31, 2020
6b5aae6
Align the structures and names of spec and ref impl
hiroshige-g Feb 3, 2020
7002919
Lint fix
hiroshige-g Feb 3, 2020
630066c
Further align ref impl and spec
domenic Feb 6, 2020
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
25 changes: 11 additions & 14 deletions spec.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,13 +371,11 @@ To <dfn>register an import map</dfn> given an {{HTMLScriptElement}} |element|:
<h2 id="resolving">Resolving module specifiers</h2>

<div class="note">
During [=resolve a module specifier|resolving a module specifier=], the following algorithms check candidate entries of [=specifier maps=], from most-specific to least-specific scopes and to top-level "`imports`", from most-specific to least-specific prefixes.
For each entry, the result is one of the following:
During [=resolve a module specifier|resolving a module specifier=], the following algorithms check candidate entries of [=specifier maps=], from most-specific to least-specific scopes (falling back to top-level "`imports`"), and from most-specific to least-specific prefixes. For each candidate, the result is one of the following:

- Successfully resolves a specifier to a [=URL=].
This makes the [=resolve a module specifier=] algorithm return immediately successfully with the [=URL=].
- Throws an error.
This makes the [=resolve a module specifier=] algorithm fail immediately, i.e. throw the error, without any further fallbacks.
- Successfully resolves a specifier to a [=URL=]. This makes the [=resolve a module specifier=] algorithm immediately return that [=URL=].
- Throws an error. This makes the [=resolve a module specifier=] algorithm rethrow the error, without any further fallbacks.
- Fail to resolve, without an error. In this case the algorithm moves on to the next candidate.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

nit: I'm wondering better labels...
IIUC the third case corresponds to Step 1.3 of #resolve-an-imports-match, right?
"Throws an error" and "Fail to resolve" sound similarly and thus the difference between them might be unclear.
Alternatives I came up with are:

  • "Throws an error" and "Continue"
  • "Fail to resolve, throwing an error" and "Fail to resolve, without throwing an error"
    (Step 1.3 is just handling non-matching specifier keys, which doesn't look like failing something though)

Any idea?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I was thinking it it corresponds to both step 1.3 and step 2.

Hmm maybe

  • Resolves the specifier to null. This terminates the resolve a module specifier algorithm with a thrown exception.
  • Does not resolve the specifier. In this case the algorithm moves on to the next candidate.

?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

"Resolves the specifier to URL", "Resolves the specifier to null" and "Does not resolve the specifier" looked great to me in terms of symmetry, but on second thought "Resolves to null" might be confusing with "Return null" in resolve an imports match (which is actually means "Does not resolve"), and might be a little inconsistent with resolve a module specifier interface (which throws exceptions rather than returning null).

"Does not resolve the specifier" still looks good to me, so how about something like:

  • "Resolves the specifier to URL", "Throws an error" and "Does not resolve the specifier"
  • "Resolves the specifier to URL", "Resolves the specifier to an error" and "Does not resolve the specifier"

?

</div>

<h3 id="new-resolve-algorithm">New "resolve a module specifier"</h3>
Expand Down Expand Up @@ -408,22 +406,21 @@ For each entry, the result is one of the following:
<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|:


1. For each |specifierKey| → |value| of |specifierMap|,
1. If |specifierKey| is |normalizedSpecifier|, then set |resolutionResult| to |value|.
1. Else if |specifierKey| ends with U+002F (/) and |normalizedSpecifier| [=/starts with=] |specifierKey|, then:
1. Otherwise, if |specifierKey| ends with U+002F (/) and |normalizedSpecifier| [=/starts with=] |specifierKey|, then:
1. If |value| is a [=URL=]:
1. Let |afterPrefix| be the portion of |normalizedSpecifier| after the initial |specifierKey| prefix.
1. Assert: |afterPrefix| ends with "`/`", as enforced during [=parse an import map string|parsing=].
1. Let |url| be the result of [=URL parser|parsing=] |afterPrefix| relative to the base URL |value|.
1. If |url| is failure, then set |resolutionResult| to null. Otherwise, set |resolutionResult| to |url|.
1. Otherwise, set |resolutionResult| to |value|.
1. Else, continue.
1. If |resolutionResult| is an [=URL=], then return |resolutionResult|.
1. Otherwise (i.e. |resolutionResult| is null), then throw a {{TypeError}} indicating that resolution of |specifierKey| was blocked by an entry in an import map.
<p class="note">This terminates the entire [=resolve a module specifier=] algorithm, without any further fallbacks.</p>
1. Otherwise (i.e., if |value| is null), set |resolutionResult| to null.
1. Otherwise, [=continue=].
1. If |resolutionResult| is a [=URL=], then return |resolutionResult|.
1. Otherwise (i.e. |resolutionResult| is null), throw a {{TypeError}} indicating that resolution of |specifierKey| was blocked by an entry in an import map.
<p class="note">This will terminate the entire [=resolve a module specifier=] algorithm, without any further fallbacks.</p>
1. Return null.
<p class="note">The caller will fallback to less specific scopes or to "`imports`", if any.</p>
<p class="note">The [=resolve a module specifier=] algorithm will fallback to a less specific scope or to "`imports`", if possible.</p>
</div>

<h3 id="resolving-updates">Updates to other algorithms</h3>
Expand Down