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

Package name map load blocking #48

Closed
guybedford opened this issue Aug 1, 2018 · 10 comments
Closed

Package name map load blocking #48

guybedford opened this issue Aug 1, 2018 · 10 comments

Comments

@guybedford
Copy link
Collaborator

This just came up from the discussion around whatwg/html#3871.

If I have the following:

<script type="packagemap" src="package-map.json"></script>
<script type="module">
  console.log('Exec first module script');
</script>

<script type="module">
  // (where x.js has no dependencies itself)
  import "./x.js";
  console.log('Exec second module script');
</script>

<script type="module">
  import "x";
  console.log('Exec third module script');
</script>

Which module script executions will be blocked by the package name map load?

All of them? Or just the last one?

@matthewp
Copy link

matthewp commented Aug 1, 2018

I would expect only the last one as packagemaps only apply to bare specifiers. If the scripts were ordered the other way, with the relative importing script was on the bottom, then I would expect both to be blocked.

@guybedford
Copy link
Collaborator Author

I believe the current implementation intent is for all to be blocked.

@hiroshige-g
Copy link
Collaborator

@domenic @nyaxt and I have discussed this previously, and our current plan is to block all of subsequent module script loading, i.e. all three module scripts are blocked, to simplify spec and implementation.

We've also considered the option of only blocking modules with bare specifiers.
But this would add more fine-grained blocking semantics and timing dependencies around module loading, and thus we don't want to do this unless there are strong needs.

@ljharb
Copy link

ljharb commented Aug 2, 2018

What happens if the first one has import() using a bare specifier? You wouldn't be able to determine that statically, so the only safe answer is always to block on all of them.

@hiroshige-g
Copy link
Collaborator

In the alternative plan of blocking only modules with bare specifiers, the first module script would be anyway loaded (at this time dynamic import()s are not processed), and when the module script is evaluated, the promise returned by import() would wait for the package name map. In other words, instead of entire module tree loading, each individual resolution (either static import statement or dynamic import()) is blocked if it is bare, by the pending package map.

@ljharb
Copy link

ljharb commented Aug 2, 2018

That does seem like a reasonable optimization that would effectively/observably be the same as "blocking on all".

@domenic
Copy link
Collaborator

domenic commented Aug 2, 2018

As @hiroshige-g points out, the plan here is to block. Any other strategy leads to bad inconsistencies, where depending on whether the network load has completed or not, scripts can fail or work or block. E.g. consider someone adding a script element dynamically, or someone doing import("./y") where ./y does import "x". The idea of it not being observable is just incorrect.

It adds a lot of complexity to the platform to have this interleaving, increases nondeterminism, and the gain is minimal: it only appears when someone is doing the already-not-performant thing of loading a package name map using src="" instead of inline. So, I don't plan to support this.

I'll leave this issue open until we clarify and document the conclusion here. As Hiroshige mentions, this is something we previously worked out as part of his implementation prototyping in Blink, but we do need to bring it back and document it in this proposal repository.

@guybedford
Copy link
Collaborator Author

Any other strategy leads to bad inconsistencies, where depending on whether the network load has completed or not, scripts can fail or work or block.

Where did fail suddenly come into this picture, as being a different outcome here than if package name maps didn't block?

it only appears when someone is doing the already-not-performant thing of loading a package name map using src="" instead of inline.

I was not aware that this was considered "already-not-performant", and in fact I would argue the reverse - a package name map that can be cached and reused while the main HTML page reloads will be more performant for larger sizes of package map, not less.

This blocking behaviour will result in making normal scripts critical to apps otherwise using module scripts to ensure immediate boot execution. That can be fine, but it seems a bit odd and a gotcha to me that performance can be improved just by removing type="module".

@matthewp
Copy link

matthewp commented Aug 2, 2018

I would think nonmodule scripts would be blocked since they could contain a dynamic import as well.

@domenic
Copy link
Collaborator

domenic commented Nov 2, 2018

This is worked out in some detail now in https://github.com/domenic/import-maps/blob/master/spec.md#installation and https://github.com/domenic/import-maps/blob/master/spec.md#modifications-to-fetch-for-import-urls step 1. Basically:

  • Encountering an import of any sort will "lock" the current set of import maps, and attempting to add more will fail.
  • Any imports will be blocked fetching until the current set of import maps finishes.

@domenic domenic closed this as completed Nov 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants