storybookjs / storybook

Storybook is the industry standard workshop for building, documenting, and testing UI components in isolation
https://storybook.js.org
MIT License
84.01k stars 9.23k forks source link

[6.5.0] Fix peer dependencies #18241

Closed aaronadamsCA closed 1 year ago

aaronadamsCA commented 2 years ago

Describe the bug Upgrading to v6.5.0 emits several new dependency warnings in our repository compared to Storybook 6.4:

➤ │ (my-package) doesn't provide require-from-string, requested by @storybook/react
➤ │ @storybook/addon-docs@npm:6.5.0 doesn't provide @babel/core, requested by @babel/preset-env
➤ │ @storybook/addon-docs@npm:6.5.0 doesn't provide @babel/core, requested by @babel/plugin-transform-react-jsx
➤ │ @storybook/addon-docs@npm:6.5.0 doesn't provide @babel/core, requested by babel-loader
➤ │ @storybook/addon-docs@npm:6.5.0 doesn't provide webpack, requested by babel-loader
➤ │ @storybook/docs-tools@npm:6.5.0 doesn't provide react, requested by @storybook/store
➤ │ @storybook/docs-tools@npm:6.5.0 doesn't provide react-dom, requested by @storybook/store
➤ │ @storybook/mdx1-csf@npm:0.0.1-canary.1.867dcd5.0 doesn't provide @babel/core, requested by @babel/preset-env
➤ │ @storybook/telemetry@npm:6.5.0 doesn't provide react, requested by @storybook/core-common
➤ │ @storybook/telemetry@npm:6.5.0 doesn't provide react-dom, requested by @storybook/core-common

I think this would be fixed with the following changes:

To Reproduce These are our Storybook dependencies today:

    "@storybook/addon-a11y": "v6.5.0",
    "@storybook/addon-essentials": "v6.5.0",
    "@storybook/addon-links": "v6.5.0",
    "@storybook/addons": "v6.5.0",
    "@storybook/builder-webpack5": "v6.5.0",
    "@storybook/manager-webpack5": "v6.5.0",
    "@storybook/react": "v6.5.0",

System

  System:
    OS: Linux 5.4 Ubuntu 22.04 LTS 22.04 (Jammy Jellyfish)
    CPU: (4) x64 Intel(R) Xeon(R) Platinum 8272CL CPU @ 2.60GHz
  Binaries:
    Node: 16.15.0 - /tmp/xfs-966161ed/node
    Yarn: 3.2.1 - /tmp/xfs-966161ed/yarn
    npm: 8.5.5 - /usr/local/share/nvm/versions/node/v16.15.0/bin/npm
nickpresta commented 2 years ago

Just to add, I'm seeing the same thing on 6.5.0 (non-rc):

warning " > @storybook/react@6.5.0" has unmet peer dependency "require-from-string@^2.0.2".
aaronadamsCA commented 2 years ago

I've updated the issue to reflect that these are still concerns in v6.5.0.

GiancarlosIO commented 2 years ago

🙏🏼

SolarLiner commented 2 years ago

It looks like require-from-string is only used in tests which seems to me that the proper resolution for @storybook/react would be to move that dependency to devDependencies.

aaronadamsCA commented 2 years ago

Good call; I've updated the issue.

spqsh commented 1 year ago

Any updates on require-from-string warning? Still getting it on storybook 6.5.14: image

ndelangen commented 1 year ago

require-from-string will be a devDependency in 7.0.0

fasani-tx commented 1 year ago

@ndelangen, what is the timeline for 7.0.0 ?

shilman commented 1 year ago

@fasani-tx it's in beta now and we're planning to release in Q1 next year

MasterOdin commented 1 year ago

Would it be possible to backport the change for require-from-string to 6.5.x, as it should (hopefully) be as simple as doing:

diff --git a/app/react/package.json b/app/react/package.json
index 77b94b7889..0b6bc57ec6 100644
--- a/app/react/package.json
+++ b/app/react/package.json
@@ -85,13 +85,13 @@
   "devDependencies": {
     "@types/util-deprecate": "^1.0.0",
     "jest-specific-snapshot": "^4.0.0",
+    "require-from-string": "^2.0.2",
     "webpack": "4"
   },
   "peerDependencies": {
     "@babel/core": "^7.11.5",
     "react": "^16.8.0 || ^17.0.0 || ^18.0.0",
-    "react-dom": "^16.8.0 || ^17.0.0 || ^18.0.0",
-    "require-from-string": "^2.0.2"
+    "react-dom": "^16.8.0 || ^17.0.0 || ^18.0.0"
   },
   "peerDependenciesMeta": {
     "@babel/core": {
diff --git a/yarn.lock b/yarn.lock
index 72c088e497..88f4ef9f50 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -8370,6 +8370,7 @@ __metadata:
     react-refresh: ^0.11.0
     read-pkg-up: ^7.0.1
     regenerator-runtime: ^0.13.7
+    require-from-string: ^2.0.2
     ts-dedent: ^2.0.0
     util-deprecate: ^1.0.2
     webpack: 4
@@ -8377,7 +8378,6 @@ __metadata:
     "@babel/core": ^7.11.5
     react: ^16.8.0 || ^17.0.0 || ^18.0.0
     react-dom: ^16.8.0 || ^17.0.0 || ^18.0.0
-    require-from-string: ^2.0.2
   peerDependenciesMeta:
     "@babel/core":
       optional: true

I'd make a PR, but I'm not sure what branch that should be made against.

ndelangen commented 1 year ago

@fasani-tx Here's the 7.0 burndown: https://github.com/orgs/storybookjs/projects/8/views/1

MasterOdin commented 1 year ago

@ndelangen It looks like the latest release of 6.5 (6.5.16) still has require-from-string as a peer dependency, though this issue was closed.

Will this dependency issue only be fixed on the 7.x line or can that change be backported? Happy to also open a PR myself with my diff above, though not sure what branch would be appropriate for it.

ndelangen commented 1 year ago

the right branch to target would be main-prerelease