[compiler] Clone computation block in change detection mode

[mvitousek]

Stack from ghstack (oldest at bottom):

• -> [compiler] Clone computation block in change detection mode #30148

Summary: In change-detection mode, we previously were spreading the contents of the computation block into the result twice. Other babel passes that cause in-place mutations of the AST would then be causing action at a distance and breaking the overall transform result. This pr creates clones of the nodes instead, so that mutations aren't reflected in both places where the block is used.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
react-compiler-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 30, 2024 9:26pm

[react-sizebot]

Comparing: 2e72ea8...e6c1cd4

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.js	=	6.68 kB	6.68 kB	=	1.83 kB	1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js	=	497.99 kB	497.99 kB	=	89.27 kB	89.27 kB
oss-experimental/react-dom/cjs/react-dom.production.js	=	6.69 kB	6.69 kB	=	1.83 kB	1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js	=	502.81 kB	502.81 kB	=	89.97 kB	89.97 kB
facebook-www/ReactDOM-prod.classic.js	=	597.08 kB	597.08 kB	=	105.33 kB	105.33 kB
facebook-www/ReactDOM-prod.modern.js	=	571.42 kB	571.42 kB	=	101.27 kB	101.27 kB
test_utils/ReactAllWarnings.js	Deleted	62.88 kB	0.00 kB	Deleted	15.69 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
test_utils/ReactAllWarnings.js	Deleted	62.88 kB	0.00 kB	Deleted	15.69 kB	0.00 kB

Generated by 🚫 dangerJS against e6c1cd4

[gsathya]

is it possible to write a test for this? i suppose not because we don't use other babel transforms in our test?

[mvitousek]

Yeah, not easy to make a test in our infra. I've tested this offline, and it's easy enough to create a repro for this: I did so with a babel config with Forget, babel/plugin-transform-destructuring, and babel/plugin-transform-block-scoping (and Hermes parser for Flow syntax); before this diff, the following program would have uses of mediaWidth while all declarations were renamed to mediaWidth_, while with this diff it compiles as expected.

export default hook useP(p) {
  const mediaItem = nullthrows(p);
  const mediaWidth = mediaItem.height;
  const mediaHeight = mediaItem.width;
  const {croppedHeightScaled, croppedWidthScaled} =
    getCS(mediaWidth, mediaHeight);
  const {ocroppedHeightScaled, ocroppedWidthScaled} =
    getCS(mediaWidth, mediaHeight);
}

But I don't know if there's a good way to build this as a regression test in the compiler's current setup.