Load AMD modules with the same crossorigin attribute as document.currentScript #801
Conversation
| @@ -278,6 +289,7 @@ function loadDeps( | |||
|
|
|||
| // We have a dependency on a real module. | |||
| const dependency = getModule(resolveUrl(module.urlBase, depSpec)); | |||
| dependency.crossorigin = module.crossorigin; | |||
There was a problem hiding this comment.
We should only set crossorigin once, just to make things more predictable.
We could use undefined to mean, not yet set to any value and check for it here.
It'd still be racy, but I'm not sure how we could ameliorate that, as we want to do as much loading as we can as early as we can. In any case, it's a pretty obscure edge case, where you import the same dep both from a script with a crossorigin and from one without.
| // can still get document.currentScript. Note: IE11 doesn't support | ||
| // crossorigin attribute nor currentScript, so it will always be null. | ||
| const crossorigin = document.currentScript ? | ||
| document.currentScript.getAttribute('crossorigin') : null; |
There was a problem hiding this comment.
It looks like the default value for modules has changed to "same-origin" in Chrome 71. That's a much more useful default, we should probably default to that as well
https://www.chromestatus.com/feature/6710957388595200
There was a problem hiding this comment.
AFAIK same-origin is not a valid value for the crossorigin attribute. When applied to a script tag in Chrome:
There was a problem hiding this comment.
It looks like a crossOrigin value of "anonymous" corresponds to the "same-origin" credentials mode, so I think that's what we should default to here.
There was a problem hiding this comment.
If the source is:
<!-- No crossorigin attribute -->
<script type="module" src="src/components/my-app.js"></script>Which AMD transforms to:
<!-- No crossorigin attribute -->
<script>define(['src/components/my-app.js']);</script>Then the AMD version is a non-module script that would use "no-cors" mode, not "same-origin". Defaulting to crossorigin="anonymous" would break the existing behavior on builds from module source that don't include the crossorigin attribute, though adding crossorigin="anonymous" makes it closer to native module behavior.
I guess this is one of those design decisions between closer to native module behavior vs. keeping it simple (i.e. not mimicking crossorigin behavior unless the user explicitly uses the attribute). I'm inclined to stick with the latter because making this default change would break existing preload headers on AMD builds (i.e. would need to add crossorigin=anonymous; to preload headers - ref Polymer/prpl-server#85), though it's true that in these cases the preload headers on module (non-transformed) builds would never have worked properly anyways.
There was a problem hiding this comment.
Hm. So the main goal – I assume – of defaulting to crossorigin="anonymous" is to behave more like normal script tags. So in that sense setting a crossorigin would make things more complicated in cases like preload headers.
On the other hand, those preload headers won't work with modules, and the goal of this loader is to be an AMD loader that behaves as much as possible like the native module loader.
How much work would it be to modify our push manifest generator to add the crossorigin property? I've approved Polymer/prpl-server#85
There was a problem hiding this comment.
The default for normal script tags is no crossorigin attribute, not crossorigin="anonymous", which requires no modification to existing preload headers. Switching the default to crossorigin="anonymous" would require changes to preload headers, which is why I'm hesitant on making the change.
We (shop and pwa-starter-kit) don't rely on the push-manifest generator anymore since we're now only using bundled output and want URL-to-assets mappings, so we write our own push-manifest which we manually modify.
There was a problem hiding this comment.
Yeah, we're in agreement on the technical details.
I think defaulting to crossorigin="anonymous" is the right call. It will cause a bit of friction in the short term, but only friction that people would see when they use real modules.
The point of this library is to emulate modules, so I believe that's what we should do here.
No description provided.