Fix: Prevent maintaining RegExp state between multiple tests

[wsmd]

Summary

Fixes #9283

Currently expect maintains the state of RegExp objects when using toMatch(RegExp) or toThrow(RegExp) with a global flag g.

For example, the second assertion here fails:

const regex = /[f]\d+/g;
expect('f123').toMatch(regex);
expect('f456').toMatch(regex); // Fails!

What's expected there is for both assertions to pass.

This happens because expect is using the same RegExp object for every test. This can be solved by instantiating a new RegExp object for every test.

Test plan

Added a new test.

[wsmd]

I doubt there be will any issues with instantiating a new RegExp object for every call, but if this is a concern, I could limit that to only RegExps with the global/sticky flag since that's where this issue seems most likely to arise. Let me know what you think!

[thymikee]

I don't really know how I feel about this. As a JS user, I would expect this regex to be stateful and would be surprised that it's re-set by a specific matcher.
Not to mention this is a breaking change and needs documentation.
cc @pedrottimark @SimenB

[wsmd]

I agree @thymikee, FWIW that's the entire point of having a global flag in the first place is to be able to test against multiple matches. The issue reported via #9283 could be solved by removing the global from the RegExp since it's doing what it's supposed to do.

That said, #9283 was marked as a bug because this (specifically with Jest) could also be considered a regression since this was not the intended behavior before v24 was released.

I'm not sure if this behavior change was intentional or documented somewhere (since it was also a breaking change), or if it was just a regression that needs to be fixed (hence this PR).

[mockdeep]

For what it's worth, I wouldn't expect Jest to change the state of an object, including a regular expression. It seems like it creates confusion and potential for false-positives in cases where the regular expression is being tested against several strings in the same test. If I want to test the lastIndex of a regex, I think I would test that directly:

regex.test('f123');
expect(regex.lastIndex).toBe(4);
regex.test('f456');
expect(regex.lastIndex).toBe(0);

[pedrottimark]

You have my apology that I missed commenting on the issue or pull request before this.

It was my mistake in #8008 not to keep the code path to create a new instance for expected RegExp when I changed an expected string to match using includes method.

Therefore, this PR fixes what I broke, no more and no less.

@wsmd Thanks for your work. If you can merge changes from master and resolve the conflict in CHANGELOG.md then this is ready to merge.

There is a related (but separate) documentation chore in ExpectAPI.md under toMatch to adapt the example in #9283 and in the preceding #9289 (comment) so people understand how to write tests that are independent of RegExp state or intentionally testing RegExp state.

[wsmd]

Thanks @pedrottimark! The merge conflict is now resolved. 👍

[codecov-io]

Codecov Report

Merging #9289 into master will decrease coverage by 0.18%.
The diff coverage is 46.44%.

@@            Coverage Diff             @@
##           master    #9289      +/-   ##
==========================================
- Coverage   64.98%   64.79%   -0.19%     
==========================================
  Files         278      281       +3     
  Lines       11882    12009     +127     
  Branches     2926     2962      +36     
==========================================
+ Hits         7721     7781      +60     
- Misses       3532     3597      +65     
- Partials      629      631       +2

Impacted Files	Coverage Δ
e2e/v8-coverage/no-sourcemap/x.css	100% <ø> (ø)
packages/jest-config/src/ValidConfig.ts	100% <ø> (ø)	⬆️
packages/jest-config/src/Defaults.ts	100% <ø> (ø)	⬆️
packages/jest-config/src/normalize.ts	77.74% <ø> (ø)	⬆️
...ckages/jest-reporters/src/generateEmptyCoverage.ts	66.66% <0%> (-22.23%)	⬇️
packages/jest-config/src/index.ts	11.76% <0%> (-0.18%)	⬇️
...ackages/jest-jasmine2/src/assertionErrorMessage.ts	0% <0%> (ø)	⬆️
packages/jest-runner/src/runTest.ts	2.19% <0%> (-0.25%)	⬇️
packages/jest-circus/src/formatNodeAssertErrors.ts	11.86% <0%> (-1.1%)	⬇️
packages/jest-snapshot/src/State.ts	0% <0%> (ø)	⬆️
... and 23 more

---

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 88a8cbd...0312bd2. Read the comment docs.

[pedrottimark]

By the way, I verified locally that new RegExp(regexp) copies any flags like i

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