Changed in behaviour between v4.2 and v4.3

benedfit commented 2 years ago

The problem

With the release of v4.3, and the speed improvements it brings, we are now experiencing the following issues:

Lazy loaded images, that are off-screen in the initial render of the story, are no longer rendered in snapshots
Dynamic components, that are rendered using useEffect, are no longer rendered in snapshots

Environment

Node version: 16.15.1
@percy/cli version: 1.5.1
Version of Percy SDK you’re using: N/A
If needed, a build or snapshot ID: 19303867
OS version: N/A
Type of shell command-line [interface]: Github Actions CI/CD

Details

Prior to v4.3 we had stories with lazy-loaded images, and dynamic rendered components, and we did not need to wait for specific elements to render before Percy took a snapshot of Storybook

My guess is that the new API no longer waits for all dynamic elements to load before taking a snapshot, or doesn't scroll to the end of the story before taking the snapshot. Any tips for how to handle this?

We have tried using waitForSelector, but as there can be multiple elements on the screen that we need to wait for, this isn't providing a solution. One thought that I has was the additional of a new waitFor function, where we run a more complex query on the output of the page before taking a screenshot (something we do with our Cypress snapshots).

For example, waitFor(() => !document.querySelector([data-dynamic="loading"])); where data-dynamic is an attribute that is toggled between loading and loaded states. Once the page is fully loaded there won't be any elements in this state, and the snapshot can be taken

Debug logs

10_Percy run.txt

Code to reproduce issue

Create a story in Storybook that contains multiple <img loading="lazy" />, some of which are off-screen on initial render. Run percy storybook and notice that any off-screen images are missing in the snapshots

wwilsman commented 2 years ago

Hey @benedfit! I wouldn't expect the changes made to effect lazy loaded images, because in both versions we only wait for network idle (we don't do any scrolling or event checking). Unfortunately, we have other OKR work that is a priority at the moment, but we'll be sure to pick this back up and look at it as soon as we can!

In the meantime, you can always pin the package to a prior version until this issue is updated.

benedfit commented 2 years ago

Thanks @wwilsman, we use Renovate to manage our updates, so can hold off. However we'd originally been interested in upgrading for the performance benefits, as we are currently experiencing regular timeouts when trying to generate snapshots with the current version, and as a result have missed some regressions when the snapshots aren't created. We shall keep waiting

benedfit commented 2 years ago

I managed to get the dynamic elements to display by using enableJavaScript: true (something I hadn't previously had to configure prior to v4.3). But I have run into the following oddity:

Some how, part of the last PNG in one of the stories is missing. Any ideas?

wwilsman commented 2 years ago

Hey @benedfit!

When setting enableJavaScript: true, it will actually skip taking a DOM snapshot of the story and instead use the raw preview.html contents. As for missing assets, this doc might be helpful in explaining what is likely happening.

Before the changes in v4.3, the Story's DOM snapshot was captured after assets were discovered. With the recent performance improvements, the Story's DOM snapshot is now taken before any assets are discovered. So this could be the timing change you are seeing with your DOM not being captured as expected. The network-idle check still happens before capturing though, so increasing the discovery network-idle-timeout could still help.

However, I also just released v4.3.1 which introduces an additional Storybook specific waiting period that might solve this issue. Now in the latest version, we wait for a storyRendered event from Storybook's API before taking the snapshot. However, as I said before, when JS is enabled it will not take a DOM snapshot at all (so it will not wait for this event).

Give the latest version a try and hopefully it helps! 🤞

benedfit commented 3 months ago

Closed as we are no longer using this particular setup

percy / percy-storybook