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

ReorderHead transformer accidentally deduplicates based on URL #125

Closed
schlessera opened this issue Apr 5, 2021 · 3 comments
Closed

ReorderHead transformer accidentally deduplicates based on URL #125

schlessera opened this issue Apr 5, 2021 · 3 comments
Labels
Bug Something isn't working Optimizer

Comments

@schlessera
Copy link
Collaborator

The ReorderHead transformer uses the URL as an array key when collecting <script> elements to reorder. This means that if multiple <script> elements use the same URL (like for .mjs script + nomodule fallback), they overwrite each other in the array and are effectively "deduplicated".

The ReorderHead transformer needs a more robust algorithm. It seems that deduplication is still desirable, but will need to rely on "URL + a selection of attributes". The exact selection needs to be clarified, but for now it might help to hash the URL + the presence of nomodule as a first bug fix.

@schlessera
Copy link
Collaborator Author

@westonruter
Copy link
Member

This means that if multiple <script> elements use the same URL (like for .mjs script + nomodule fallback), they overwrite each other in the array and are effectively "deduplicated".

How would they use the same URL? The module scripts end in .mjs and the nomodule ones end in .js. The problem seems to rather be the reliance on the custom-element and custom-template attributes. Switching this to use src as per ampproject/amp-toolbox#1192 seems to do it.

@schlessera
Copy link
Collaborator Author

Fixed via #130

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Optimizer
Projects
None yet
Development

No branches or pull requests

2 participants