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

Remove vite-plugin-svelte-kit when detected #19522

Merged
merged 9 commits into from
Oct 19, 2022

Conversation

JReinhold
Copy link
Contributor

@JReinhold JReinhold commented Oct 18, 2022

Issue: #19280

What I did

How to test

  • Create the svelte-kit/skeleton-js sandbox
  • Ensure that it both runs in dev and it builds

@JReinhold JReinhold self-assigned this Oct 18, 2022
@JReinhold JReinhold added the build Internal-facing build tooling & test updates label Oct 18, 2022
@JReinhold JReinhold requested a review from IanVS October 18, 2022 21:48
@JReinhold JReinhold marked this pull request as ready for review October 18, 2022 21:48
@JReinhold JReinhold changed the title Jeppe/sb-544-sveltekit-follow-up Remove vite-plugin-svelte-kit when detected Oct 19, 2022
// recursively remove all plugins with the given names
export const withoutVitePlugins = (
plugins: PluginOption[] = [],
namesToRemove: string[]
Copy link
Member

Choose a reason for hiding this comment

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

I think you've unintentionally helped me add another feature which I've had in mind, which is allowing users to remove plugins from their vite config by providing a list of names, similar to what Histoire allows: https://histoire.dev/guide/config.html#ignoring-plugins. Having this function will come in handy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ha great. I just thought that I'd written it 3 times now that it probably deserves it's own utility.

.circleci/config.yml Outdated Show resolved Hide resolved
@JReinhold JReinhold merged commit 3020e77 into next Oct 19, 2022
@JReinhold JReinhold deleted the jeppe/sb-544-sveltekit-follow-up branch October 19, 2022 21:17
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

@JReinhold FWIW I think this is a maintenance PR since it touches user-facing code in a significant way. build means it's completely internal to our build system.

@shilman shilman added maintenance User-facing maintenance tasks and removed build Internal-facing build tooling & test updates labels Oct 20, 2022
@JReinhold
Copy link
Contributor Author

@shilman We really need a written guide on those labels soon. 😬

@gbkwiatt
Copy link

Can someone explain how it should work now ? Previously I've had workaround const plugins = config.plugins.flat(1).filter((p) => p.name !== 'vite-plugin-svelte-kit'); but now it doesn't work on 42 version, and it doesn't build correctly anymore.

@JReinhold
Copy link
Contributor Author

Can someone explain how it should work now ? Previously I've had workaround const plugins = config.plugins.flat(1).filter((p) => p.name !== 'vite-plugin-svelte-kit'); but now it doesn't work on 42 version, and it doesn't build correctly anymore.

None of that should be necessary anymore as Storybook automatically handles that, so hopefully you should be able to delete that.

If you're still experiencing issues with your specific setup I might be easier to reach out on Discord. I'm JReinhold#4884

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants