-
Notifications
You must be signed in to change notification settings - Fork 20
chore: updated SvelteKit framework info #818
Conversation
@@ -4,7 +4,7 @@ | |||
"category": "build_tool", | |||
"detect": { | |||
"npmDependencies": ["vite"], | |||
"excludedNpmDependencies": ["@shopify/hydrogen", "@builder.io/qwik", "solid-start", "solid-js"], | |||
"excludedNpmDependencies": ["@shopify/hydrogen", "@builder.io/qwik", "solid-start", "solid-js", "@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.
Adding SvelteKit as an excluded dependency so it doesn't get picked up by the vite framework info config.
"command": "svelte-kit build", | ||
"directory": "build" | ||
"command": "vite build", | ||
"directory": ".svelte-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.
Do we have to worry about sites with older versions of SvelteKit breaking with this change? I know we had a similar conversation with respect to vite
but I think there might be a lot more users that this change would affect.
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.
I had thought about this, kind of like the PR #792, but in this case, these are the settings for the first official release for SvelteKit.
This also brings up the conversation again about potentially versioning framework information.
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.
This is something I mentioned to Nick in slack. There is a workaround by adding the targetPort
to the netlify,toml
file that can be added by anyone on an older version, but this will fix it so new updated sites don't have to do the workaround to get the CLI command to work.
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.
Got it, thanks both!
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.
Looks good to me.
🎉 Thanks for submitting a pull request! 🎉
Summary
Updates the framework info for SvelteKit. I confirmed these changes with Brittney.
Fixes #817
For us to review and ship your PR efficiently, please perform the following steps:
ensures we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a
typo or something that`s on fire 🔥 (e.g. incident related), you can skip this step.
passes our tests.
A picture of a cute animal (not mandatory, but encouraged)