fix(hmr): add timestamp for assets in dev

[ArnaudBarre]

Fixes: #13379

[stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[ArnaudBarre]

Expected various break of existing test on the exact matching of path. I will fix them once we agree on the fix!

[patak-dev]

We shouldn't do this for public files, no?

I'm thinking we may also need to wait for Vite 5 to apply this change 🤔

[ArnaudBarre]

I need to test how this behaves for public files. But I think this currently marked as deprecated to import public files in source code so I would not worry too much about this.

What are you afraid it will break apart from test suite that are using exact match for paths?

[patak-dev]

I would think that projects shouldn't break as having a ?t= is expected. It still feels like a non-trivial change. Maybe we could move with it during the 4.4 beta and we see if some projects have issues with this.

[bluwy]

But I think this currently marked as deprecated to import public files in source code so I would not worry too much about this.

I don't think it's deprecated (see https://vitejs.dev/guide/assets.html#the-public-directory). But nevertheless I don't see much harm if we add the timestamp for them either.

I think however we should only add the timestamp in dev.

[patak-dev]

/ecosystem-ci run

[vite-ecosystem-ci]

📝 Ran ecosystem CI: Open

suite	result
astro	✅ success
histoire	✅ success
iles	✅ success
ladle	✅ success
laravel	✅ success
marko	❌ failure
nuxt	❌ failure
nx	✅ success
previewjs	✅ success
qwik	✅ success
rakkas	❌ failure
sveltekit	❌ failure
unocss	✅ success
vite-plugin-pwa	✅ success
vite-plugin-ssr	❌ failure
vite-plugin-react	✅ success
vite-plugin-react-pages	✅ success
vite-plugin-react-swc	✅ success
vite-plugin-svelte	✅ success
vite-plugin-vue	✅ success
vite-setup-catalogue	✅ success
vitepress	✅ success
vitest	❌ failure

[patak-dev]

SvelteKit and Vitest are new failures in the ecosystem-ci run above. It is getting a bit difficult to work with so many reds in the results. But maybe better to check these two before merging.

[patak-dev]

Maybe merging main and re-run could also be a good idea.

[ArnaudBarre]

Totally related:
vitest: expected '/src/node_modules/file.mp3?t=16860633…' to be '/src/node_modules/file.mp3'
svletekit:

      -   "http://localhost:5173/src/routes/asset-import/small.png",
      -   "http://localhost:5173/src/routes/asset-import/large.jpg",
      +   "http://localhost:5173/src/routes/asset-import/small.png?t=1686063253094",
      +   "http://localhost:5173/src/routes/asset-import/large.jpg?t=1686063253094",

[patak-dev]

The tests should be updated to match instead of to be equal. I'm hesitant about doing this change in 4.4, I'll let others comment here. We could try it out and revert, or directly move this one to Vite 5.

[bluwy]

I think it's a bit borderline a breaking change that it would be safer to move it to Vite 5. Some user apps who used ?url might not anticipated the path to contain queries too.

[patak-dev]

Ok, let's move it to Vite 5 👍🏼