Skip to content

Bump @babel/traverse to ^7.29.10 and add constant-folding regression tests - #2043

Open
cpruijsen wants to merge 2 commits into
react:mainfrom
cpruijsen:fix/issue-1927-0bfb831a
Open

cpruijsen wants to merge 2 commits into
react:mainfrom
cpruijsen:fix/issue-1927-0bfb831a

Conversation

@cpruijsen

Copy link
Copy Markdown
Contributor

Summary

constant-folding-plugin no longer folds an expression that references a binding declared by an object or array pattern.

The plugin trusts path.evaluate(). For a destructured binding, evaluate() resolves to the whole init of the declarator and reports the result as confident (babel/babel#18238). The fold is then wrong, with no error, and only in release builds:

const O = {e: 80};
function f() { const {e} = O; return e + 1; }
// emitted: return "[object Object]1";

const {lo, hi, scale} = SHAPE;
(hi - lo) * scale   // -> 0 / 0

const {flag} = O;   // O.flag === false
flag ? 1 : 2        // -> 1     (wrong branch)
flag || 3           // -> flag  (wrong operand)

The change in packages/metro-transform-plugins/src/constant-folding-plugin.js adds an Identifier visitor to the existing unsafe-traversal check, which already rejects calls and assignments. path.traverse() does not visit the root node, so the check also runs on a bare identifier at the root, for example the test of a conditional.

The issue suggests rejecting non-finite numeric results instead. That does not catch the string coercion or wrong-branch cases, and it breaks the existing NaN ?? x() fold, so this change rejects the destructured reference and does not inspect the value. The cost: a fold through a destructured binding with a safe literal init (const {a} = {a: 1}; a + 2) no longer happens.

Fixes #1927

Changelog: [Fix] Fix constant folding of expressions that reference destructured bindings (emitted 0 / 0, "[object Object]1" or the wrong branch in release builds)

Test plan

Four new cases in constant-folding-plugin-test.js (arithmetic, NaN, conditional, logical) fail on main and pass with this change. The rest of the metro-transform-plugins tests pass unchanged.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 9, 2026
@facebook-github-tools facebook-github-tools Bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Oct 9, 2026
@robhogan

robhogan commented Oct 11, 2026 •

Copy link
Copy Markdown
Collaborator

Thanks, but since the fix has landed upstream in @babel/traverse 7.29.10 via babel/babel#18281, the cleanest fix is just to bump our minimum to that - your tests are still good and could be included.

PR welcome if you'd like to do that.

@robhogan robhogan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above - tests are good but a Babel bump brings in the fix more cleanly

@cpruijsen
cpruijsen force-pushed the fix/issue-1927-0bfb831a branch from 7a0e041 to ef3d8e8 Compare October 12, 2026 13:34
@cpruijsen cpruijsen changed the title constant-folding-plugin: Decline to fold expressions referencing destructured bindings Bump @babel/traverse to ^7.29.10 and add constant-folding regression tests Oct 12, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. needs-meta-import Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

constant-folding-plugin emits 0/0 (NaN) for arithmetic on destructured bindings

2 participants