From 265fef1cd70d6893898fbda0c9bd68f756decab1 Mon Sep 17 00:00:00 2001 From: Gamote Date: Tue, 16 Jan 2024 05:56:25 -0800 Subject: [PATCH] fix(#1167): function calls prefixed with `void` together with the optional chain (?.) are skipped (#1178) Summary: Fixes: https://github.com/facebook/metro/issues/1167 `constant-folding-plugin` replaces this call `void foo?.();` with `undefined;`. This is because the `evaluate()` function marks the optional call expressions as safe, making the `UnaryExpression` visitor think it is safe to replace it with the `value` returned by the `evaluate()`, in this case `undefined`. This PR makes the `evaluate()` function mark the optional call expressions as unsafe. ``` * **[Fix]**: `constant-folding-plugin`: Don't fold optional function calls (`foo.?()`). ``` Pull Request resolved: https://github.com/facebook/metro/pull/1178 Test Plan: I have added 2 tests to cover these changes: - does not transform optional chained call into `undefined` - does not transform `void` prefixed optional chained call into `undefined` Reviewed By: GijsWeterings Differential Revision: D52780448 Pulled By: robhogan fbshipit-source-id: c84288adc1fec6c2e875fd766c5a6b0997a36c62 --- .../__tests__/constant-folding-plugin-test.js | 16 ++++++++++++++++ .../src/constant-folding-plugin.js | 12 ++++++++++-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/packages/metro-transform-plugins/src/__tests__/constant-folding-plugin-test.js b/packages/metro-transform-plugins/src/__tests__/constant-folding-plugin-test.js index b9b78569db..a50ad2f0e1 100644 --- a/packages/metro-transform-plugins/src/__tests__/constant-folding-plugin-test.js +++ b/packages/metro-transform-plugins/src/__tests__/constant-folding-plugin-test.js @@ -361,4 +361,20 @@ describe('constant expressions', () => { compare([constantFoldingPlugin], code, expected); }); + + it('does not transform optional chained call into `undefined`', () => { + const code = `foo?.();`; + + const expected = `foo?.();`; + + compare([constantFoldingPlugin], code, expected); + }); + + it('does not transform `void` prefixed optional chained call into `undefined`', () => { + const code = `void foo?.();`; + + const expected = `void foo?.();`; + + compare([constantFoldingPlugin], code, expected); + }); }); diff --git a/packages/metro-transform-plugins/src/constant-folding-plugin.js b/packages/metro-transform-plugins/src/constant-folding-plugin.js index f2e207fcd9..be365e97c2 100644 --- a/packages/metro-transform-plugins/src/constant-folding-plugin.js +++ b/packages/metro-transform-plugins/src/constant-folding-plugin.js @@ -38,7 +38,8 @@ function constantFoldingPlugin(context: { const unsafe = ( path: | NodePath - | NodePath, + | NodePath + | NodePath, state: {safe: boolean}, ) => { state.safe = false; @@ -46,8 +47,15 @@ function constantFoldingPlugin(context: { path.traverse( { - CallExpression: unsafe, AssignmentExpression: unsafe, + CallExpression: unsafe, + /** + * This will mark `foo?.()` as unsafe, so it is not replaced with `undefined` down the line. + * + * We saw this case in the wild, where the unary expression `void foo?.()` was replaced with `undefined` + * resulting in the expression call being skipped. + */ + OptionalCallExpression: unsafe, }, state, );