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

[@rollup/plugin-html] Generated HTML includes _all_ JS chunks, should only include entry chunks #1834

Open
egnor opened this issue Jan 15, 2025 · 4 comments · May be fixed by #1835
Open

[@rollup/plugin-html] Generated HTML includes _all_ JS chunks, should only include entry chunks #1834

egnor opened this issue Jan 15, 2025 · 4 comments · May be fixed by #1835

Comments

@egnor
Copy link

egnor commented Jan 15, 2025

Expected Behavior

The generated HTML should include <script> tags loading only (compiled versions of) the entry points specified in input in rollup.js.

The stackblitz example above uses a random coin-flip to decide whether or not to execute a dynamic import. This makes rollup splits out the dynamic part of the code, which should not be imported by default (that's the whole point of a dynamic import), so the HTML should look like this:

...
  <body>
    <script src="main.js" type="module"></script>
  </body>
...

In the browser it should look like this, depending on the coinflip:

Hello from main.js!
NOT importing dynamic_load.js
End of main.js

or

Hello from main.js!
Importing dynamic_load.js
Hello from dynamic_load.js!
End of main.js

Actual Behavior

The generated HTML includes <script> tags for those scripts, but ALSO includes <script> tags for generated assets, including in this example the dynamically loaded code:

...
  <body>
    <script src="main.js" type="module"></script>
<script src="dynamic_load-DsWEb2CU.js" type="module"></script>
  </body>
...

That means the dynamically loaded JS gets loaded whether or not the code actually opts to perform the load, so you get output that looks like this (depending on the coin flip in main.js).

Hello from main.js!
NOT importing dynamic_load.js
End of main.js
Hello from dynamic_load.js!

Additional Information

NOTE WELL that I used dynamic imports to demonstrate this, but this is not specific to dynamic imports, it will affect any situation where there's a *.js file that should not be immediately imported, i.e. all cases of code-splitting.

@shellscape
Copy link
Collaborator

We had an open PR to address this, but it languished and went stale. Happy to review another.

@egnor egnor changed the title [@rollup/plugin-html] Generated HTML includes _all_ JS assets, should only include entry assets [@rollup/plugin-html] Generated HTML includes _all_ JS chunks, should only include entry chunks Jan 15, 2025
@egnor
Copy link
Author

egnor commented Jan 15, 2025

Aha, found that old PR: #688

@egnor
Copy link
Author

egnor commented Jan 15, 2025

@shellscape I'm not sure I understand all the back-and-forth in that original PR, but just to clarify, I think this could be a very surgical change to the default template which changes this code

.map(({ fileName }) => {

to something like this

    .map((file) => {
      if (file.type === "chunk" && file.isEntry) {
        const attrs = makeHtmlAttributes(attributes.script);
        return `<script src="${publicPath}${file.fileName}"${attrs}></script>`;
      }
    })

and then update/enrich some tests/docs. If that's even approximately correct I can see about spinning up such a PR?

@shellscape
Copy link
Collaborator

Sounds good to me. That one was pretty old, I wouldn't give the conversation on that PR much influence and would encourage you to apply a fix as you see it best.

egnor added a commit to egnor/rollup-plugins that referenced this issue Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants