fix(plugin-svelte): update ignored sveltekit plugins (fix #392) #393
Conversation
|
|
|
|
✅ Deploy Preview for histoire-examples-svelte3 ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for histoire-examples-vue3 ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for histoire-controls ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for histoire-site ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
@Akryum unfortunately if only the build plugin was ignored, we'll get this error: I've tested and both plugins need to be ignored, at least for my particular setup Edit: oops sorry I've misread your comment. If only middleware is ignored, I get 404, not sure why |
|
I meant "only the middleware plugin" needs to be ignored |
|
Yes my apologies, I misread that. Added an edit to my previous comment just now. |
| // svelte-kit vite plugins | ||
| // reference: https://github.com/sveltejs/kit/blob/master/packages/kit/src/exports/vite/index.js | ||
| 'vite-plugin-sveltekit-build', | ||
| 'vite-plugin-sveltekit-middleware', |
There was a problem hiding this comment.
The two plugins have been renamed to vite-plugin-sveltekit-setup and vite-plugin-sveltekit-compile in the latest version. Sorry for the breaking change. I wanted to sneak this in before SvelteKit 1.0.
We made this change working together with the Storybook team, so I'm hoping it will help Histoire as well. The idea is that you should be able to leave the setup plugin in place to setup SvelteKit's aliases, etc. and just remove the compile plugin. The Storybook folks said that's working for them in dev mode and almost working in build mode, but currently failing so we need to tweak a couple lines in SvelteKit to fix build mode, but hopefully the plugin names should have settled down at least now.
There was a problem hiding this comment.
Thanks for the work @benmccann. I'll adjust the PR
|
@benmccann I tried to include export default defineConfig({
plugins: [HstSvelte()],
viteIgnorePlugins: [
'vite-plugin-sveltekit-setup',
'vite-plugin-sveltekit-compile',
]
setupFile: "./src/routes/histoire.setup.ts",
})Based on your comment, you meant to ignore only Even if I patch the @histoire/plugin-svelte under node_modules with these changes, I get the same result. Like @vnphanquang mentioned in #392, if I revert sveltekit to Maybe I am missing something here. |
@vnphanquang Even if I do that, I get the same result. I have updated my reproduction here. |
|
@vnphanquang FYI - Your workaround works fine for svelte |
vnphanquang
force-pushed
the
fix/392-sveltekit-async-vite-plugin
branch
from
df7641b to
064eedd
Compare
|
@bhvngt thanks for taking the time to test & reproduce. I can confirm your findings are correct. Breaking change is likely from sveltejs/kit#8055, although I lack the knowledge of the ecosystem to determine the specifics. Hopefully someone with more insights can jump in. This is also blocking one of my projects at work from upgrading to latest svelte-kit. For now we're sicking with |
|
This change looks good to me. We released a new version of SvelteKit today that fully support Storybook. I'd love to figure out how to get to the same place with Histoire. I'm on vacation this week, but can take a look at this before too long. To reproduce the error, do I just follow the setup instructions at https://histoire.dev/guide/svelte3/getting-started.html? |
|
@benmccann thanks for the initiative. Yes following instructions from histoire is enough. To save you some time, here is a newly created reproduction repo: https://github.com/vnphanquang/sveltekit-histoire-reproduction (I created a stackblitz in the linked issue but there have been quite some releases of But you know what, in latest I guess what's left to do (if no further improvement is possible) is include instruction to |
|
@bhvngt sorry to bother you again, if possible, can you give it a try again with latest /// <reference types="histoire" />
import { defineConfig as defineViteConfig } from 'vite';
import { sveltekit } from '@sveltejs/kit/vite';
import { defineConfig as defineHistoireConfig } from 'histoire';
import { HstSvelte } from '@histoire/plugin-svelte';
/** @type {import('vite').UserConfig} */
const config = defineViteConfig(async () => ({
plugins: [
- sveltekit(),
+ await sveltekit(),
],
histoire: defineHistoireConfig({
plugins: [HstSvelte()],
+ viteIgnorePlugins: ['vite-plugin-sveltekit-compile'],
}),
}));
export default config; |
|
Yes I can confirm from my end that with the latest @benmccann Like @vnphanquang mentioned in his setup, I had to |
|
Fantastic news that it is working! Users of SvelteKit do not need the |
|
Thanks for all the great work guys! Package VersionsSvelte
Histoire
Vite
|
|
It looks to me like Histoire already does an |
|
Closing in favor of #402 |
|
@benmccann Thanks for pointing out the source. There was an issue with the promise resolution logic. I have created a separate PR #402 that fixes this issue as well as make other changes to make @vnphanquang @Akryum Since there were couple of other changes that were required, I ended up creating a separate PR. Hope thats fine. |
Description
Additional context
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123).