fix: add experimental.forkPreloads flag
#15135
Conversation
Adds a new experimental flag to gate the use of Svelte's `fork` API for preloading. The flag defaults to `false`, disabling the fork-based preloading behavior until explicitly enabled. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: e2451ef The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
Conduitry
left a comment
Conduitry
left a comment
There was a problem hiding this comment.
The PR description says "feat" but the changelog says "fix" and changesets bump says "patch". What do we want to call this?
I might be in favor of the changelog saying "breaking". I don't have a strong opinion about whether the bump itself should be minor or patch.
elliott-with-the-longest-name-on-github
changed the title
experimental.enhancedPreloading flagexperimental.enhancedPreloading flag
|
Hmm yeah, I'd be okay with putting |
|
I like |
| }, /^config\.kit\.experimental\.tracing\.server should be true or false, if specified$/); | ||
| }); | ||
|
|
||
| test('errors on invalid enhancedPreloading values', () => { |
There was a problem hiding this comment.
i'm not entirely sure we need these tests, which are just testing the existing config handling code, which should already be tested. There's nothing unique about this config key
There was a problem hiding this comment.
Yeah, I figured they can't hurt -- they at least protect from regressions and cost basically nothing
There was a problem hiding this comment.
yeah but we have equivalent tests for the others. and this test will only last as long as the flag remains
| __SVELTEKIT_APP_DIR__: s(kit.appDir), | ||
| __SVELTEKIT_EMBEDDED__: s(kit.embedded), | ||
| __SVELTEKIT_EXPERIMENTAL__REMOTE_FUNCTIONS__: s(kit.experimental.remoteFunctions), | ||
| __SVELTEKIT_ENHANCED_PRELOADING__: s(kit.experimental.enhancedPreloading), |
There was a problem hiding this comment.
I assume s() is turning it into a string. If so I think this will leave it enabled by default because false will be converted to the string 'false' which will be evaluated as true when you check the value
There was a problem hiding this comment.
no, it won't — it will be interpreted as false, same as __SVELTEKIT_EXPERIMENTAL__REMOTE_FUNCTIONS__
There was a problem hiding this comment.
Not quite. It's very confusing. It turns it into a string, which is false, but that string is injected as-is into the runtime code. So the string 'foo' would become foo in the runtime code replacement, and 'false' becomes the literal false
Rich-Harris
left a comment
Rich-Harris
left a comment
There was a problem hiding this comment.
LGTM, though would like to give people a chance to come up with anything more descriptive than enhancedPreloading...
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Conduitry
changed the title
experimental.enhancedPreloading flagexperimental.forkPreloads flag
Adds a new experimental flag to gate the use of Svelte's
forkAPI for preloading. The flag defaults tofalse, disabling the fork-based preloading behavior until explicitly enabled.We're making this change because the current behaviour is buggy in some cases, such as when a
redirectoccurs when rendering the destination, and we don't want people to be prevented from updating to the latest version.Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm testand lint the project withpnpm lintandpnpm checkChangesets
pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.Edits