Patch devtools before running useMemo function in strict mode

[gsathya]

Summary

This fixes a regression #25583 where we stopped patching before calling useMemo function.

Fixes #27989

Test

yarn test --build --project=devtools --ci

[react-sizebot]

Comparing: ad720f3...35e4250

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js	+0.12%	176.37 kB	176.58 kB	+0.07%	54.96 kB	55.00 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	+0.12%	178.37 kB	178.58 kB	+0.06%	55.54 kB	55.57 kB
facebook-www/ReactDOM-prod.classic.js	+0.10%	590.79 kB	591.35 kB	+0.13%	104.26 kB	104.39 kB
facebook-www/ReactDOM-prod.modern.js	+0.10%	574.55 kB	575.11 kB	+0.13%	101.36 kB	101.49 kB
test_utils/ReactAllWarnings.js	Deleted	67.02 kB	0.00 kB	Deleted	16.42 kB	0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
facebook-react-native/react-test-renderer/cjs/ReactTestRenderer-dev.js	+0.35%	935.63 kB	938.89 kB	+0.36%	187.64 kB	188.31 kB
react-native/implementations/ReactFabric-dev.js	+0.33%	977.43 kB	980.69 kB	+0.34%	197.66 kB	198.32 kB
react-native/implementations/ReactNativeRenderer-dev.js	+0.33%	993.71 kB	996.98 kB	+0.34%	201.93 kB	202.61 kB
oss-stable-semver/react-art/cjs/react-art.production.min.js	+0.21%	99.52 kB	99.73 kB	+0.25%	30.56 kB	30.64 kB
oss-stable/react-art/cjs/react-art.production.min.js	+0.21%	99.57 kB	99.78 kB	+0.25%	30.59 kB	30.66 kB
oss-experimental/react-art/cjs/react-art.production.min.js	+0.21%	101.40 kB	101.61 kB	+0.17%	31.16 kB	31.21 kB
test_utils/ReactAllWarnings.js	Deleted	67.02 kB	0.00 kB	Deleted	16.42 kB	0.00 kB

Generated by 🚫 dangerJS against 35e4250

[hoxyq]

Thanks for fixing this!

[devongovett]

FYI, this change broke the React Aria test suite when running against Canary: https://app.circleci.com/pipelines/github/adobe/react-spectrum/18713/workflows/5e19b423-38fa-414a-85f0-2a63fcd2a5f2/jobs/268672

Was it intentional that this changes the behavior to take the first result of the memo function rather than the second? Changing this broke some code that relied on the most recent call being the current value. The patching of devtools seems fine, but we verified that reverting the change that swapped which instance nextValue comes from fixed our tests. i.e. changing it to this:

- var nextValue = nextCreate();
if (shouldDoubleInvokeUserFnsInHooksDEV) {
  setIsStrictModeForDevtools(true);
  nextCreate();
  setIsStrictModeForDevtools(false);
}

+ var nextValue = nextCreate();

We should probably fix our code not to rely on this assumption, and we will, but since canary is used in production by Next.js I feel like this part of the change should be reverted until a major version. Otherwise users will need to sync their next.js, react, and react-aria updates to avoid running into an issue.

[gsathya]

Thanks for testing against React canary and flagging this, @devongovett! memo functions being stateful is very worrying (especially in the context of Forget), so it's great to see adobe/react-spectrum#5835. I agree it's non ideal to break Aria though, we'll discuss this and I'll follow up here.