happo / happo-plugin-storybook

A happo.io plugin for Storybook
MIT License
11 stars 5 forks source link

Allow setting "skippedExamples" for unchanged stories #118

Closed rafael-anachoreta closed 5 months ago

rafael-anachoreta commented 5 months ago

Hi, @trotzig!

The idea behind this request is the same as https://github.com/happo/happo-e2e/issues/21. Basically, we have a few monorepo projects with several hundred stories, and we would like to be able to mark some of them as skipped for a given run. This would give us the flexibility we need to add Happo to new projects without having to worry about spurious differences from one team blocking another, for instance.

Is this something you agree would make sense? Please let me know!

trotzig commented 5 months ago

Hi @rafael-anachoreta! This definitely makes sense. Do you know how you would figure out which stories should be skipped? I'm asking to figure out what would be a good interface here. Let's say we allowed something like this:

const happoPluginStorybook = require('happo-plugin-storybook');
module.exports = {
  plugins: [
    happoPluginStorybook({
      skip: ['Button', 'Card'], 
    }),
  ],
};

When Happo is iterating through stories, we can skip the stories that you defined in the array.

Figuring out what to skip would probably require some async stuff to happen, in which case we could also allow a function to resolve with an array, like this:

const happoPluginStorybook = require('happo-plugin-storybook');
module.exports = {
  plugins: [
    happoPluginStorybook({
      skip: async () => {
        const toSkip = await findSkippedStories();
        return toSkip; 
      }
    }),
  ],
};

Another alternative would be to allow a skip parameter to a story. Something like

export default {
  title: 'FooComponent',
  parameters: {
    happo: 'skip', // this will skip generating screenshots for all `FooComponent` stories
  },
};

const WithBorder = () => <FooComponent bordered />;

WithBorder.parameters = {
  happo: 'skip', // this will skip making screenshots for the `WithBorder` story but still keep it in the report
};

export { WithBorder };

Similar to https://docs.happo.io/docs/storybook#disabling-a-story

Let me know what you think! The infrastructure is mostly there so it won't take long to adapt to Storybook as well.

rafael-anachoreta commented 5 months ago

Do you know how you would figure out which stories should be skipped?

We would probably use something like NX's affected feature, though that remains to be seen for sure. We will most likely compile a list of Stories to be skipped by their full name, keeping in mind the possible structure and hierarchy, or just by their relative file path.

I think your suggestion below would be perfect for us.

const happoPluginStorybook = require('happo-plugin-storybook');
module.exports = {
  plugins: [
    happoPluginStorybook({
      skip: async () => {
        const toSkip = await findSkippedStories();
        return toSkip; 
      }
    }),
  ],
};

Maybe we can then use the same interface we have for the e2e tests?

type SkippedExamples = {
  component: string
  variant: string
  target: string
}[]

Do you foresee any issues with that, @trotzig ?

trotzig commented 5 months ago

Thanks!

Yeah that makes sense, although I wonder if we can skip target here and instead assume it's skipped for all targets. This is how I envision implementing this:

I'm making the assumption that even if things are skipped, we still build a full Storybook. If you think the Storybook build will be a subset variant, we probably have to come up with a slightly alternative solution.

rafael-anachoreta commented 5 months ago

I think we can start with this assumption, as this is indeed how we currently run stories!

trotzig commented 5 months ago

I've pushed a first version of this support in 4.3.0: https://github.com/happo/happo-plugin-storybook/releases/tag/v4.3.0

So far this only works in Chrome and Firefox but other targets will be updated in the coming week.

trotzig commented 5 months ago

It would be interesting to understand the full process of figuring out what examples to skip. If you make a blog post or something, I can link our docs.happo.io docs to the post! 🙏

rafael-anachoreta commented 1 month ago

Hey @trotzig , we just ran into an issue where the skipped logic was running for the main branch and leading to all sorts of problems with the baseline. I understand we can update the skip call to account for that, but I was thinking it might make sense for skip to not run on Happo's side when we know we are on the main/master branch of the project

What are your thoughts on this?