chromaui / storybook-addon-pseudo-states

CSS pseudo-classes for Storybook
MIT License
86 stars 28 forks source link

Pseudo classes persist when switching stories under load or fast #90

Open ConcernedHobbit opened 10 months ago

ConcernedHobbit commented 10 months ago

Pseudo classes seem to take some time to be removed from the DOM. I'm running into an issue where switching a story within the same component causes pseudo states from the previous story to still be present. We can also reliably trigger this with test-runner enabled visual snapshot tests. As the story switches happen very fast, this addon does not react in time to the changes.

This regression was introduced because of a Storybook change sometime between 7.1 and 7.4. I speculate it is because of a race condition with event firing, but do not have time to investigate further.

We've manually cleaned up the leftover classes in visual testing, in test-runner's postRender hook:

if (!parameters.pseudo) {
    const body = await page.$('body');
    await body?.evaluate((node) => {
        const pseudoedElements = node.querySelectorAll('*[class*="pseudo-"]');
        for (const element of pseudoedElements) {
            element.classList.remove(...Array.from(element.classList).filter((c) => c.startsWith('pseudo-')));
        }
    });
}

This occurs before taking a snapshot, and allows us flake-free visual tests.

Let me know if I can assist in debugging! :)

drewbrend commented 9 months ago

+1

I also hit this in my stories when running visual tests with applitools, but I haven't been able to make a simple reproduction so I didn't open an issue. I've had to disable visual testing on some stories because I haven't been able to resolve this.

codingedgar commented 3 months ago

It seems to happen when switching from, to and between stories with pseudo, the configuration persists.

codingedgar commented 3 months ago

It still happens in 3.1.0; it is not an issue with chromatic, only during manual testing.

codingedgar commented 3 months ago

Downloaded and tested the project, works fine, maybe is an issue with Vite in my configuration?

/// <reference types="vitest" />
import react from '@vitejs/plugin-react';
import { defineConfig } from 'vite';
import sassDts from 'vite-plugin-sass-dts';
import { version } from '../../release/app/package.json';
import { srcRendererPath } from './vite.paths';

export default defineConfig({
  mode: process.env.NODE_ENV === 'production' ? 'production' : 'development',
  root: srcRendererPath,
  cacheDir: 'node_modules/.vite_storybook',
  define: {
    'process.env.BUILD_TYPE': JSON.stringify(process.env.BUILD_TYPE),
    'process.env.VERSION': JSON.stringify(version),
  },
  plugins: [react(), sassDts()],
});
codingedgar commented 3 months ago

It seems the bug happens when I use CSS Selectors:

parameters = { pseudo: { hover: ["button"] } }

codingedgar commented 3 months ago

Seems

https://github.com/chromaui/storybook-addon-pseudo-states/blob/cab0bc6e8f944f0915cded5158d4635d9dedbc23/src/preview/withPseudoState.ts#L162-L172

runs twice,

first with old parameters and second with new parameters

The first time, it adds pseudo-classes.

If it ran once, with the latest parameters, the problem wouldn't exist, but I'm not sure how to fix that or if it should be ad-hoc fixed here.

Another approach

If there was a way to remove cleanup previous classes before adding new, that would also work, I think.

https://github.com/chromaui/storybook-addon-pseudo-states/blob/cab0bc6e8f944f0915cded5158d4635d9dedbc23/src/preview/withPseudoState.ts#L49-L79