Repository navigation
fix(deps): update @sveltejs/vite-plugin-svelte to v6 - #14821
Conversation
🦋 Changeset detectedLatest commit: 2a1104c The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 Changeset Validation Results❌ Changeset validation failed Issues Found:
|
|
The HMR testing is failing. It seems that It might be best for this PR to wait until #14306 is merged into |
|
Hello, #14306 has been merged. Would you be willing to update this PR for that? No worries if you do not have time or anything, we'll get to it eventually otherwise! |
809e318 to
2b75cb2
Compare
|
| 📦 Package | 🔒 Before | 🔓 After |
|---|---|---|
| @cloudflare/kv-asset-handler | trusted-with-provenance | none |
| @cloudflare/unenv-preset | trusted-with-provenance | none |
| workerd | trusted-with-provenance | none |
| miniflare | trusted-with-provenance | none |
| youch | provenance | none |
| @cloudflare/workerd-darwin-64 | trusted-with-provenance | none |
| @cloudflare/workerd-darwin-arm64 | trusted-with-provenance | none |
| @cloudflare/workerd-linux-64 | trusted-with-provenance | none |
| @cloudflare/workerd-linux-arm64 | trusted-with-provenance | none |
| @cloudflare/workerd-windows-64 | trusted-with-provenance | none |
| wrangler | trusted-with-provenance | none |
|
Hi @Princesseuh. I've merged the latest There is an unresolved issue from the e2e tests. The HMR for svelte is broken. Here is a step-by-step reproduction to show this issue. I'm not sure how to fix it.
cd astro
pnpm run build
cd packages/astro/e2e/fixtures/svelte-component
./node_modules/.bin/astro dev
astro-hmr1.mp4 |
|
It looks like if I make astro-hmr2.mp4@dominikg I would really appreciate any suggestions you might have to help debug this issue. |
|
for the virtual module ids please note that As for hmr, vite-plugin-svelte doesn't know if a .svelte component contributed to the ssr response and if you want to reload the page or not, it only provides updates for the client environment. In the past (i havn't checked in a while) this worked in astro for svelte components with :client directive. |
I was considering that allowing |
|
zero-tagged ids are excluded in vite-plugin-svelte@6.2.3 |
|
heads up that there is going to be a new major of vite-plugin-svelte soon that will exclusively support vite8. If astro@6 plans to use vite8 you might want to wait on that. |
|
@dominikg It looks like this patch resolves the HMR issue. I included some analysis in the description of that PR. Do you think my thoughts make sense? |
I'm not part of the Astro team. From what I understand, the Astro team's goal is to release |
aed0ff3 to
265cda3
Compare
|
@Princesseuh Hi! With the help of the maintainer of vite-plugin-svelte, I've resolved all the issues in this PR, and it should now be ready for review. However, some CI tests are timing out, and I'm not sure whether they are related to the changes in this PR. |
|
I'll rerun the CI, but it's quite possible that it is related to this PR as the tests pass on |
c969c0e to
416e054
Compare
|
Yes, I can reproduce this issue locally. The integration test for I can also confirm that the following updates in {
"name": "@test/astro-cloudflare-with-svelte",
"version": "0.0.0",
"private": true,
"dependencies": {
"@astrojs/cloudflare": "workspace:*",
- "@astrojs/svelte": "^7.2.4",
+ "@astrojs/svelte": "workspace:*",
"astro": "workspace:*",
"svelte": "^5.46.1"
}
}In other words, this test issue is not related to upgrading I guess that's why only I'll try to figure it out. If I fail, I will just remove the update in |
416e054 to
0d5bfd4
Compare
|
All tests have passed now. Two changes have been made:
|
|
I believe that pre-bundle error thing is an actual bug in the Cloudflare integration, so we shouldn't work around it, let me notify people who knows how that work! |
|
So I had a look a this PR. I added a Svelte component to the cloudflare fixture, and I'm seeing the following warnings: It seems that some Svelte code is using Node.js APIs, while they shouldn't. |
|
I just pushed a change that should improve the environment configuration of our adapter. Unfortunately, we can't do much to avoid Node.js code from svelte source code, unless there's some vite trick I don't know about. I agree with @Princesseuh that the pre-bundle bug is weird, and regardless of the configuration I tried, it doesn't change much. |
node:crypto is used as a dynamic fallback here: https://github.com/sveltejs/svelte/blob/6d90b96e990d9dd45f55effb4431393c3e610937/packages/svelte/src/internal/server/crypto.js#L15 async_hooks is required for AsyncLocalStorage, used here: https://github.com/sveltejs/svelte/blob/6d90b96e990d9dd45f55effb4431393c3e610937/packages/svelte/src/internal/server/render-context.js#L73 note that both are used in server modules. Whatever is producing the warnings above needs to be updated or you have to check if for some reason you are bleeding svelte server code into client outputs. |
|
@dominikg the warnings are emitted by the Cloudflare vite plugin, which does static analysis of the code. Cloudflare doesn't support Node.js APIs out of the box, and it requires a flag in the |
|
We should be able to enable just the |
|
What is missing to land this PR? Apologies, I didn't quite follow |
|
I think we can merge it. Users will need to add the Node.js flags in their |
The actions-build fixture test could not fail when the withastro#16961 guard was reverted: it pinned prerenderEnvironment 'node' (skipping the default workerd prerender path), its second build was a pure optimizer cache hit rather than a stale-cache scenario, and its assertions never touched Actions output. Replace it with a unit test that invokes the adapter's astro:config:setup hook directly and asserts both observable effects of the isTypeGenPhase guard: configureServer is stripped from the Cloudflare Vite plugins and dependency discovery is disabled for every environment during build/sync, while dev keeps both. Verified the new test fails when the guard is neutered to sync-only or removed. Restore buildWithRetry: the stale-prebundle race it guards (withastro#14821) was never root-caused or fixed, and the suite runs all 49 files serially in one process with no CI-level retry to absorb a flake. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The actions-build fixture test could not fail when the withastro#16961 guard was reverted: it pinned prerenderEnvironment 'node' (skipping the default workerd prerender path), its second build was a pure optimizer cache hit rather than a stale-cache scenario, and its assertions never touched Actions output. Replace it with a unit test that invokes the adapter's astro:config:setup hook directly and asserts both observable effects of the isTypeGenPhase guard: configureServer is stripped from the Cloudflare Vite plugins and dependency discovery is disabled for every environment during build/sync, while dev keeps both. Verified the new test fails when the guard is neutered to sync-only or removed. Restore buildWithRetry: the stale-prebundle race it guards (withastro#14821) was never root-caused or fixed, and the suite runs all 49 files serially in one process with no CI-level retry to absorb a flake.
Changes
Updates
@sveltejs/vite-plugin-svelteinpackage.jsonfrom v5 to v6@sveltejs/vite-plugin-svelte@6.0.0drops support for Node v18 and requires Node v20.19+. This is the same requirement as the upcoming astro v6.Check this link for the full changelog of
@sveltejs/vite-plugin-svelte@6.0.0:Handle virtual modules IDs with
\0vite-plugin-svelte v5 used to use
createFilterfromrollupto match Svelte files.createFilterincludes all IDs with\0at createFilter.ts#L51. This ensures that all virtual modules with an ID starting with\0astro-entry:won't be matched.vite-plugin-svelte v6 uses Vite's new object hook syntax for matching and simply matches all IDs ending with
.svelte.This change causes the following errors:
I fixed this issue by updating
packages/astro/src/core/build/plugins/plugin-component-entry.tsand making sure the virtual module doesn't end with.svelte(it now ends with.svelte.jsinstead).Update: fixed on the
vite-plugin-svelteside.Testing
Update some package.json files of the integration tests so that they will use the local build of
@astrojs/svelte.Docs
See changeset.