Align StrictMode behaviour with production

[sammy-SC]

Summary

PR changes behaviour of StrictMode to better align it what mount/unmount/mount would look like in production.
We change custom traversal function that was previously used for StrictMode to functions used in production.
This has following implications:

• refs are now attached/detached/attached in StrictMode.
• When <unstable_Offscreen /> is hidden, StrictMode doesn't double invoke effects. StrictMode will double invoke effects when <unstable_Offscreen /> becomes visible.

How did you test this change?

I've added tests for how <unstable_Offscreen /> should behave in and a test to verify refs are detached in StrictMode.

[sizebot]

Comparing: 32baab3...6b3de9f

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.02%	134.28 kB	134.31 kB	+0.01%	42.93 kB	42.93 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	+0.02%	139.84 kB	139.86 kB	+0.03%	44.55 kB	44.57 kB
facebook-www/ReactDOM-prod.classic.js	=	474.44 kB	474.46 kB	=	84.86 kB	84.87 kB
facebook-www/ReactDOM-prod.modern.js	=	459.68 kB	459.70 kB	=	82.62 kB	82.63 kB
facebook-www/ReactDOMForked-prod.classic.js	=	474.44 kB	474.46 kB	=	84.86 kB	84.87 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
facebook-www/ReactDOM-dev.classic.js	=	1,217.86 kB	1,215.13 kB	=	265.80 kB	265.53 kB
facebook-www/ReactDOMForked-dev.classic.js	=	1,217.86 kB	1,215.13 kB	=	265.80 kB	265.52 kB
facebook-www/ReactDOM-dev.modern.js	=	1,194.24 kB	1,191.51 kB	=	261.32 kB	261.02 kB
facebook-www/ReactDOMForked-dev.modern.js	=	1,194.24 kB	1,191.51 kB	=	261.32 kB	261.02 kB
oss-experimental/react-dom/cjs/react-dom.development.js	=	1,087.26 kB	1,084.61 kB	=	242.25 kB	242.00 kB
oss-experimental/react-dom/umd/react-dom.development.js	=	1,140.49 kB	1,137.65 kB	=	244.96 kB	244.70 kB
oss-stable/react-dom/cjs/react-dom.development.js	=	1,054.39 kB	1,051.73 kB	=	235.48 kB	235.21 kB
oss-stable-semver/react-dom/cjs/react-dom.development.js	=	1,054.36 kB	1,051.70 kB	=	235.45 kB	235.19 kB
oss-stable/react-dom/umd/react-dom.development.js	=	1,106.03 kB	1,103.19 kB	=	238.12 kB	237.87 kB
oss-stable-semver/react-dom/umd/react-dom.development.js	=	1,106.00 kB	1,103.17 kB	=	238.10 kB	237.84 kB
react-native/implementations/ReactNativeRenderer-dev.fb.js	=	847.44 kB	844.84 kB	=	182.57 kB	182.32 kB
react-native/implementations/ReactFabric-dev.fb.js	=	837.80 kB	835.19 kB	=	179.91 kB	179.68 kB
facebook-www/ReactART-dev.classic.js	=	832.87 kB	830.14 kB	=	175.53 kB	175.23 kB
oss-experimental/react-reconciler/cjs/react-reconciler.development.js	=	805.04 kB	802.38 kB	=	170.51 kB	170.27 kB
facebook-www/ReactART-dev.modern.js	=	822.65 kB	819.92 kB	=	173.39 kB	173.09 kB
oss-experimental/react-art/umd/react-art.development.js	=	844.28 kB	841.44 kB	=	176.67 kB	176.40 kB
oss-stable/react-reconciler/cjs/react-reconciler.development.js	=	774.12 kB	771.47 kB	=	164.30 kB	164.06 kB
oss-stable-semver/react-reconciler/cjs/react-reconciler.development.js	=	774.10 kB	771.44 kB	=	164.28 kB	164.04 kB
oss-stable/react-art/umd/react-art.development.js	=	812.17 kB	809.34 kB	=	170.45 kB	170.21 kB
oss-stable-semver/react-art/umd/react-art.development.js	=	812.15 kB	809.31 kB	=	170.43 kB	170.19 kB
facebook-www/ReactTestRenderer-dev.classic.js	=	745.49 kB	742.89 kB	=	158.67 kB	158.43 kB
facebook-www/ReactTestRenderer-dev.modern.js	=	745.49 kB	742.88 kB	=	158.67 kB	158.43 kB
oss-experimental/react-art/cjs/react-art.development.js	=	738.25 kB	735.59 kB	=	158.50 kB	158.25 kB
oss-stable/react-art/cjs/react-art.development.js	=	707.62 kB	704.96 kB	=	152.26 kB	152.00 kB
oss-stable-semver/react-art/cjs/react-art.development.js	=	707.60 kB	704.94 kB	=	152.24 kB	151.98 kB

Generated by 🚫 dangerJS against 6b3de9f

[acdlite]

This looks great! I left a handful of factoring suggestions but nothing blocking.

[acdlite]

We should wrap this whole function in if (parentFiber.subtreeFlags & PlacementDEV). That way we can skip over any paths that don't include insertions.

[sammy-SC]

Adding this breaks tests in experimental environment.

yarn test -r=experimental ReactOffscreenStrictMode-test.js

I'll merge PR as is and dig into this later since it is an optimisation.

[acdlite]

Factoring nit: All of these successive if branches include a isInStrictMode check. You can factor that out:

if (isInStrictMode) {
  disappearLayoutEffects(...);
  reappearLayoutEfffects(...);
  disconnectPassiveEffect(...);
  reconnectPassiveEffects(...);
}

Also I don't think the setCurrentDebugFiberInDEV stuff is necessary anymore because disappearLayoutEffects et al already set it before they run the effects code.

[sammy-SC]

setCurrentDebugFiberInDEV is needed for DevTools. I'm not sure what the interaction between DevTools + React is in this case but DevTool tests start failing without it.