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

issue with Storybook 6.5 #567

Closed
okize opened this issue May 18, 2022 · 14 comments · Fixed by #573
Closed

issue with Storybook 6.5 #567

okize opened this issue May 18, 2022 · 14 comments · Fixed by #573
Labels
🐛 bug Something isn't working

Comments

@okize
Copy link

okize commented May 18, 2022

hit this error in Ci after upgrading @storybook/cli to 6.5:

[percy:cli] file:///home/runner/work/.../node_modules/@percy/storybook/dist/snapshots.js:2
import { buildArgsParam } from '@storybook/router/utils.js';
         ^^^^^^^^^^^^^^
SyntaxError: Named export 'buildArgsParam' not found. The requested module '@storybook/router/utils.js' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from '@storybook/router/utils.js';
const { buildArgsParam } = pkg;

    at ModuleJob._instantiate (node:internal/modules/esm/module_job:124:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:179:5)
    at async Loader.import (node:internal/modules/esm/loader:178:24)
    at async Object.callback (file:///home/runner/work/vhs/vhs/node_modules/@percy/storybook/dist/storybook.js:31:7)
    at async generatePromise (file:///home/runner/work/vhs/vhs/node_modules/@percy/core/dist/utils.js:61:196)
    at async runCommandWithContext (file:///home/runner/work/vhs/vhs/node_modules/@percy/cli-command/dist/command.js:120:3)
    at async percy (file:///home/runner/work/vhs/vhs/node_modules/@percy/cli-command/dist/command.js:153:9)
    at async /home/runner/work/.../node_modules/@percy/cli/bin/run.cjs:13:5 (14ms)
error Command failed with exit code 1.

I didn't find any use of buildArgsParam in @storybook/cli but I do see it in this repo - https://github.com/percy/percy-storybook/blob/master/src/snapshots.js#L2. Apologies if this issue should have been opened elsewhere.

@jjh-grio
Copy link

Seeing this as well, here are the versions we are using. So it may not be due to 6.5:

"@percy/cli": "1.2.1",
"@percy/puppeteer": "2.0.1",
"@percy/storybook": "4.2.0",
"@storybook/addon-a11y": "6.4.22",
"@storybook/addon-actions": "6.4.22",
"@storybook/addon-docs": "6.4.22",
"@storybook/addon-essentials": "6.4.22",
"@storybook/addon-links": "6.4.22",
"@storybook/builder-webpack5": "6.4.22",
"@storybook/manager-webpack5": "6.4.22",
"@storybook/node-logger": "6.4.22",
"@storybook/preset-create-react-app": "4.1.0",
"@storybook/react": "6.4.22",
"storybook-dark-mode": "1.1.0"

@lukedigby
Copy link

lukedigby commented May 20, 2022

Does anyone have any more info on how to fix this?

Following the example from the docs - https://docs.percy.io/docs/storybook - I get the same error when running percy storybook http://localhost:3000

@kevinmpowell
Copy link

I'm also having this issue. I don't know the fix. Here are my versions:

"@babel/core": "^7.17.12",
"@percy/cli": "^1.2.1",
"@percy/storybook": "^4.2.0",
"@storybook/addon-actions": "^6.5.2",
"@storybook/addon-essentials": "^6.5.2",
"@storybook/addon-interactions": "^6.5.2",
"@storybook/addon-links": "^6.5.2",
"@storybook/builder-webpack4": "^6.5.2",
"@storybook/manager-webpack4": "^6.5.2",
"@storybook/react": "^6.5.2",
"@storybook/testing-library": "^0.0.11",
"babel-loader": "^8.2.5",
"default-browser-id": "^2.0.0",
"react": "^16.9.0",
"react-dom": "^16.9.0"

@Robdel12 Robdel12 added the 🐛 bug Something isn't working label May 23, 2022
@Robdel12
Copy link
Contributor

👋🏼 hey everyone! We’ll have a fix for this out this week — we’re going to be shipping a performance feature that eliminates this dependency

@okize
Copy link
Author

okize commented May 26, 2022

@Robdel12 I can confirm this fixed the issue for me, thanks so much!

@fernandopasik
Copy link

👋🏼 hey everyone! We’ll have a fix for this out this week — we’re going to be shipping a performance feature that eliminates this dependency

Sorry that is not related to this issue but are there more details into what performance improvements can we gain?

@Robdel12
Copy link
Contributor

@fernandopasik we haven’t shipped that yet, it’s taking a bit of extra time to get the test coverage we need.

Basically we’re separating the DOM capture from asset discovery so asset discovery doesn’t have to process JS requests for each snapshot. There’s some customer storybook builds that have over 350 JS requests per-story so this is going to be a massive speed increase for them.

And then after that in the future we’re going to be adding parallel sharding for those who need JS enabled and have large/heavy storybook stories.

@fernandopasik
Copy link

That sounds good. My company is a customer too and we have been experiencing not great times running all the snapshots. Excited to look what you are about to deliver!

@fernandopasik
Copy link

I'm not so sure about parallel processing because CI usually doesn't offer many cpus per machine.

@fernandopasik
Copy link

👋🏼 hey everyone! We’ll have a fix for this out this week — we’re going to be shipping a performance feature that eliminates this dependency

Has this landed? Is there an issue to follow on this? @Robdel12

@passbyval
Copy link

Upgrading to:

"@percy/cli": "^1.3.1",
"@percy/storybook": "4.2.1",

seems to have fixed the issue for me.

@fernandopasik
Copy link

fernandopasik commented Jun 28, 2022

👋🏼 hey everyone! We’ll have a fix for this out this week — we’re going to be shipping a performance feature that eliminates this dependency

I noticed this update #592 but that's not removing @storybook/router yet, is there another update coming @Robdel12 ?

@Robdel12
Copy link
Contributor

Storybook has a minefield of breaking changes between patch versions so we weren't able to get backward compatibility without keeping it: #592 (comment)

The performance update was shipped: https://github.com/percy/percy-storybook/releases/tag/v4.3.0

@fernandopasik
Copy link

Great thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants