Jest: Discuss alternative to global configuration of snapshotSerializers

[pedrottimark]

Let’s discuss a potential feature

What is the current behavior?

• Example of problem: the prototype of diff snapshots for https://github.com/facebook/jest/issues/2197#issuecomment-266059285 depends on a new snapshot serializer.
• Why it is a problem: the snapshotSerializers property of Jest configuration is internal to create-react-app configuration, in the react-scripts/utils/createJestConfig.js file. What to do? Eject? Hack into it?

What is the expected behavior?

• Solution for discussion: A built-in generic serializer accepts a “self-serializing” object whose props are a function and its arguments.
• How it solves the problem: Tests can use application-specific serialization functions without:
  
  
  
  • ejecting
  
  
  • requesting a way to extend the snapshotSerializers property in create-react-app
  
  
  • needing to change global configuration, even for Jest not in create-react-app
  
  
  • increasing the number of global serializers to test, even if there becomes a way to extend serializers like middleware
  
  

@cpojer @gaearon What do you think about the balance in this situation between “zero-configuration” goal for what many people need and “local configuration” ability for what a few people need?

Example of problem for prototype diffSnapshot serializer:

describe('component', () => {
  it('updates something', () => {
    const wrapper = mount(/* component in its initial state */);
    const prev = mountedDomToObject(wrapper.find(/* result selector */));
    wrapper.find(/* interaction selector */).simulate(/* event */);
    const next = mountedDomToObject(wrapper.find(/* result selector */);
    expect(diffSnapshot(prev, next)).toMatchSnapshot();
  });
});

exports[`component updates something 1`] = `
  <li
    onClick={[Function]}
    style={
      Object {
<       "textDecoration": "none",
>       "textDecoration": "line-through",
      }
    }>
    Commit snapshot files with code files
  </li>
`;

Off the topic under discussion, goal is halve the number of snapshots for update operations:

• Focus attention of reviewers by making the expected differences explicit.
• Minimize noise from irrelevant changes (for example, additional event handlers).

[pedrottimark]

Here is current trial implementation as a starting point to discuss how.

Minimal generic serializer and factory function to get specific self-serializer:

const fSymbol = Symbol.for('selfSerializer-function');
const argsSymbol = Symbol.for('selfSerializer-args');

module.exports = {
  test(val) {
    return typeof val === 'object' && val !== null &&
      val.hasOwnProperty(fSymbol) && val.hasOwnProperty(argsSymbol);
  },
  print(val, serialize, indent, options) {
    const f = val[fSymbol];
    if (typeof f !== 'function') {
      throw new Error('selfSerializer requires a function');
    }
    return f(val[argsSymbol], serialize, indent, options);
  },
};

// Hmm, so this is another property of exported object above?
// Oh well, it’s a first draft ;)
module.exports.selfSerializer = (f) => (...args) => ({
  [fSymbol]: f,
  [argsSymbol]: args,
});

Example of calling factory function to get self-serializer for diff snapshots:

import {selfSerializer} from 'pretty-format/build/plugins/selfSerializer'; // ?
import {diffLines} from 'diff';

const diffSnapshot = selfSerializer(([prev, next], serialize) => {
  // serialize encapsulates global serializers, including ReactTestComponent :)
  const changes = diffLines(serialize(prev), serialize(next));
  return changes.length === 0
    ? '\u200C' // zero width non-joiner because empty string doesn’t work :(
    : changes.map(({added, removed, value}, index, array) => { … }).join('');
});

describe('component', () => { … });

[cpojer]

I think exposing require('jest-snapshot').addPlugins as expect.addSnapshotSerializer makes sense. Would you mind sending a PR for this use-case?

[pedrottimark]

Yes, that makes sense, I will submit a PR. Thank you for asking.

[pedrottimark]

To apply it to create-react-app:

if you need a global setup before running your tests, add a src/setupTests.js to your project. It will be automatically executed before running your tests.

https://github.com/facebookincubator/create-react-app/blob/master/packages/react-scripts/template/README.md#initializing-test-environment

• Is setupTests.js a recommended place to call expect.addSnapshotSerializer?
• When setupTests.js executes at every test run in watch mode, does addPlugins concatenate to the original PLUGINS array each time, or does it add redundant items to a growing array? See https://github.com/facebook/jest/blob/master/packages/jest-snapshot/src/plugins.js#L17-L21

[cpojer]

I think right now it would be in setupTestFrameworkScriptFile or after (in the test file), so "setupTests" will work. In the long-term, setupFiles should work for this as well, from a Jest perspective.

This array is specific to each individual test file, so adding to it will only work per test and has to be repeated per test file :)

[pedrottimark]

Oh nice, so expect.addSnapshotSerializers(paths) can go near the import or require statements in each test file that depends on external serializers? That’s clear to explain and understand. Also independent of whether an app was built from create-react-app or not.

[cpojer]

Yes!

[pedrottimark]

Because the argument of addPlugins and the value of snapshotSerializers in Jest configuration is an array of module paths, does it make sense to expose it as expect.addSnapshotSerializers(paths) for consistency (that is plural, instead of singular).

[cpojer]

Actually, I think the API should be expect.addSnapshotSerializer(plugin). Ie. it should be a singular API and expect a plugin, not a path to a plugin.

[pedrottimark]

Yes, will do. More than one in a test file sounds very rare.

[pedrottimark]

Yeah, agree to expect a plugin. So an example from a test file would look like:

import serializer from 'app-specific-serializer';
expect.addSnapshotSerializer(serializer);

I need to export another method from 'jest-snapshot' to do that, true?

https://github.com/facebook/jest/blob/master/packages/jest-snapshot/src/plugins.js#L21

To prevent a false analogy between arguments of current addPlugins and similar sounding addPlugin how about I export the new method as addSnapshotSerializer so it sounds different and the name matches when expect exposes it, like the asymmetric matchers:

https://github.com/facebook/jest/blob/master/packages/jest-matchers/src/index.js#L146-L150

[cpojer]

yeah sounds good. It seems to me like you could potentially remove addPlugin and change this: https://github.com/cpojer/jest/blob/master/packages/jest-jasmine2/src/setup-jest-globals.js#L101

to something like:

snapshotSerializers.forEach(serializer => addSerializer(require(serializer)))

that way you aren't increasing the API surface of jest-snapshot by making it more generic. Finally, I'd recommend to call it addSerializer, the "snapshot" part is redundant coming from jest-snapshot. I still suggest expect.addSnapshotSerializer though, I'm just talking about the module boundary.

[pedrottimark]

Here is a sanity check question about https://github.com/facebook/jest/blob/master/packages/jest-jasmine2/src/__tests__/setup-jest-globals-test.js#L18-L25

  const test = serializers => {
    const {getPlugins} = require('jest-snapshot');
    const config = {
      snapshotSerializers: [], // the value should be serializers, true?
    };
    setup({config});
    expect(getPlugins()).toEqual(config.snapshotSerializers);
  };

[cpojer]

No, I think it should be something like this:

config.snapshotSerializers.forEach(serializer => addSerializer(require(serializer)));

expect(getSerializers()).toEqual(
  config.snapshotSerializers.map(serializer => addSerializer(require(serializer)))
);

does that make sense?

[pedrottimark]

Yes, good point that the assertion needs to change from paths to modules.

Sorry I was too terse. Wanted to confirm my impression that the existing test passes with the current incorrect assertion only because in the test helper function config.snapshotSerializers is always empty array independent of the serializers argument.

Because addSerializer works as Last-Added-First-Tested, I plan to call it for the modules from the snapshotSerializers config value in reverse order from the last item to the first item, to preserve the current behavior of plugins = snapshotSerializers.map(…).concat(plugins)

EDIT:

Indeed, 2 tests to add single and multiple plugins are failing now in my branch. I will read about mocking modules, look at what I have done with fresh eyes, and then submit it as PR so we can critique the implementation and improve the testing there :)