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

Don't handle impossible errors in HostResolveImportedModule #2625

Merged
merged 1 commit into from
May 12, 2017

Conversation

domenic
Copy link
Member

@domenic domenic commented May 4, 2017

Don't handle impossible errors in HostResolveImportedModule

Several of the cases that were handled as errors are actually impossible
to hit. Instead of throwing errors in these impossible situations, we
can just assert that they are impossible.

This also cleans up "resolve a module specifier" and the module map to
be clear that they operate on URL records, not on absolute URL strings.


Builds on top of #2604 (which itself builds on top of #2595), so should not be merged until those are. But that commit can be reviewed separately, and I guess if it gets reviewed first I can rebase things on top of it.

@GeorgNeis to review, as he found this issue.


Preview of this at https://dl.dropboxusercontent.com/u/20140634/modules-all-better/index.html

<li><p>Let <var>resolved module script</var> be <var>moduleMap</var>[<var>url</var>]. (This entry
must <span data-x="map exists">exist</span> for us to have gotten to this point.)</p></li>

<li>
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that depending on how we fix #2629 and #2630 this may no longer be possible, if we step away from our strategy of "store some error sentinel then just instantiate anyway to get the error to propagate". That strategy is seeming increasingly unwise.

Copy link

@GeorgNeis GeorgNeis left a comment

Choose a reason for hiding this comment

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

rubberstamp lgtm to unblock larger revisions

Several of the cases that were handled as errors are actually impossible
to hit. Instead of throwing errors in these impossible situations, we
can just assert that they are impossible.

This also cleans up "resolve a module specifier" and the module map to
be clear that they operate on URL records, not on absolute URL strings.
@domenic domenic force-pushed the remove-impossible-module-error-cases branch from 56b7bcb to 2a286f3 Compare May 12, 2017 16:55
@domenic domenic merged commit 616df18 into master May 12, 2017
@domenic domenic deleted the remove-impossible-module-error-cases branch May 12, 2017 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification Standard could be clearer topic: script
Development

Successfully merging this pull request may close these issues.

2 participants