Call print for non-string children in ReactTestComponent plugin

[pedrottimark]

Summary

The most likely reason for the redundant branches deleted in #3731 was an unsuccessful attempt at a shortcut as in ReactTestComponent plugin to call itself directly for children, instead of calling the generic print serializer argument.

When the plugin was written a year ago, that optimization made sense, but now since #3017 pretty-format runs plugins before serializing nested basic values, it is more consistent:

• to handle child test objects like prop values and other non-string children (that is, numbers :)
• to maximize similarity of printChildren in ReactTestComponent, ReactElement, and HTMLElement

This pull request has a small optimization to make up some lost time. Reorder the plugins in jest-snapshot to what seems like decreasing frequency of occurrence:

• ReactTestComponent moves to first place
• ReactElement stays in second place
• HTMLElement moves to third place
• Immutable plugins stay in last place, mainly because of concat method

Here are some measurements for 22 tests adapted from React-test.js:

plugins order	ReactTestComponent original	ReactTestComponent proposed	ReactElement
original	708388	721070	789576
improved	679633	702370	786811

Interpretation:

• The proposed code change to ReactTestComponent with the improved plugins order is less time than original ReactTestComponent and plugins order.
• Preliminary measurements of solutions to Discuss edge cases of indentation in pretty-format plugins #3518 suggest that future code will run in even less time than original code of ReactTestComponent with improved plugins order.
• Because the order of ReactElement plugin stays the same, its test time is within the timing precision, although the test logic for ReactTestComponent is indeed less than HTMLElement.
• I think that ReactElement is slower because it distinguishes children from rest of props and flattens children.

Test plan

Jest

Residue

This PR will make PLUGINS order in jest-snapshot consistent with jest-diff but can y’all explain why jest-matcher-utils doesn’t include ReactTestComponent and what is the purpose of its serialize function?

• jest-diff https://github.com/facebook/jest/blame/master/packages/jest-diff/src/index.js#L28-L33

  const PLUGINS = [
    ReactTestComponent,
    ReactElement,
    AsymmetricMatcher,
    HTMLElement,
  ].concat(Immutable);

• jest-matcher-utils https://github.com/facebook/jest/blob/master/packages/jest-matcher-utils/src/index.js#L21-L23

  const PLUGINS = [AsymmetricMatcher, ReactElement, HTMLElement].concat(
    Immutable,
  );

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

  let PLUGINS = [HTMLElement, ReactElement, ReactTestComponent].concat(Immutable);

[cpojer]

Thanks again @pedrottimark!

I think they should all be using the same set of plugins. Would you mind sending another PR to make them all the same? :)

[pedrottimark]

Yes, sure. What do you recommend:

• Make the PLUGINS array the same in these packages at this point in time?
• Export array (named what :) from pretty-format which the packages import to stay consistent?

[cpojer]

I think the problem is that I didn't want to import all those plugins when simply requiring "pretty-format". If you can make it lazy, like:

prettyFormat.getPlugins = () => {
  ReactElement: require('plugins/...'),
};

that would work for me.

[pedrottimark]

Including ConvertAnsi or not?

[cpojer]

See that's where it gets difficult. That one shouldn't be included in the default list of plugins that Jest uses, for example.

[pedrottimark]

Yes, agree. I will make the separate arrays as consistent as reasonable for now.

For example, AsymmetricMatcher makes sense for jest-diff but not necessarily jest-snapshot

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.