vite icon indicating copy to clipboard operation
vite copied to clipboard

fix(html): move importmap before module scripts

Open bluwy opened this issue 1 year ago • 6 comments

Description

fix #9334

Arrange script type="importmap" as first in transformIndexHtml.

Additional context

Needed this for dev external tests 🙂


What is the purpose of this pull request?

  • [x] Bug fix
  • [ ] New Feature
  • [ ] Documentation update
  • [ ] Other

Before submitting the PR, please make sure you do the following

  • [x] Read the Contributing Guidelines.
  • [x] Read the Pull Request Guidelines and follow the Commit Convention.
  • [x] Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • [x] Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • [x] Ideally, include relevant tests that fail without this PR but pass with it.

bluwy avatar Jul 27 '22 09:07 bluwy

Nice fix. I'm leaning towards the alternative proposal though. IIUC this PR could end up reordering styles and scrpts, no?

patak-dev avatar Jul 27 '22 09:07 patak-dev

This PR currently reorders script type-"module" only if an importmap exist, so other cases it should work like before. Also happy to try out the alternative though.

bluwy avatar Jul 27 '22 09:07 bluwy

I'm leaning towards the alternative one, too. I think it is more easy to understand the behavior, especially when an importmap is injected by transformIndexHtml.

Not sure if this would be surprising for users though.

I think a warning will make it less surprising. For example, when the input index.html has <script type="module"> before importmap.

sapphi-red avatar Jul 27 '22 10:07 sapphi-red

Updated to use a Vite plugin with transformIndexHtml to move the importmap instead. I didn't add a warning though as I think there are cases where someone has no control over where the importmap is injected in a Vite plugin (?).

Also avoiding traverseHtml to save a bit of perf.

bluwy avatar Jul 27 '22 14:07 bluwy

I didn't add a warning though as I think there are cases where someone has no control over where the importmap is injected in a Vite plugin (?).

I was thinking of when the input of transformIndexHtml is invalid, not during the transformIndexHtml. Example: https://stackblitz.com/edit/vitejs-vite-e3cqtr?file=index.html&terminal=dev This one is invalid and won't work before this PR but IIUC this works after this PR.

sapphi-red avatar Jul 27 '22 15:07 sapphi-red

Yeah I think that could be something that we can warn by default. Would have to warn before any transfromIndexHtml happens 🤔 I'll see if I can add that tomorrow.

bluwy avatar Jul 27 '22 15:07 bluwy