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

Update dependencies to support svelte@4 #112

Merged
merged 1 commit into from
Jul 19, 2023
Merged

Conversation

RSWilli
Copy link
Contributor

@RSWilli RSWilli commented Jul 14, 2023

Closes #109

The included storybook run fine for me locally.

I am not familiar with the build process with github actions, so feel free to edit anything.

@socket-security
Copy link

🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎

To accept the risk, merge this PR and you will not be notified again.

Issue Package Version Note Source
Bin script confusion @nicolo-ribaudo/semver-v6 6.3.3

Next steps

What is bin script confusion?

This package has multiple bin scripts with the same name. This can cause non-deterministic behavior when installing or could be a sign of a supply chain attack

Consider removing one of the conflicting packages. Packages should only export bin scripts with their name

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

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@* or ignore all packages with @SocketSecurity ignore-all

  • @SocketSecurity ignore @nicolo-ribaudo/semver-v6@6.3.3

@RSWilli
Copy link
Contributor Author

RSWilli commented Jul 14, 2023

I do not know if the peer dependency version of storybook should also be ^7.0.27, or if it will be fine this way

@JReinhold JReinhold added the patch Increment the patch version when merged label Jul 19, 2023
@JReinhold JReinhold self-assigned this Jul 19, 2023
@JReinhold JReinhold added the dependencies Update one or more dependencies version label Jul 19, 2023
Copy link
Collaborator

@JReinhold JReinhold left a comment

Choose a reason for hiding this comment

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

This is great, thank you for this! 💪

@JReinhold JReinhold merged commit 67da658 into storybookjs:main Jul 19, 2023
1 check failed
@shilman
Copy link
Member

shilman commented Jul 19, 2023

🚀 PR was released in v3.0.4 🚀

@shilman shilman added the released This issue/pull request has been released. label Jul 19, 2023
@oscard0m
Copy link

Thanks for the great job and for the time invested in this @RSWilli ! 👏🏽


Is it possible this should be using @storybook/*@7.1.0? I see some extra commits supporting Svelte 4 done in that release:


Also, when running my project with v3.0.4 I get the following error which I'm not sure if it's related:

stacktrace

Failed to load preset: {"type":"presets","name":"/Users/XXX/dev/mainmatter/chouchou/node_modules/.pnpm/@storybook+addon-svelte-csf@3.0.4_@storybook+svelte@7.1.0_@storybook+theming@7.0.22_@sveltejs_h2rr5co6q2uverg6qodhdmucde/node_modules/@storybook/addon-svelte-csf/preset.js"} on level 1
ERR! /Users/XXX/dev/mainmatter/chouchou/node_modules/.pnpm/@storybook+addon-svelte-csf@3.0.4_@storybook+svelte@7.1.0_@storybook+theming@7.0.22_@sveltejs_h2rr5co6q2uverg6qodhdmucde/node_modules/@storybook/addon-svelte-csf/dist/cjs/preset/index.js:1
ERR! import { svelteIndexer } from './indexer.js';
ERR! ^^^^^^
ERR!
ERR! SyntaxError: Cannot use import statement outside a module
ERR!     at internalCompileFunction (node:internal/vm:73:18)
ERR!     at wrapSafe (node:internal/modules/cjs/loader:1178:20)
ERR!     at Module._compile (node:internal/modules/cjs/loader:1220:27)
ERR!     at Module._extensions..js (node:internal/modules/cjs/loader:1310:10)
ERR!     at Object.newLoader (/Users/XXX/dev/mainmatter/chouchou/node_modules/.pnpm/esbuild-register@3.4.2_esbuild@0.18.11/node_modules/esbuild-register/dist/node.js:2262:9)
ERR!     at extensions..js (/Users/XXX/dev/mainmatter/chouchou/node_modules/.pnpm/esbuild-register@3.4.2_esbuild@0.18.11/node_modules/esbuild-register/dist/node.js:4807:24)
ERR!     at Module.load (node:internal/modules/cjs/loader:1119:32)
ERR!     at Module._load (node:internal/modules/cjs/loader:960:12)
ERR!     at Module.require (node:internal/modules/cjs/loader:1143:19)
ERR!     at require (node:internal/modules/cjs/helpers:110:18)
ERR!  /Users/XXX/dev/mainmatter/chouchou/node_modules/.pnpm/@storybook+addon-svelte-csf@3.0.4_@storybook+svelte@7.1.0_@storybook+theming@7.0.22_@sveltejs_h2rr5co6q2uverg6qodhdmucde/node_modules/@storybook/addon-svelte-csf/dist/cjs/preset/index.js:1
ERR! import { svelteIndexer } from './indexer.js';
ERR! ^^^^^^
ERR!
ERR! SyntaxError: Cannot use import statement outside a module
ERR!     at internalCompileFunction (node:internal/vm:73:18)
ERR!     at wrapSafe (node:internal/modules/cjs/loader:1178:20)
ERR!     at Module._compile (node:internal/modules/cjs/loader:1220:27)
ERR!     at Module._extensions..js (node:internal/modules/cjs/loader:1310:10)
ERR!     at Object.newLoader (/Users/XXX/dev/mainmatter/chouchou/node_modules/.pnpm/esbuild-register@3.4.2_esbuild@0.18.11/node_modules/esbuild-register/dist/node.js:2262:9)
ERR!     at extensions..js (/Users/XXX/dev/mainmatter/chouchou/node_modules/.pnpm/esbuild-register@3.4.2_esbuild@0.18.11/node_modules/esbuild-register/dist/node.js:4807:24)
ERR!     at Module.load (node:internal/modules/cjs/loader:1119:32)
ERR!     at Module._load (node:internal/modules/cjs/loader:960:12)
ERR!     at Module.require (node:internal/modules/cjs/loader:1143:19)
ERR!     at require (node:internal/modules/cjs/helpers:110:18)

This is my list of dependencies

"devDependencies": {
   "@fontsource-variable/inter": "^5.0.3",
   "@playwright/test": "1.36.1",
   "@storybook/addon-essentials": "^7.1.0",
   "@storybook/addon-interactions": "^7.1.0",
   "@storybook/addon-links": "^7.1.0",
   "@storybook/addon-styling": "^1.3.4",
   "@storybook/addon-svelte-csf": "^3.0.4",
   "@storybook/blocks": "^7.1.0",
   "@storybook/jest": "^0.1.0",
   "@storybook/svelte": "^7.1.0",
   "@storybook/sveltekit": "^7.1.0",
   "@storybook/test-runner": "^0.11.0",
   "@storybook/testing-library": "^0.2.0",
   "@sveltejs/adapter-cloudflare": "^2.3.1",
   "@sveltejs/kit": "1.22.3",
   "@tailwindcss/forms": "^0.5.3",
   "@tailwindcss/typography": "^0.5.9",
   "@types/testing-library__jest-dom": "^5.14.8",
   "@typescript-eslint/eslint-plugin": "5.62.0",
   "@typescript-eslint/parser": "5.62.0",
   "autoprefixer": "10.4.14",
   "chromatic": "^6.19.8",
   "concurrently": "^8.2.0",
   "eslint": "8.45.0",
   "eslint-config-prettier": "8.8.0",
   "eslint-plugin-playwright": "^0.15.1",
   "eslint-plugin-promise": "^6.1.1",
   "eslint-plugin-storybook": "^0.6.12",
   "eslint-plugin-svelte": "2.32.2",
   "postcss": "8.4.26",
   "postcss-import": "^15.1.0",
   "postcss-load-config": "4.0.1",
   "prettier": "2.8.8",
   "prettier-plugin-jsdoc": "^0.4.2",
   "prettier-plugin-svelte": "2.10.1",
   "prettier-plugin-tailwindcss": "^0.4.0",
   "react": "^18.2.0",
   "react-dom": "^18.2.0",
   "storybook": "^7.1.0",
   "svelte": "4.0.5",
   "svelte-check": "3.4.6",
   "tailwindcss": "3.3.3",
   "tslib": "2.6.0",
   "typescript": "5.1.6",
   "vite": "4.4.4",
   "vitest": "0.33.0"
},

Let me know if this relates to this issue or if I need to open a new one with more information.

Again, thanks for the effort and giving back to the community ❤️

@RSWilli
Copy link
Contributor Author

RSWilli commented Jul 19, 2023

@oscard0m I am getting the same error here, I think this happens because the types are now svelte 4 typings, which are incompatible with the svelte 3 typings in my project

I think this needs a new PR, if we want to support both svelte 3 and 4

@oscard0m
Copy link

@oscard0m I am getting the same error here, I think this happens because the types are now svelte 4 typings, which are incompatible with the svelte 3 typings in my project

Thanks for the quick answer and for giving some light here @RSWilli!

I think this needs a new PR, if we want to support both svelte 3 and 4

I'm not sure what are the steps to follow:

  • Do we want addon-svelte-csf to be retro-compatible with Svelte 3? In my mind, this should be a major release, and users can stay with in the previous version, which supports Svelte v3
  • Do you know What is the versioning criteria for this addon? @JReinhold

@joelmukuthu
Copy link

Hey @RSWilli @oscard0m, I think the syntax errors are unrelated: #111. They've existed since v3.0.0. so I don't think they're coming from this PR

@JReinhold
Copy link
Collaborator

I think @joelmukuthu is correct, that what you're seeing @oscard0m is #111.

I think this happens because the types are now svelte 4 typings, which are incompatible with the svelte 3 typings in my project

@RSWilli I don't think the typings should be breaking, because we did the same changes in core without breaking anything. But if they indeed are, then we should revert this PR and release it as a major instead.

@benpaquier
Copy link

First of all, great job 👏

@oscard0m, did you manage to make it work ? I've got the exact same error as you:

/Users/*/*/*/node_modules/@storybook/addon-svelte-csf/dist/cjs/preset/index.js:1
import { svelteIndexer } from './indexer.js';
^^^^^^
SyntaxError: Cannot use import statement outside a module
    at internalCompileFunction (node:internal/vm:73:18)
    at wrapSafe (node:internal/modules/cjs/loader:1195:20)
    at Module._compile (node:internal/modules/cjs/loader:1239:27)
    at Module._extensions..js (node:internal/modules/cjs/loader:1329:10)
    at Object.newLoader (/Users/*/*/*/node_modules/esbuild-register/dist/node.js:2262:9)
    at extensions..js (/Users/*/*/*/node_modules/esbuild-register/dist/node.js:4807:24)
    at Module.load (node:internal/modules/cjs/loader:1133:32)
    at Module._load (node:internal/modules/cjs/loader:972:12)
    at Module.require (node:internal/modules/cjs/loader:1157:19)
    at require (node:internal/modules/helpers:119:18)
 /Users/*/*/*/node_modules/@storybook/addon-svelte-csf/dist/cjs/preset/index.js:1

And here are my dependencies:

"peerDependencies": {
    "svelte": "^4.0.0"
  },
  "devDependencies": {
    "@storybook/addon-essentials": "^7.1.1",
    "@storybook/addon-interactions": "^7.1.1",
    "@storybook/addon-links": "^7.1.1",
    "@storybook/addon-svelte-csf": "^3.0.4",
    "@storybook/blocks": "^7.1.1",
    "@storybook/svelte": "^7.1.1",
    "@storybook/sveltekit": "^7.1.1",
    "@storybook/testing-library": "^0.2.0",
    "@sveltejs/adapter-auto": "^2.0.0",
    "@sveltejs/kit": "^1.20.4",
    "@sveltejs/package": "^2.0.0",
    "@typescript-eslint/eslint-plugin": "^5.45.0",
    "@typescript-eslint/parser": "^5.45.0",
    "eslint": "^8.28.0",
    "eslint-config-prettier": "^8.5.0",
    "eslint-plugin-storybook": "^0.6.13",
    "eslint-plugin-svelte": "^2.30.0",
    "prettier": "^2.8.0",
    "prettier-plugin-svelte": "^2.10.1",
    "publint": "^0.1.9",
    "react": "^18.2.0",
    "react-dom": "^18.2.0",
    "storybook": "^7.1.1",
    "svelte": "^4.0.5",
    "svelte-check": "^3.4.3",
    "tslib": "^2.4.1",
    "typescript": "^5.0.0",
    "vite": "^4.4.2"
  },

Thanks in advance, cheers !

@JReinhold
Copy link
Collaborator

JReinhold commented Jul 28, 2023

@benpaquier the error you're seeing is #111, which comes from Storybook v7.1. I suggest you try pinning your Storybook dependencies to version 7.0.27, or pin this addon to version 3.0.5--canary.113.fa5505b.0, which is a temporary canary release of #113 that attempts to fix this.

(if using the canary version, it would be great if you could report on #113 if it works for you or not)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Update one or more dependencies version patch Increment the patch version when merged released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Support of Svelte 4
6 participants