percy / percy-storybook

Percy's Storybook SDK.
https://docs.percy.io/docs/storybook
MIT License
150 stars 45 forks source link

Storybook v7 support #715

Closed davidandrewfrey closed 1 year ago

davidandrewfrey commented 1 year ago

The problem

Storybook v7 support.

We recently updated to using Vite and made the switch to Storybook v7.

Would very much like to included Percy in our stack, but Storybook support is limited to v6.

Environment

Details

N/A

Debug logs

N/A

Code to reproduce issue

N/A

itsjwala commented 1 year ago

@davidandrewfrey I don't think so this is related to storybook v7, since we've few customers using Percy storybook with storybook v7.

There might be some integration errors with Vite <> Storybook V7. I'd like to know some more details to help.

dyaa commented 1 year ago

I had a similar issue when enabling the storyStoreV7 feature, is that your case? https://storybook.js.org/docs/react/configure/overview#feature-flags

davidandrewfrey commented 1 year ago

@itsjwala @dyaa Thank you for the info – apologies for the delayed reply.

That's good news about the other customers using Percy with Storybook v7.

We're going to be digging into this issue more next week and I'll post updates as they become available.

davidandrewfrey commented 1 year ago

@itsjwala I think your assessment of Vite <> Storybook v7 is likely the case. We upgraded from Storybook to 7.0.0-beta.48 to 7.0.0-beta.63 and got closer to Percy happiness. With this latest Storybook v7 update Percy would complete a dry run, but barfed on the actual build. Terminal output below.

@dyaa Thanks for the suggestion. I tried the storyStoreV7 flag but it made no difference.

Storybook v7 is rapidly evolving, so it probably makes the most sense for us to wait for another week or two and try our luck again with a future version of Storybook v7.

What we're using @percy/cli 1.20.3 @percy/storybook 4.3.5 react 18.2.0 vite 4.1.1

Terminal output from the latest attempt

> percy storybook http://localhost:6006 --verbose

[percy:config] Found config file: .percy.json (0ms)
[percy:config] Using config:
{
  version: 2,
  snapshot: {
    widths: [
      1280
    ],
    minHeight: 1024,
    percyCSS: 'iframe {\n  display: none;\n}\n'
  },
  discovery: {
    networkIdleTimeout: 150,
    disableCache: false,
    concurrency: 15
  },
  upload: {
    files: '**/*.{png,jpg,jpeg}',
    ignore: ''
  },
  storybook: {
    waitForTimeout: 1,
    waitForSelector: '',
    include: [
      '/Components/Button/'
    ]
  }
} (20ms)
[percy:storybook] Requesting Storybook: http://localhost:6006 (15ms)
[percy:core:browser] Launching browser (204ms)
[percy:core:browser] Browser connected [27138]: HeadlessChrome/96.0.4664.0 (715ms)
[percy:core] Percy has started! (1ms)
[percy:core:page] Page created (6ms)
[percy:core:page] Page created (1ms)
[percy:core:page] Navigate to: http://localhost:6006/?path=/settings/about (168ms)
[percy:core:page] Navigate to: http://localhost:6006/iframe.html (1ms)
[percy:core:page] Page navigated (526ms)
[percy:core:page] Page navigated (482ms)
[percy:core:page] Page closed (6ms)
[percy:core:page] Page closed (319ms)
[percy:storybook:config] Skipping story: Components/Forms/Textarea: Default (0ms)
[percy:storybook:config] Skipping story: Components/Forms/Textarea: Errored (0ms)
[percy:storybook:config] Skipping story: Components/Forms/Textarea: Disabled (0ms)
[percy:storybook:config] Skipping story: Components/Forms/Textarea: Placeholder (0ms)
[percy:storybook:config] Skipping story: Components/Icon: All Props (0ms)
[percy:storybook:config] Skipping story: Components/Icon: Default (0ms)
[percy:storybook:config] Skipping story: Components/Icon: Custom Icon (0ms)
[percy:storybook:config] Skipping story: Components/Modal: All Props (0ms)
[percy:storybook:config] Skipping story: Components/Modal: Content Component (0ms)
[percy:storybook:config] Skipping story: Components/Modal: Warn (1ms)
[percy:storybook:config] Skipping story: Components/Modal: Danger (0ms)
[percy:core:page] Page created (2ms)
[percy:core:page] Navigate to: http://localhost:6006/iframe.html (33ms)
[percy:core:page] Page navigated (289ms)
[percy:core:page] Page closed (13ms)
[percy:core] Stopping percy... (0ms)
[percy:core:browser] Closing browser (0ms)
[percy:core:browser] Browser closed (456ms)
[percy:core] Build not created (0ms)
[percy:cli] TypeError: Cannot read properties of undefined (reading 'message')
    at withPage (file:///Users/david.frey/Documents/WebStorm/conduit/node_modules/@percy/storybook/dist/utils.js:108:33)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async takeStorybookSnapshots (file:///Users/david.frey/Documents/WebStorm/conduit/node_modules/@percy/storybook/dist/snapshots.js:187:5)
    at async Object.callback (file:///Users/david.frey/Documents/WebStorm/conduit/node_modules/@percy/storybook/dist/storybook.js:36:3)
    at async generatePromise (file:///Users/david.frey/Documents/WebStorm/conduit/node_modules/@percy/core/dist/utils.js:85:115)
    at async runCommandWithContext (file:///Users/david.frey/Documents/WebStorm/conduit/node_modules/@percy/cli-command/dist/command.js:132:3)
    at async percy (file:///Users/david.frey/Documents/WebStorm/conduit/node_modules/@percy/cli-command/dist/command.js:162:9)
    at async /Users/david.frey/Documents/WebStorm/conduit/node_modules/@percy/cli/bin/run.cjs:13:5 (0ms)
cgaube commented 1 year ago

Storybook 7 is getting a release candidate really soon https://github.com/storybookjs/storybook/releases/tag/v7.0.0-rc.0

so i don't foresee storyStoreV7 to change much more

FYI I'm getting the same error message TypeError: Cannot read properties of undefined (reading 'message') Setting storyStoreV7 to false makes it work - which is not ideal as none of my storybook .mdx stories are used with that configuration Which means i need two different storybook build processes - one for Percy with storyStoreV7= false and my "normal" one with storyStoreV7=true

cgaube commented 1 year ago

on the release candidate v7.0.0-rc.0 i get a different error message

  | [percy] Downloading Chromium 929511...
  | [percy] Successfully downloaded Chromium 929511
  | [percy] Percy has started!
  | [percy] Stopping percy...
  | [percy] Build not created
  | [percy] Error: Storybook object not found on the window. Open Storybook and check the console for errors.
  | ELIFECYCLE  Command failed with exit code 1.
  |  

and this is happening because the RC show a warning on .mdx files when storyStoreV7=false

itsjwala commented 1 year ago

I've not dived into the storybook v7 implementation yet, I think they've likely updated their global story store object we depend on this object to interact with storybook - ref

We'd want them to release a stable release for v7 first before making a change in Percy Storybook SDK.

I'll go ahead and add this to our feature request list, we'll prioritize and update this thread if/when we pick this up.

cgaube commented 1 year ago

Ok my previous error message was still related to storyStoreV7 with RC i need to exclude all .mdx from the build so that percy/storybook can work

so please ignore my last message
Sorry about that

.storybook/main.ts. (exclude mdx files)

  // stories: ['../src/**/*.mdx', '../src/**/*.stories.ts'],
  // replaced with 
  stories: ['../src/**/*.stories.ts'],

and

features: {
    storyStoreV7: false
  },

still not ideal :)

cgaube commented 1 year ago

For the actual bug I was able to investigate a bit more and it looks like there is an error being thrown But somehow error is undefined and this piece of code then fail https://github.com/percy/percy-storybook/blob/master/src/utils.js#L130

Screenshot 2023-03-10 at 09 05 20

so it is hiding the real issue. which I have not been able to debug yet. - hopefully it is something very simple to fix

czabaj commented 1 year ago

I have the same problem, the error at this line is undefined so it fails with a TypeError: Cannot read properties of undefined (reading 'message')

https://github.com/percy/percy-storybook/blob/e2673c460ea455ff0e497c04fbdf434ae8d84867/src/utils.js#L130

setting storyStoreV7 : false breaks the Storybook for me, so I was trying to debug @percy/storybook instead and I chased the error to this line, where it delegates to the eval which should do waitFor and I'm lost in all that async logic and different contexts.

https://github.com/percy/percy-storybook/blob/e2673c460ea455ff0e497c04fbdf434ae8d84867/src/snapshots.js#L205

But when I debug the code with debugger attached to the node process, I'm receiving different error

Error: Protocol error (Runtime.callFunctionOn): Cannot find context with specified id

This seems to be a puppeteer error and I think it is not related with the root cause - the --inspect-brk probably causes the runtime to choke on something else.

cgaube commented 1 year ago

Little update: I recently updated to storybook 7.02 https://github.com/storybookjs/storybook/releases/tag/v7.0.2 and added enableJavascript option to the percy conf file

# https://docs.percy.io/docs/cli-configuration
version: 2
snapshot:
  widths:
    - 375
    - 1280
  minHeight: 1024
  percyCSS: ''
  disableShadowDOM: false
  enableJavascript: true. <--- 

and was able to use percy/storybook without the workarounds related to storyStoreV7: false

the only other thing i changed as well was using yarn (3) instead Pnpm but i doubt it matters (i really hope it does not :| )

The only issue is that by enabling javascript - some animation via css get flagged as visual changes which i am sure i can disable thanks to percyCSS (or it might be tricky because i am using shadowRoot :| ) https://docs.percy.io/docs/animations

czabaj commented 1 year ago

Little update from me

using

// .storybook/main.ts
{ 
 features: {
    storyStoreV7: false,
  },
}

blows during Storybook static assets build on

FATAL ERROR: Reached heap limit Allocation failed - JavaScript heap out of memory

setting env NODE_OPTIONS=--max-old-space-size=8192 helps, the storybook builds, it works served with local web-server but Percy fails to load iframe

[percy:core:page] Navigate to: http://localhost:55493/iframe.html (78ms)
[percy:core:page] Page closed (30007ms)
[percy:core] Stopping percy... (1ms)
[percy:core:browser] Closing browser (0ms)
[percy:core:browser] Browser closed (32ms)
[percy:core] Build not created (0ms)
[percy:cli] Error: Navigation failed: Timed out waiting for the page load event

If I keep enabled the storyStoreV7 and use

# .percy.yaml
snapshot:
  enableJavascript: true

Then snapshots are created and sent to Percy app, but all contain just an error page with messages like

Error: at PreviewWeb.getStoryIndexFromServer (http://render.percy.local/sb-preview/runtime.mjs:80:1192)

Or

@http://render.percy.local/sb-preview/runtime.mjs:80:1201
asyncFunctionResume@[native code]
@[native code]
promiseReactionJobWithoutPromise@[native code]
promiseReactionJob@[native code]
braddialpad commented 1 year ago

Percy and storybook 7 is working for me, but how do you snapshot a docs page in storybook 7?

queryParams: {
  viewMode: 'docs',
},

doesn't work anymore

csantos1113 commented 1 year ago

For the actual bug I was able to investigate a bit more and it looks like there is an error being thrown But somehow error is undefined and this piece of code then fail https://github.com/percy/percy-storybook/blob/master/src/utils.js#L130

Screenshot 2023-03-10 at 09 05 20

so it is hiding the real issue. which I have not been able to debug yet. - hopefully it is something very simple to fix

I'm facing the same error:

[percy:cli] TypeError: Cannot read properties of undefined (reading 'message')

I also set enableJavascript: true but no luck!

Yama-Tomo commented 1 year ago

https://github.com/percy/percy-storybook/issues/715#issuecomment-1508268359 same issue.

I found a workaround for that issue.

  1. set storyStoreV7 to false (only build)

    // .storybook/main.js
    module.exports = {
    features: {
    // disable `storyStorev7` when storybook build
    storyStoreV7: process.env.NODE_ENV !== 'production',
    },
    ...
    }
  2. run percy storybook with url

    port=8080
    npx http-server -s -p ${port} storybook-static &
    yarn storybook build
    yarn percy storybook http://$(hostname):${port}

    The "$(hostname)" part of the URL is important. Strangely, specifying localhost or 127.0.0.1 will result in repeated requests for specific assets (e.g. /sb-preview/runtime.js, /iframe.html, etc.) This results in a "Error: Navigation failed: Timed out waiting for the page load event" error. (This does not seem to be a problem if the domain is not localhost or 127.0.0.1)

version

tomekbuszewski commented 1 year ago

I am also having this issue. While disabling storyStoreV7 works, it feels like a hack.

alextreppass commented 1 year ago

https://github.com/percy/percy-storybook/issues/751 - same problem, I traced it down to an issue with evalSetCurrentStory. See that issue for a deeper diagnosis.

Any timeline on proper storybook 7 support from Percy here? We pay a lot for Percy, and downgrading to SB 6 isn't something we'd like to do. Thanks

alextreppass commented 1 year ago

Hi team, any update on this please?

itsjwala commented 1 year ago

Hey folks!

Apologies for the delays in an update here, we've prioritized storybook V7 support, it's part of our roadmap for the coming quarter (Oct-Dec)

nilshah98 commented 1 year ago

Hey folks,

A quick update here. We have shipped percy/storybook v5.0.0-beta.1 SDK.

In this thread, there are 2 major issues that people have posted -

Setting storyStoreV7: true either breaks their percy builds or shows incorrect snapshots

  • We have tried and fixed this error in the new released SDK.

[percy:cli] TypeError: Cannot read properties of undefined (reading 'message') breaks their percy build

  • We are unable to reproduce this error, but we have tried enabling better error messages for this.
  • If someone could provide us with a reproducible example for this error, that would be great!

That being said, do try out the new SDK, and provide us feedback. Till stable release is done, there could be other issues that might crop (though we've done good enough sanity for this beta release), please do create separate issues if so for the same.

Thanks.

alextreppass commented 1 year ago

Thanks - this has allowed us to remove our workarounds, and it seems stable so far 🎉

nilshah98 commented 1 year ago

Hello,

We've released version 5.0.0 of the percy/storybook SDK, which is expected to address the reported issue. Consequently, we are closing this matter. If you continue to experience the problem, please don't hesitate to open a new issue.

Thank you.