Add vite support #72

IanVS · 2022-10-06T01:29:35Z

Closes #64

This adds the vite plugin from https://github.com/storybookjs/storybook/blob/next/code/frameworks/svelte-vite/src/plugins/csf-plugin.ts into this repo, and sets up a viteFinal to load it on vite storybook projects. There are a few things I'd like to call out here:

Without setting a target for @babel/preset-env, it was trying to add regeneratorRuntime into the built code, which failed because we're not adding it as a polyfill. I'm imagining that this PR, if merged, would be for use with storybook 7.0+, which supports Chrome 100+ and Node 14+, so those are the targets I added. This means it's probably a breaking change for this package, so I wanted to call that out explicitly.
I needed to turn off some typescript eslint rules, because this repo does not use typescript.
If/when we merge this, I'll remove the vite plugin from the storybook vite builder, so that they're not duplicated.
Dual-builder support is a little bit of a bummer in terms of dependencies. I added magic-string as a dependency because it's used in the plugin, and it doesn't make sense to have as a peer dep. The others, I made optional peer deps. It's safe to assume that vite will be available when viteFinal is run, and @sveltejs/vite-plugin-svelte should be as well in a svelte vite project, but I added a little safety check around that one.

I tested this by creating an example svelte project, modifying the vite builder to not inject the plugin and copying its dist over to the example, and building this addon and copying it's dist over as well. It worked correctly, but it might be nice to consider some tests or example project in this addon, for testing, as well. I didn't undertake that here, but can if desired.

📦 Published PR as canary version: 2.0.11--canary.72.ae738ba.0

✨ Test out this PR locally via:

npm install @storybook/addon-svelte-csf@2.0.11--canary.72.ae738ba.0
# or 
yarn add @storybook/addon-svelte-csf@2.0.11--canary.72.ae738ba.0

Version

Published prerelease version: v3.0.0-next.1

Changelog

💥 Breaking Change

Story Indexer #77 (@JReinhold)

🐛 Bug Fix

Add vite support #72 (@IanVS)

Authors: 2

Ian VanSchooten (@IanVS)
Jeppe Reinhold (@JReinhold)

socket-security · 2022-10-06T01:31:30Z

Socket Security Pull Request Report

👍 No new dependency issues detected in pull request

Pull request report summary

Issue	Status
Install scripts	✅ 0 issues
Native code	✅ 0 issues
Bin script confusion	✅ 0 issues
Bin script shell injection	✅ 0 issues
Unresolved require	✅ 0 issues
Invalid package.json	✅ 0 issues
HTTP dependency	✅ 0 issues
Git dependency	✅ 0 issues
Potential typo squat	✅ 0 issues
Known Malware	✅ 0 issues
Telemetry	✅ 0 issues
Protestware/Troll package	✅ 0 issues

Bot Commands

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore foo@1.0.0 bar@2.4.2

⚠️ Please accept the latest app permissions to ensure bot commands work properly. Accept the new permissions here.

Powered by socket.dev

IanVS · 2022-10-12T12:46:15Z

@j3rem1e do you have any thoughts about this PR? I'm curious if you can think of any other ways that we can avoid using peer dependencies. There's some related discussion going on in storybookjs/storybook#19385, because I think this is an issue that will come up for other addons as well.

JReinhold

This looks good to me, I think this is the correct way forward!

I changed the base branch to next which caused a few conflicts though. I assumed this only works with the new Vite builder structure in 7.0, but I might be wrong?

JReinhold · 2022-11-21T20:54:18Z

@IanVS do you want to take a look at the merge conflicts, or instead target this back to 6.5? To me it looks like the conflicts are mostly about the upgrade to 7.0.

# Conflicts: # .babelrc.js # package.json # src/preset/index.js # yarn.lock

IanVS · 2022-11-22T13:46:16Z

package.json

    "svelte": "^3.50.0",
    "svelte-jester": "^2.3.2",
    "svelte-loader": "^3.1.2",
-    "typescript": "^3.9.7"
+    "typescript": "^3.9.7",
+    "vite": "^3.1.4"
  },
  "peerDependencies": {
    "@storybook/svelte": ">=6.4.20",
    "@storybook/theming": ">=6.4.20",


Is this correct? Shouldn't these be 7.0?

You're right. Fixing in another PR

IanVS · 2022-11-22T13:47:10Z

Thanks for the nudge @JReinhold, I think this is ready for re-review. I didn't have a chance to test it all out again.

IanVS · 2022-12-07T15:50:34Z

@JReinhold @j3rem1e either of you care to give this a last lookover? Should we try merging and getting things ready on the builder-vite side before the upcoming 7.0 beta release?

shilman · 2023-04-03T12:59:09Z

🚀 PR was released in v3.0.0 🚀

IanVS added 3 commits October 5, 2022 21:18

Add vite support

05a4b17

Fix eslint config

5d94f75

Assign targets to babel/preset-env

062b54a

IanVS requested a review from j3rem1e October 6, 2022 01:29

IanVS requested a review from joshwooding October 6, 2022 01:32

IanVS mentioned this pull request Oct 11, 2022

Addon-docs: Make babel-loader an optional peer dependency storybookjs/storybook#19385

Merged

IanVS requested a review from JReinhold October 27, 2022 15:15

JReinhold changed the base branch from main to next October 28, 2022 08:59

JReinhold approved these changes Oct 28, 2022

View reviewed changes

JReinhold added the enhancement New feature or request label Oct 28, 2022

IanVS added 3 commits November 22, 2022 08:39

Merge remote-tracking branch 'origin/next' into vite-support

df49ef8

# Conflicts: # .babelrc.js # package.json # src/preset/index.js # yarn.lock

Add vite as peer dep

a205ba2

Target node 16

ae738ba

IanVS commented Nov 22, 2022

View reviewed changes

JReinhold added the patch Increment the patch version when merged label Dec 7, 2022

JReinhold merged commit ecaf1b5 into next Dec 7, 2022

JReinhold deleted the vite-support branch December 7, 2022 20:54

shilman added the prerelease This change is available in a prerelease. label Dec 7, 2022

JReinhold mentioned this pull request Dec 7, 2022

Require Storybook v7 in v3 #81

Merged

shilman mentioned this pull request Dec 16, 2022

Support newest ESM-only @sveltejs/vite-plugin #84

Merged

shilman mentioned this pull request Mar 21, 2023

fix: preprocess svelte file during indexing #94

Merged

shilman mentioned this pull request Mar 31, 2023

Expand Storybook version range #95

Merged

shilman added released This issue/pull request has been released. and removed prerelease This change is available in a prerelease. labels Apr 3, 2023

benmccann mentioned this pull request Aug 23, 2023

[Feature] Add Vite-Builder Support #64

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add vite support #72

Add vite support #72

IanVS commented Oct 6, 2022 •

edited by shilman

Loading

socket-security bot commented Oct 6, 2022 •

edited

Loading

IanVS commented Oct 12, 2022

JReinhold left a comment •

edited

Loading

JReinhold commented Nov 21, 2022

IanVS Nov 22, 2022

JReinhold Nov 23, 2022

IanVS commented Nov 22, 2022

IanVS commented Dec 7, 2022

shilman commented Apr 3, 2023

Add vite support #72

Add vite support #72

Conversation

IanVS commented Oct 6, 2022 • edited by shilman Loading

Version

💥 Breaking Change

🐛 Bug Fix

Authors: 2

socket-security bot commented Oct 6, 2022 • edited Loading

Socket Security Pull Request Report

IanVS commented Oct 12, 2022

JReinhold left a comment • edited Loading

Choose a reason for hiding this comment

JReinhold commented Nov 21, 2022

IanVS Nov 22, 2022

Choose a reason for hiding this comment

JReinhold Nov 23, 2022

Choose a reason for hiding this comment

IanVS commented Nov 22, 2022

IanVS commented Dec 7, 2022

shilman commented Apr 3, 2023

IanVS commented Oct 6, 2022 •

edited by shilman

Loading

socket-security bot commented Oct 6, 2022 •

edited

Loading

JReinhold left a comment •

edited

Loading