-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(sveltekit): Only inject fetch proxy script for SvelteKit < 2.16.0 #15126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(sveltekit): Only inject fetch proxy script for SvelteKit < 2.16.0 #15126
Conversation
size-limit report 📦
|
44f6aea
to
ce32c24
Compare
46cc1a4
to
a0bc030
Compare
❌ 1 Tests Failed:
View the top 1 failed tests by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
383920f
to
f52fd16
Compare
dev-packages/e2e-tests/test-applications/sveltekit-2-twp/tests/sdk.test.ts
Outdated
Show resolved
Hide resolved
f52fd16
to
0b53e27
Compare
if (options.injectFetchProxyScript == null) { | ||
try { | ||
// @ts-expect-error - the dynamic import is fine here | ||
const { VERSION } = await import('@sveltejs/kit'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
l: Does it make sense to cache this somewhere? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do :) options.injectFetchProxyScript
will only be set once
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
With v9 onwards, we'll only inject the fetch proxy script into SvelteKit versions where we actually need it. This is all kit versions below 2.16.0. I'll update docs in a follow-up PR.
fixes #15007