Storybook: Enable snapshot property matcher functionality for addon-storyshots

[tobilen]

If you are creating a issue to track work to be completed, start here:

Work summary

With jest 23, snapshot property matchers were introduced as a way to exempt props from causing red snapshots. It is summarized best by this tweet: https://twitter.com/fbjest/status/999733224178896896?lang=en

There is currently no way to make use of this functionality while using the snapshot testing functions exported by addon-storyshots.

Where to start

I would like to add an additional parameter to snapshotWithOptions and multiSnapshotWithOptions that gets passed through as an argument to toMatchSnapshot and toMatchSpecificSnapshotrespectively. This argument can then be used to pass the required property matchers to jest

example usage:

import initStoryshots, {
  multiSnapshotWithOptions,
} from '@storybook/addon-storyshots';

initStoryshots({
  framework: 'react',
  storyKindRegex: /^((?!.*?ErrorBoundary).)*$/,
  test: multiSnapshotWithOptions({
    propertyMatchers: { timestamp: expect.any(Number) },
  }),
});

Acceptance criteria

• [ ] Snapshot property matchers can be passed via snapshotWithOptions and multiSnapshotWithOptions
• [ ] Passed property matchers are respected by the storyshots test

Who to contact

@tobilen

[shuklajay117]

I would like to work on this. This will be my first PR so can you please help me a bit around where to start? I go through the issue and I am clear about what is needed but still if you suggest some points to keep in mind, that would be appreciated!

[tobilen]

hey @shuklajay117,

really great that you want to tackle this problem!

the biggest problem will probably be getting the property matchers to apply to the "objects" (i.e. react components) coming in. i would start by getting them to work in a local enviroment with react-test-renderer to get an understanding of how they work when combined with components.

when you have a solid handle on that, try matching to objects returned from the enzyme renderer. you probably have to serialize them first (storybook uses the enzyme-to-json serializer by default, so i would try with that one). this is where i suspect the "monkey" of this feature by the way. enzyme objects are way more complex and not as easy to match.

when you have that working, port that solution into the appropriate files of the addon. again, start with the simple one (snapshotWithOptions) and when you get that working, multiSnapshotWithOptions. For the second one, there might be a PR onto jest-specific-snapshot required.

[waynebrantley]

Any chance for this support. We use several libraries that set Ids and other things that change when rebuilding - we do not want those snapshots to fail for those changes.

[shilman]

PR's welcome! 😘

[badsketch]

@shilman I'd like a chance to work on this 🙂

[badsketch]

Just an update: Still working on this. Sort of have a grasp by now on how this addon works and learning more as I go along, but for a good first issue this is surprisingly involved since it touches a lot of pieces haha (which I guess makes it a good first issue). If this is taking too long, I welcome anybody else submit a PR before me since I know this is a pretty desirable feature and I don't want to hold it up 🚀

[shilman]

@badsketch given that the issue is almost two years old, i think it's safe to assume you're the only one working on it. jump onto our discord if you need help getting through it! https://discord.gg/UUt2PJb

[badsketch]

the biggest problem will probably be getting the property matchers to apply to the "objects" (i.e. react components) coming in. i would start by getting them to work in a local enviroment with react-test-renderer to get an understanding of how they work when combined with components.

If I understand correctly, I can see that jest property matchers are intended for matching objects only and since storyshots uses renderTrees to snapshot and compare components, it isn't particularly useful. Does that mean the scope of this issue includes writing custom serializer options that can then be 'injected' with these property matchers?

[tobilen]

since render trees are also objects, if it were me, i would try to get the necessary information from the render tree of react-test-renderer. that requires you to take a closer look at the internal structure of react component instances and extract the result that will be put into the DOM. if that is what you mean with "serialize", then yes.

[badsketch]

sorry I guess my concern is more along the lines of this issue

Property matchers can only be used to assert on an object

it('will check the matchers and pass', () => {
  const user = {
    createdAt: new Date(),
    id: Math.floor(Math.random() * 20),
    name: 'LeBron James',
  };

  expect(user).toMatchSnapshot({
    createdAt: expect.any(Date),
    id: expect.any(Number),
  });
});

It does not support something like this:

it('renders correctly', () => {
  const tree = renderer
    .create(<Link page="http://www.facebook.com">Facebook</Link>)
    .toJSON();
  expect(tree).toMatchSnapshot({
    for: expect.any(String),
    id: expect.any(String),
  });
});

and the test would fail trying to compare '<Link ...></Link>' with '{ for: Any(String), id: Any(String)}'
So although react-test-renderer render tree produces an object, it won't match any assertions that use property matchers because jest doesn't support a way for them to be put into a 'component' serialization ('<Link for: Any(String)...'), only a plain js object serialization ('{for:Any(String)...').

Like the issue above ,the only way around it was to create a custom serializer for the component I believe. So that's what I mean by needing to include a custom serializer for this feature, unless I'm missing something. Hope that makes sense. And thanks for the help!

[tobilen]

first of all, thanks for working on this issue! When i wrote it, i was was imagining a solution where you don't serialize the component before you pass it to jest, so basically:

it('renders correctly', () => {
  const tree = renderer
    .create(<Link page="http://www.facebook.com">Facebook</Link>)

  expect(tree).toMatchSnapshot({
    for: expect.any(String),
    id: expect.any(String),
  });
});

but it might actually be easier to either:

1. serialize and then parse the JSON back into an object:

it('renders correctly', () => {
  const tree = JSON.parse(renderer
    .create(<Link page="http://www.facebook.com">Facebook</Link>).toJSON())

  expect(tree).toMatchSnapshot({
    for: expect.any(String),
    id: expect.any(String),
  });
});

1. or better yet, use the .toTree() function:

it('renders correctly', () => {
  const tree = renderer
    .create(<Link page="http://www.facebook.com">Facebook</Link>)
    .toTree();
  expect(tree).toMatchSnapshot({
    for: expect.any(String),
    id: expect.any(String),
  });
});

react testing library exposes a toTree method as well: https://testing-library.com/docs/native-testing-library/api#container

for enzyme, i think you need to fall back to the json serialization method. if you even want to support enzyme at all.

your next problem will be how to find out which kind of renderer the user specified and call the appropriate logic. i don't know how a good interface for that would look like. lets see what you can come up with :smiley: