search

WICG / import-maps

How to control the behavior of JavaScript imports
https://html.spec.whatwg.org/multipage/webappapis.html#import-maps
Other
2.66k stars 69 forks source link

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

Closed hiroshige-g closed 4 years ago

hiroshige-g commented 4 years ago

This PR all errors (found either at parsing or resolution time) throw errors at resolution.

Before this PR, errors found at parsing time, namely:

were ignored, allowing fallback to next candidates at resolution time, and thus can't be used as a definitive way to block particular resolution. After this PR, such entries are kept in specifier maps as entries with null value, and cause resolutions fail (without further fallback) by throwing errors. For thus purpose, this PR changes the type of values of specifier maps to URL or null.

Also, before this PR, relative URL resolution failures at prefix matching in #resolve-a-module-specifier didn't allow fallback to less specific prefixes while allowing fallback to less specific scopes. This PR makes such errors throw errors, disallowing fallback to less specific prefixes and scopes.

This PR unifies resolution failures into one: throwing an error. After this PR, all kind of errors throw at resolution time and definitively blocking resolution. Fallback to less specific scopes occur only when there are no matching entries in a scope.

Fixes #184.

Preview | Diff

hiroshige-g commented 4 years ago

I'll create an associated test update PR later (due to dependency to #204; drafted locally).

hiroshige-g commented 4 years ago

Added/updated ref impl and tests. Ready for review.

hiroshige-g commented 4 years ago

The reference implementation

should we align the spec with the reference implementation or the reference implementation with the spec.

One point I'm aware (and I was not sure) is Step 1.4-1.5 of #resolve-an-imports-match. In the spec, all control paths sets resolutionResult and then Step 1.4-1.5 handle resolutionResult in a centralized fashion, while in the ref impl each cases immediately throws etc.

Pros of ref impl style:

Cons of ref impl style:

Probably I should further refactor both the spec and implementation. I will try tomorow.

Or are there other diffs?

domenic commented 4 years ago

It's not clear that there are only three kind of resolution results for each entry

I don't think this is super clear even from the spec style. (The note attempts to clarify it, but as seen from our above discussion, the exact correspondence between those three cases and different steps in the algorithms is not 1:1.) So, I think I lean toward the reference implementation style.

hiroshige-g commented 4 years ago

+1 to the reference implementation style. I tried to refactor the spec to clarify the invariant, but it didn't turned successful (needed more plumbings, subalgorithms etc.).

Preparing a commit...

hiroshige-g commented 4 years ago

Pushed. WDYT?

hiroshige-g commented 4 years ago

Looks good!