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

CSF is great, but it lacks an important feature for visual testing #127

Closed
mykas opened this issue May 5, 2022 · 5 comments
Closed

CSF is great, but it lacks an important feature for visual testing #127

mykas opened this issue May 5, 2022 · 5 comments
Labels
enhancement New feature or request

Comments

@mykas
Copy link

mykas commented May 5, 2022

Is your feature request related to a problem? Please describe.

We heavily use Storybook in Wix for screenshot testing our library components. What current storybook supports and will not deprecate until find a solution is storiesOf function where you can generate a bunch of stories by using arrays of objects. With CSF you cannot achieve this.

Consider this story file as a way to render props permutation test stories:

const tests = [
  {
    describe: 'alignItems',
    its: [
      {
        it: 'center',
        props: {
          alignItems: 'center',
        },
      },
      {
        it: 'right',
        props: {
          alignItems: 'right',
        },
      },
      {
        it: 'left',
        props: {
          alignItems: 'left',
        },
      },
    ],
  },
  
  tests.forEach(({ describe, its }) => {
  its.forEach(({ it, props }) => {
    storiesOf(`AddItem${describe ? '/' + describe : ''}`, module).add(
      it,
      () => <AddItem {...defaultProps} {...props} />,
    );
  });
});

This generates 3 stories and adding another one is easy for the next contributor.

Describe the solution you'd like

I think just by supporting similar method like storiesOf ladle could be used for such cases or alternative solution that would support generating bunch of stories in a file based on some data structure.

Describe alternatives you've considered

For now storybook is the clear alternative or we would need to create some sort of story files with CSF generator based on mentioned data structure.

@mykas mykas added the needs triage needs to be reviewed label May 5, 2022
@tajo tajo added enhancement New feature or request and removed needs triage needs to be reviewed labels May 5, 2022
@tajo
Copy link
Owner

tajo commented May 5, 2022

There will be some new development when it comes to test automation with Ladle. Not using storiesOf though.

@quantizor
Copy link

quantizor commented Jun 10, 2022

Came to say the same thing; we have a setup where we autogenerate story variants with different themes, color modes, and sometimes combinations of props that don't need a proper story but should be snapshotted:

export function snapshotVariantsForVisualTesting(stories: DSIStories, options: DSIStoryConfig<any>) {
  stories.forEach(([name, Story]) => {
    // create unique suite instances for each so we can compose decorators per-story
    const suite = storiesOf('Variants', module);

    Story.decorators?.forEach((decorator) => suite.addDecorator(decorator));
    options.decorators?.forEach((decorator) => suite.addDecorator(decorator));

    const baseProps = {
      ...(options.argTypes
        ? Object.entries(options.argTypes).reduce(
            // @ts-ignore
            (map, [key, value]) => ((map[key as keyof Props] = 'action' in value ? value : value.defaultValue), map),
            {} as Record<string, unknown>
          )
        : undefined),
      ...options.args!,
      ...Story.args!,
    };

    const argTypes = { ...options.argTypes, ...Story.argTypes };

    const shouldSnapshotAlternateThemes =
      Story.snapshotAlternateThemes !== undefined ? Story.snapshotAlternateThemes : options.snapshotAlternateThemes;
    const shouldSnapshotDarkMode =
      Story.snapshotDarkMode !== undefined ? Story.snapshotDarkMode : options.snapshotDarkMode;
    const shouldSnapshotPropVariants = (
      Story.snapshotPropVariants !== undefined
        ? { ...options.snapshotPropVariants, ...Story.snapshotPropVariants }
        : options.snapshotPropVariants
    ) as { [key: string]: any[] };

    if (shouldSnapshotPropVariants) {
      Object.entries(shouldSnapshotPropVariants).forEach(([key, value]) => {
        value.forEach((propValue) => {
          suite.add(
            `${name} [prop "${key}" ${propValue}]`,
            (props: any) => (
              <Story
                {...merge(baseProps, props, {
                  [key]: argTypes[key]?.mapping ? argTypes[key]?.mapping[propValue] : propValue,
                })}
              />
            ),
            Story.parameters
          );
        });
      });
    }

    if (shouldSnapshotDarkMode) {
      suite.add(
        `${name} [Dark Mode]`,
        (props: any) => (
          <DarkMode>
            <Story {...merge(baseProps, props)} />
          </DarkMode>
        ),
        Story.parameters
      );

      if (shouldSnapshotPropVariants) {
        Object.entries(shouldSnapshotPropVariants).forEach(([key, value]) => {
          const variants = Array.isArray(value) ? value : ([] as any[]).concat(value);

          variants.forEach((propValue) => {
            suite.add(
              `${name} [Dark Mode] [prop "${key}" ${propValue}]`,
              (props: any) => (
                <DarkMode>
                  <Story
                    {...merge(baseProps, props, {
                      [key]: argTypes[key]?.mapping ? argTypes[key]?.mapping[propValue] : propValue,
                    })}
                  />
                </DarkMode>
              ),
              Story.parameters
            );
          });
        });
      }
    }

    if (shouldSnapshotAlternateThemes) {
      otherThemes.forEach(([themeName, theme]) => {
        suite.add(
          `${name} [${themeName} Theme]`,
          (props: any) => (
            <DSStyleProvider theme={theme}>
              <Story {...merge(baseProps, props)} />
            </DSStyleProvider>
          ),
          Story.parameters
        );

        if (shouldSnapshotDarkMode) {
          suite.add(
            `${name} [${themeName} Theme] [Dark Mode]`,
            (props: any) => (
              <DSStyleProvider theme={theme}>
                <DarkMode>
                  <Story {...merge(baseProps, props)} />
                </DarkMode>
              </DSStyleProvider>
            ),
            Story.parameters
          );
        }
      });
    }
  });
}

A first-party way to do this inside CSF would be fantastic, perhaps an optional transformer on the default export that allows for generation of variants?

@shilman
Copy link

shilman commented Jul 28, 2022

Our rationale for moving away from storiesOf in Storybook is that we can statically analyze CSF, which allows us to (1) generate a list of all the stories without needing to evaluate them, (2) bundle / evaluate stories on demand.

This enables significant optimizations, especially for large storybooks. It also allows us to extract story metadata in Node, which can be brittle in JSDom.

We're also looking at several different alternative approaches to satisfy the use cases listed here and in Storybook's corresponding issue storybookjs/storybook#9828, and will be creating an RFC once we think we've got a suitable answer to storiesOf users.

Here are a few different directions we're considering:

  • programmatically generated stories using a stylized syntax
  • programmatically generated visual tests against a story, similar to how today it's possible to test with multiple viewports, you could also test it in different themes, locales, etc.
  • a stylized way to generate a grid of variants within a single story
  • a programmatic loader similar to @probablyup 's suggestion above, so that e.g. it would be possible to generate stories based on JSON data in fixture files

I'd also love to hear @tajo 's take on things!

@tajo
Copy link
Owner

tajo commented Jul 29, 2022

Our rationale for moving away from storiesOf in Storybook is that we can statically analyze CSF, which allows us to (1) generate a list of all the stories without needing to evaluate them, (2) bundle / evaluate stories on demand.
This enables significant optimizations, especially for large storybooks. It also allows us to extract story metadata in Node, which can be brittle in JSDom.

💯 This is exactly the reason why Ladle doesn't and can't have storiesOf like syntax.

When it comes to snapshot testing including different variants of your stories, you can do it already.

Ladle serializes all its state into the URL including things like controls, theme and RTL. So if you are running some Playwright script opening stories and taking their screenshots, your real problem is to generate URLs exhausting all the stories and their variants - we could provide some utilities for that.

There is one obstacle - args / argTypes are not statically analyzable. I really wish they would be because we could export them as a part of meta.json and that would make it really easy to create those testing URLs. @shilman I guess that's why storybook is exploring some of the mentioned options?

@shilman
Copy link

shilman commented Jul 30, 2022

@tajo That makes a ton of sense. I mentioned collaborating on future iterations of CSF before--perhaps this is an area we can improve in a future iteration. It would be a disruptive change, but if it solves a key architectural problem in both tools and we had a clean solution & migration path, I could definitely be convinced. I think we'll be revisiting argTypes in 7.x, so I'll loop you in once we have something worth discussing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants