Avoid unnecessary function declarations and call in pretty-format

[pedrottimark]

Summary

Problem:

• printPlugin “hoists” the boundPrint and boundIndent declarations and therefore initializes the functions whether or not a plugin matches the value.
• prettyFormat calls createIndent if any plugins exist instead of if some plugin matches the value. Because plugins always exist in Jest, the conditional logic to avoid creating indentation for basic values doesn’t save any time.

Solution:

• Factor out a findPlugin function.
• Factor out if statement to call printPlugin only if a plugin matches the value.
• Call createIndent for arg in printPlugin and printComplexValue function calls.

Your review is especially wanted about the decision to throw an error if a plugin returns a non-string value. Am happy to revert that change if it’s a step in a wrong direction. Current logic quietly falls through to print the value as either basic or complex.

Interpretation of the following performance measurements:

• Big improvement for massive object with no plugins outside Jest.
• Almost as big improvement with default plugins (not called) inside Jest.
• Only slight change for small elements with default plugins (called) inside Jest.

scenario	plugins	proposed	baseline	ratio
geo.json	0	30693309	72540558	0.423
geo.json	10	70169013	127109022	0.552
React elements	10	1283359	1287893	0.996

P.S. The calls to printPlugin are nice example of not ignoring indentation in a diff, heh :)

Test plan

Jest

[codecov-io]

Codecov Report

Merging #3962 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3962      +/-   ##
==========================================
- Coverage   59.86%   59.84%   -0.03%     
==========================================
  Files         196      196              
  Lines        6763     6758       -5     
  Branches        6        6              
==========================================
- Hits         4049     4044       -5     
  Misses       2711     2711              
  Partials        3        3

Impacted Files	Coverage Δ
packages/pretty-format/src/index.js	98% <100%> (-0.05%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b079d2a...77bae77. Read the comment docs.

[cpojer]

This is great, thanks so much for optimizing the code here. How did you track this down, did you do some profiling?

As for throwing an error: while technically a breaking change, the type of the print function of a Plugin is defined to return a string. I think your added constraint in the code makes sense, given that we don't have control over the behavior of user plugins. While it may be beneficial to return another value that will then be printed by prettyFormat, you have the print function available from within plugins so we aren't limiting the functionality of plugins.

Thanks for coming up with this PR!

[pedrottimark]

You’re welcome. Something didn’t feel right in the function when I thought about a code change to support alternative API discussed in #3518 (comment). You would laugh to see a scan of my Pooh Bear brain puzzling it out. The performance improvements are a side effect of making the package more correct and clear where it has been hard for me understand, so a wider range of people might contribute successfully in the future to keep up with ECMAScript and application-specific data types.

When Jest turns 21 people can reasonably expect it to break some things, heh?

[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.