fix(plugin-svelte): update ignored sveltekit plugins (fix #392) #393

vnphanquang · 2022-12-09T14:52:09Z

Description

Additional context

What is the purpose of this pull request?

Bug fix
New Feature
Documentation update
Other

Before submitting the PR, please make sure you do the following

If it's a new feature, provide a convincing reason to add it. Ideally, you should open a suggestion issue first and have it approved before working on it.
Read the Contributing Guidelines.
Read the Pull Request Guidelines and follow the Commit Convention.
Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
Ideally, include relevant tests that fail without this PR but pass with it.

stackblitz · 2022-12-09T14:52:11Z

Run & review this pull request in StackBlitz Codeflow.

codesandbox · 2022-12-09T14:52:11Z

Open in CodeSandbox Web Editor | VS Code | VS Code Insiders

netlify · 2022-12-09T14:52:14Z

✅ Deploy Preview for histoire-examples-svelte3 ready!

Name	Link
🔨 Latest commit	`064eedd`
🔍 Latest deploy log	https://app.netlify.com/sites/histoire-examples-svelte3/deploys/6396e41f35051d00095ef9be
😎 Deploy Preview	https://deploy-preview-393--histoire-examples-svelte3.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

netlify · 2022-12-09T14:52:15Z

✅ Deploy Preview for histoire-examples-vue3 ready!

Name	Link
🔨 Latest commit	`064eedd`
🔍 Latest deploy log	https://app.netlify.com/sites/histoire-examples-vue3/deploys/6396e41fa4824d0008aad956
😎 Deploy Preview	https://deploy-preview-393--histoire-examples-vue3.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

netlify · 2022-12-09T14:52:15Z

✅ Deploy Preview for histoire-controls ready!

Name	Link
🔨 Latest commit	`064eedd`
🔍 Latest deploy log	https://app.netlify.com/sites/histoire-controls/deploys/6396e41f2248cf00080ca44f
😎 Deploy Preview	https://deploy-preview-393--histoire-controls.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

netlify · 2022-12-09T14:52:15Z

✅ Deploy Preview for histoire-site ready!

Name	Link
🔨 Latest commit	`064eedd`
🔍 Latest deploy log	https://app.netlify.com/sites/histoire-site/deploys/6396e41fa98275000811fb37
😎 Deploy Preview	https://deploy-preview-393--histoire-site.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Akryum

If I understood the svelte kit pr probably, we can keep the build plugin and only disallow the middleware plugin?

vnphanquang · 2022-12-09T15:40:40Z

@Akryum unfortunately if only the build plugin was ignored, we'll get this error:

node_modules/@sveltejs/kit/src/exports/vite/dev/index.js:37
        sync.init(svelte_config, vite_config.mode);
                                             ^

TypeError: Cannot read properties of undefined (reading 'mode')

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

Akryum · 2022-12-09T15:46:59Z

I meant "only the middleware plugin" needs to be ignored

vnphanquang · 2022-12-09T15:48:01Z

Yes my apologies, I misread that. Added an edit to my previous comment just now.

benmccann · 2022-12-11T16:43:46Z

packages/histoire-plugin-svelte/src/index.ts

+          // 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',


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.

Thanks for the work @benmccann. I'll adjust the PR

bhvngt · 2022-12-12T07:27:46Z

@benmccann I tried to include vite-plugin-sveltekit-setup and vite-plugin-sveltekit-compile as part of viteIgnorePlugins. Running histoire dev still shows me the default svelte-kit home page Here's my re-production.

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 vite-plugin-sveltekit-compile. Even that produces status quo.

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 1.0.0-next.573 everything works fine.

Maybe I am missing something here.

vnphanquang · 2022-12-12T07:33:32Z

@bhvngt you probably have to await sveltekit() in your vite.config as I mentioned in #392.

bhvngt · 2022-12-12T07:49:33Z

@bhvngt you probably have to await sveltekit()

@vnphanquang Even if I do that, I get the same result. I have updated my reproduction here.

bhvngt · 2022-12-12T08:01:31Z

@vnphanquang FYI - Your workaround works fine for svelte 1.0.0-next.574 and vite ^3.2.5. But it stops working once I upgrade to sveltkit 1.0.0-next.582 with vite set as ^4.0.0.

…v#392)

vnphanquang · 2022-12-12T08:22:13Z

@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 574.

benmccann · 2022-12-14T05:16:55Z

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?

vnphanquang · 2022-12-14T08:06:03Z

@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 kit since then, please use the repo instead)

But you know what, in latest kit by ignoring the right plugin AND await sveltekit() things are working again. Previously it wasn't working for me & @bhvngt in 582 🤔 .

I guess what's left to do (if no further improvement is possible) is include instruction to await kit in histoire docs and merge this?

vnphanquang · 2022-12-14T08:18:51Z

@bhvngt sorry to bother you again, if possible, can you give it a try again with latest kit in your setup? It is working for me at https://github.com/vnphanquang/sveltekit-histoire-reproduction with the following config:

/// <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;

bhvngt · 2022-12-14T10:04:52Z

Yes I can confirm from my end that with the latest sveltekit and with config shared by @vnphanquang (thanks a ton), its working fine on my setup.

@benmccann Like @vnphanquang mentioned in his setup, I had to await sveltekit() to make this work. When I create a new kit project from the skeleton template using pnpm create svelte@next, by default call to sveltekit() is not awaited inside vite.config.js. Just curious, if this is something that users of histoire needs to do or is in general all sveltekit project needs to change.

benmccann · 2022-12-14T14:21:31Z

Fantastic news that it is working! Users of SvelteKit do not need the await because it is something that Vite will handle. Ideally we should update Histoire to handle it as well so that users do not need to add the await

oneezy · 2022-12-14T18:44:25Z

Thanks for all the great work guys!
I can confirm this working on my end as well with @vnphanquang 's await sveltekit() suggestion (and on kit 1.0.0! 🎉)

Package Versions

Svelte

@sveltejs/kit 1.0.0 🎉
@sveltejs/adapter-auto 1.0.0
svelte 3.55.0
svelte-check 2.10.2

Histoire

histoire 0.11.9
@histoire/controls 0.11.9
@histoire/plugin-svelte 0.11.9

Vite

vite 4.0.1

benmccann · 2022-12-14T22:17:18Z

It looks to me like Histoire already does an await so I'm not quite sure why it'd be required in the user's config:

histoire/packages/histoire/src/node/vite.ts

Line 82 in 62b3fd5

flatPlugins.push(...await Promise.all(pluginOption))

vnphanquang · 2022-12-15T11:18:52Z

Closing in favor of #402

bhvngt · 2022-12-15T11:19:59Z

@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 histoire work with the v1.0.0 of sveltekit.

@vnphanquang @Akryum Since there were couple of other changes that were required, I ended up creating a separate PR. Hope thats fine.

vnphanquang mentioned this pull request Dec 9, 2022

Svelte plugin: incompatibility with svelte-kit starting from 1.0.0-next.574 #392

Closed

6 tasks

Akryum reviewed Dec 9, 2022

View reviewed changes

benmccann reviewed Dec 11, 2022

View reviewed changes

This was referenced Dec 11, 2022

🟧 Sveltekit: Can't import Sveltekit modules in components (i.e. $path) #384

Closed

Fails to resolve aliases within a sveltekit project #375

Closed

fix(plugin-svelte): update ignored sveltekit plugins (fix histoire-de…

064eedd

…v#392)

vnphanquang force-pushed the fix/392-sveltekit-async-vite-plugin branch from df7641b to 064eedd Compare December 12, 2022 08:19

bhvngt mentioned this pull request Dec 15, 2022

feat(sveltekit): upgraded sveltekit support to svelltekit v1.0.0 (fix #392, #397) #402

Merged

10 tasks

vnphanquang closed this Dec 15, 2022

jacob-8 mentioned this pull request Sep 12, 2023

[BUG] Rollup failed to resolve import "$env/static/public" microsoft/playwright#18825

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(plugin-svelte): update ignored sveltekit plugins (fix #392) #393

fix(plugin-svelte): update ignored sveltekit plugins (fix #392) #393

vnphanquang commented Dec 9, 2022 •

edited

stackblitz bot commented Dec 9, 2022

codesandbox bot commented Dec 9, 2022

netlify bot commented Dec 9, 2022 •

edited

netlify bot commented Dec 9, 2022 •

edited

netlify bot commented Dec 9, 2022 •

edited

netlify bot commented Dec 9, 2022 •

edited

Akryum left a comment

vnphanquang commented Dec 9, 2022 •

edited

Akryum commented Dec 9, 2022 •

edited

vnphanquang commented Dec 9, 2022

benmccann Dec 11, 2022

vnphanquang Dec 12, 2022

bhvngt commented Dec 12, 2022

vnphanquang commented Dec 12, 2022

bhvngt commented Dec 12, 2022

bhvngt commented Dec 12, 2022

vnphanquang commented Dec 12, 2022 •

edited

benmccann commented Dec 14, 2022

vnphanquang commented Dec 14, 2022 •

edited

vnphanquang commented Dec 14, 2022 •

edited

bhvngt commented Dec 14, 2022

benmccann commented Dec 14, 2022

oneezy commented Dec 14, 2022 •

edited

benmccann commented Dec 14, 2022

vnphanquang commented Dec 15, 2022

bhvngt commented Dec 15, 2022

fix(plugin-svelte): update ignored sveltekit plugins (fix #392) #393

fix(plugin-svelte): update ignored sveltekit plugins (fix #392) #393

Conversation

vnphanquang commented Dec 9, 2022 • edited

Description

Additional context

What is the purpose of this pull request?

Before submitting the PR, please make sure you do the following

stackblitz bot commented Dec 9, 2022

codesandbox bot commented Dec 9, 2022

netlify bot commented Dec 9, 2022 • edited

✅ Deploy Preview for histoire-examples-svelte3 ready!

netlify bot commented Dec 9, 2022 • edited

✅ Deploy Preview for histoire-examples-vue3 ready!

netlify bot commented Dec 9, 2022 • edited

✅ Deploy Preview for histoire-controls ready!

netlify bot commented Dec 9, 2022 • edited

✅ Deploy Preview for histoire-site ready!

Akryum left a comment

Choose a reason for hiding this comment

vnphanquang commented Dec 9, 2022 • edited

Akryum commented Dec 9, 2022 • edited

vnphanquang commented Dec 9, 2022

benmccann Dec 11, 2022

Choose a reason for hiding this comment

vnphanquang Dec 12, 2022

Choose a reason for hiding this comment

bhvngt commented Dec 12, 2022

vnphanquang commented Dec 12, 2022

bhvngt commented Dec 12, 2022

bhvngt commented Dec 12, 2022

vnphanquang commented Dec 12, 2022 • edited

benmccann commented Dec 14, 2022

vnphanquang commented Dec 14, 2022 • edited

vnphanquang commented Dec 14, 2022 • edited

bhvngt commented Dec 14, 2022

benmccann commented Dec 14, 2022

oneezy commented Dec 14, 2022 • edited

Package Versions

benmccann commented Dec 14, 2022

vnphanquang commented Dec 15, 2022

bhvngt commented Dec 15, 2022

vnphanquang commented Dec 9, 2022 •

edited

netlify bot commented Dec 9, 2022 •

edited

netlify bot commented Dec 9, 2022 •

edited

netlify bot commented Dec 9, 2022 •

edited

netlify bot commented Dec 9, 2022 •

edited

vnphanquang commented Dec 9, 2022 •

edited

Akryum commented Dec 9, 2022 •

edited

vnphanquang commented Dec 12, 2022 •

edited

vnphanquang commented Dec 14, 2022 •

edited

vnphanquang commented Dec 14, 2022 •

edited

oneezy commented Dec 14, 2022 •

edited