Skip to content

Conversation

@teemingc
Copy link
Member

fixes #13179

This PR backports the default value for Vite 6's ssr.resolve.conditions options so that it doesn't fallback to resolve.conditions which seemingly causes it to resolve to the browser conditional export.

However, I'm not sure if these condition values are right. @dominikg probably knows better.

  • Should we also include 'svelte' as a ssr resolve condition here? I noticed it's included when using vite-plugin-svelte 5 while 4 only includes it in the root resolve condition (which we are falling back to by having ssr.config.resolve as undefined?).

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

No test because it only happens on Vite 5 and our test suite is on Vite 6.

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

Edits

  • Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed.

@changeset-bot
Copy link

changeset-bot bot commented Dec 17, 2024

🦋 Changeset detected

Latest commit: 0c5ca07

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@sveltejs/kit Patch

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

@dominikg
Copy link
Member

development|production is for vite6 only and the other two should already be set.

if it is really resolved to the client implementation the browser condition is active where it should not be.

needs debugging and a testcase.

@dummdidumm
Copy link
Member

Teatcase would be overblown IMO, but I'll try to confirm if this fixes it. Maybe we need to switch to an approach similar to the others where we're leveraging the env package instead (or look what that package is doing to fix it)

@dominikg
Copy link
Member

a testcase and running it with vite5 and vite6 is needed i believe. that should have been there in the first place to make sure the feature works.

Also not sure if this needs a change in the sveltekit vite plugin or vite-plugin-svelte itself or neither and instead the virtual module resolving needs to check the ssr flag.

also not sure why the initial value would be "example.com" in any case, it should take the value from the existing origin ?

@dominikg
Copy link
Member

dominikg commented Dec 17, 2024

soo $app/state isn't a virtual module, it is a combination of a generated alias (https://github.com/sveltejs/kit/tree/2b925b4e5fd9c35f50c2b584ddb1976fb3d255a3/packages/kit/src/runtime/app) and a nested package.json that uses exports conditions to branch for client/server.

this seems to work in vite6 but fail in vite5. surprisingly trying to debug the resolve conditions in vite5 reveals that only the "svelte" condition is exposed in configResolved hook, but that can't be correct. default resolve conditions should include "browser" for client and "node" for ssr. Not sure if they are just not logged or something else is fishy, downgrading to vite 5.3 and vps 4.0 does not change this so not a recent regression.

if this is a general issue with resolve conditions it needs fixing in vite or vite-plugin-svelte, but not in the sveltekit plugin.

But the setup with a generated alias that reveals the path to a nested package.json with exports conditions is special, i am kind of surprised it works in vite6 even. Unfortunately i am out of time, hopefully this gives some pointers.

DEBUG=vite:resolve pnpm dev and DEBUG=vite:config pnpm dev as well as DEBUG=vite-plugin-svelte:* pnpm dev can be used to obtain more information.

vite-plugin-inspect can show resolve results as well.

@Rich-Harris
Copy link
Member

merged #13192 so I'll close this — I assume either approach would have worked fine but it's nice to have the consistency. Would like to investigate using the package.json solution more broadly in SK3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SvelteKit 2.12.1 with @sveltejs/vite-plugin-svelte 4.0.3 return weird server-side results

5 participants