[compiler] Wrap inline jsx transform codegen in conditional

[jackpope]

JSX inlining is a prod-only optimization. We want to enforce this while maintaining the same compiler output in DEV and PROD.

Here we add a conditional to the transform that only replaces JSX with object literals outside of DEV. Then a later build step can handle DCE based on the value of __DEV__

[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	Oct 15, 2024 6:18pm

[jackpope]

@mofeiZ it looks like the fbt/fbt-call-complex-param-value test is failing here with a debug build. This happens to the inline jsx test when I use __DEV__ instead of DEV

[mofeiZ]

@mofeiZ it looks like the fbt/fbt-call-complex-param-value test is failing here with a debug build. This happens to the inline jsx test when I use __DEV__ instead of DEV

Ahh looks like the issue is that the newly added test fixture writes to global.__DEV__. We currently do not run snap fixtures in with an isolated global object/dom, so this also affects compilation (see check for whether we need to insert fast refresh).

Do you need to set the global __DEV__ in this unit test? We probably patch the evaluator to support this if needed

[josephsavona]

Sorry for the delay on review, see comments

[josephsavona]

probably safer to reset phis, preds, etc here, or more generally to just copy the specific properties you want to keep (which is really only block.kind)

[josephsavona]

Hmm this isn't safe. There are two main categories of blocks which correspond roughly to statement-level blocks (kind 'block') and expression-level blocks (kind 'logical' etc). It's only safe to add a variable declaration at a block statement. So i think what you'd need to do is write this as a ternary, with an unnamed (non-promoted) variable that is getting assigned one of two different values without a variable declaration.

ie you're doing:

let t0;
if (__DEV__) {
  t0 = { $$typeof: 'react.element', ...}
} else {
  t0 = jsx(...);
}

When it needs to be

__DEV__ ? { $$typeof: ... } : jsx(...)

So that it works in statement and expression contexts.

Ternaries are a bit tedious to create, you need both a ternary terminal and a branch terminal. But you get to avoid the variable declaration. You'll still need to create the phi in the fallthrough block.

[josephsavona]

A failing example would be something like:

function Component({cond}) {
  return cond ? <div /> : null;
}

because we'll try to create a variable declaration inside the consequent of the ternary, but that's a statement where only expressions are allowed.

[josephsavona]

Chatted with @mofeiZ about this briefly — she suggested that we could unblock getting some experimental results by just skipping JSX within value blocks. That seems like a great compromise, and if the data is promising we can come back to handle value blocks.