From b1eaf88c61dfaf3fedf4f58e45fad4cf519e8049 Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Wed, 26 Apr 2023 15:16:21 -0400 Subject: [PATCH] [babel] Ensure only adding import specifier to non-namespace Missed this in the previous PR Test plan: P706162189 (some babel errors) before this PR, P706255523 has no errors --- compiler/forget/src/Babel/BabelPlugin.ts | 27 +++-- ...xisting-react-kitchensink-import.expect.md | 99 +++++++++++++++++++ ...babel-existing-react-kitchensink-import.js | 16 +++ 3 files changed, 133 insertions(+), 9 deletions(-) create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/babel-existing-react-kitchensink-import.expect.md create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/babel-existing-react-kitchensink-import.js diff --git a/compiler/forget/src/Babel/BabelPlugin.ts b/compiler/forget/src/Babel/BabelPlugin.ts index 1cb4257106..ddb5c9b56a 100644 --- a/compiler/forget/src/Babel/BabelPlugin.ts +++ b/compiler/forget/src/Babel/BabelPlugin.ts @@ -233,14 +233,7 @@ export default function ReactForgetBabelPlugin( } }, ImportDeclaration(importDeclPath) { - // Matches `import { /* ... */ } from 'react';` - // but not `import * as React from 'react';` - if ( - importDeclPath.get("source").node.value === "react" && - importDeclPath - .get("specifiers") - .every((specifier) => specifier.isImportSpecifier()) - ) { + if (isNonNamespacedImportOfReact(importDeclPath)) { hasExistingReactImport = true; } }, @@ -253,7 +246,7 @@ export default function ReactForgetBabelPlugin( let didUpdateImport = false; path.traverse({ ImportDeclaration(importDeclPath) { - if (importDeclPath.get("source").node.value === "react") { + if (isNonNamespacedImportOfReact(importDeclPath)) { importDeclPath.pushContainer( "specifiers", t.importSpecifier( @@ -262,6 +255,7 @@ export default function ReactForgetBabelPlugin( ) ); didUpdateImport = true; + path.stop(); } }, }); @@ -440,3 +434,18 @@ function buildImportForGatingModule( function buildSpecifierIdent(gating: GatingOptions): t.Identifier { return t.identifier(gating.importSpecifierName); } + +/** + * Matches `import { ... } from 'react';` + * but not `import * as React from 'react';` + */ +function isNonNamespacedImportOfReact( + importDeclPath: BabelCore.NodePath +): boolean { + return ( + importDeclPath.get("source").node.value === "react" && + importDeclPath + .get("specifiers") + .every((specifier) => specifier.isImportSpecifier()) + ); +} diff --git a/compiler/forget/src/__tests__/fixtures/compiler/babel-existing-react-kitchensink-import.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/babel-existing-react-kitchensink-import.expect.md new file mode 100644 index 0000000000..3ff22edbed --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/babel-existing-react-kitchensink-import.expect.md @@ -0,0 +1,99 @@ + +## Input + +```javascript +import * as React from "react"; +import { useState } from "react"; + +function Component(props) { + const [x] = useState(0); + const expensiveNumber = React.useMemo(() => calculateExpensiveNumber(x), [x]); + + return
{expensiveNumber}
; +} + +function Component2(props) { + const [x] = useState(0); + const expensiveNumber = React.useMemo(() => calculateExpensiveNumber(x), [x]); + + return
{expensiveNumber}
; +} + +``` + +## Code + +```javascript +import * as React from "react"; +import { useState, unstable_useMemoCache as useMemoCache } from "react"; + +function Component(props) { + const $ = useMemoCache(6); + const [x] = useState(0); + const c_0 = $[0] !== x; + let t1; + if (c_0) { + const c_2 = $[2] !== x; + let t0; + if (c_2) { + t0 = () => calculateExpensiveNumber(x); + $[2] = x; + $[3] = t0; + } else { + t0 = $[3]; + } + t1 = React.useMemo(t0, [x]); + $[0] = x; + $[1] = t1; + } else { + t1 = $[1]; + } + const expensiveNumber = t1; + const c_4 = $[4] !== expensiveNumber; + let t2; + if (c_4) { + t2 =
{expensiveNumber}
; + $[4] = expensiveNumber; + $[5] = t2; + } else { + t2 = $[5]; + } + return t2; +} + +function Component2(props) { + const $ = useMemoCache(6); + const [x] = useState(0); + const c_0 = $[0] !== x; + let t1; + if (c_0) { + const c_2 = $[2] !== x; + let t0; + if (c_2) { + t0 = () => calculateExpensiveNumber(x); + $[2] = x; + $[3] = t0; + } else { + t0 = $[3]; + } + t1 = React.useMemo(t0, [x]); + $[0] = x; + $[1] = t1; + } else { + t1 = $[1]; + } + const expensiveNumber = t1; + const c_4 = $[4] !== expensiveNumber; + let t2; + if (c_4) { + t2 =
{expensiveNumber}
; + $[4] = expensiveNumber; + $[5] = t2; + } else { + t2 = $[5]; + } + return t2; +} + +``` + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/compiler/babel-existing-react-kitchensink-import.js b/compiler/forget/src/__tests__/fixtures/compiler/babel-existing-react-kitchensink-import.js new file mode 100644 index 0000000000..4880c50eb2 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/babel-existing-react-kitchensink-import.js @@ -0,0 +1,16 @@ +import * as React from "react"; +import { useState } from "react"; + +function Component(props) { + const [x] = useState(0); + const expensiveNumber = React.useMemo(() => calculateExpensiveNumber(x), [x]); + + return
{expensiveNumber}
; +} + +function Component2(props) { + const [x] = useState(0); + const expensiveNumber = React.useMemo(() => calculateExpensiveNumber(x), [x]); + + return
{expensiveNumber}
; +}