Skip to content
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: remove SvelteKit specific handling #638

Merged
merged 7 commits into from Apr 26, 2023
Merged

Conversation

bluwy
Copy link
Member

@bluwy bluwy commented Apr 26, 2023

Per changeset:

Remove internal SvelteKit specific handling

  • Disallow kit prop in inline options
  • Remove default hydratable: true option for SvelteKit
  • Inspector code mounts on /@vite/client to be framework agnostic

Re no1, I'm not sure how we're using that today, but seems safe to me to remove the runtime validation for it.

Re no2, SvelteKit has set it by itself for quite a while now: https://github.com/sveltejs/kit/blob/fb68c253a59402df2b2b1b5db7a54fad504cdd32/packages/kit/src/exports/vite/index.js#L136

Re no3, this removes the need for the inspector.appendTo option so we don't need kit specific handling for it. Makes way for #631

return;
transform(code: string, id: string) {
if (id.includes('vite/dist/client/client.mjs')) {
return { code: `${code}\nimport('virtual:svelte-inspector-path:load-inspector.js')` };
Copy link
Member Author

@bluwy bluwy Apr 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dynamic import since the Inspector.svelte would rely on /@vite/client too, and it otherwise would cause an import loop and a runtime ReferenceError.

@bluwy bluwy changed the title feat: import inspector from vite client feat: remove SvelteKit specific handling Apr 26, 2023
@dominikg
Copy link
Member

the kit option and setting hydratable: true have been remnants for older versions of sveltekit.
Does removing them make this a breaking change?

Technically applications on older versions of kit can't update to newer v-p-s until they also update kit. But given that v-p-s is a dependency of kit and kit had a critical advisory that urged people to update to a version that is compatible, we could assume that updating v-p-s without also updating kit is not something we expect anyone to do?

@dominikg
Copy link
Member

Older version above refers to sveltekit less than 1.0.0-beta.576. Anyone still using that should have updating to 1.0 as their first priority.

@dominikg dominikg merged commit 5202b0a into main Apr 26, 2023
7 checks passed
@dominikg dominikg deleted the remove-kit-specific-stuff branch April 26, 2023 18:30
@github-actions github-actions bot mentioned this pull request Apr 26, 2023
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.

None yet

2 participants