From 3b72b8f53e87d6030215ec175995e15d7c188167 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 26 Mar 2021 00:58:43 +0000 Subject: [PATCH 1/8] [Fast Refresh] Support callthrough HOCs --- .../src/ReactFreshBabelPlugin.js | 29 ++++- .../react-refresh/src/ReactFreshRuntime.js | 3 + .../__tests__/ReactFreshIntegration-test.js | 102 ++++++++++++++++++ .../ReactFreshBabelPlugin-test.js.snap | 14 +-- 4 files changed, 140 insertions(+), 8 deletions(-) diff --git a/packages/react-refresh/src/ReactFreshBabelPlugin.js b/packages/react-refresh/src/ReactFreshBabelPlugin.js index 4171893e41ddd..82f290b7321d1 100644 --- a/packages/react-refresh/src/ReactFreshBabelPlugin.js +++ b/packages/react-refresh/src/ReactFreshBabelPlugin.js @@ -333,6 +333,27 @@ export default function(babel, opts = {}) { return args; } + // Traverse HOC calls upwards to the rootmost one. + function findOuterCallPath(path) { + let outerCallPath = null; + while (true) { + if (!path) { + return outerCallPath; + } + if (path.node.type === 'AssignmentExpression') { + // Ignore registrations. + path = path.parentPath; + continue; + } + if (path.node.type === 'CallExpression') { + outerCallPath = path; + path = path.parentPath; + continue; + } + return outerCallPath; // Stop at other types. + } + } + const seenForRegistration = new WeakSet(); const seenForSignature = new WeakSet(); const seenForOutro = new WeakSet(); @@ -630,13 +651,17 @@ export default function(babel, opts = {}) { // Result: let Foo = () => {}; __signature(Foo, ...); } else { // let Foo = hoc(() => {}) + const outerCallPath = findOuterCallPath(path.parentPath); + if (outerCallPath) { + path = outerCallPath; + } path.replaceWith( t.callExpression( sigCallID, - createArgumentsForSignature(node, signature, path.scope), + createArgumentsForSignature(path.node, signature, path.scope), ), ); - // Result: let Foo = hoc(__signature(() => {}, ...)) + // Result: let Foo = __signature(hoc(() => {}), ...) } }, }, diff --git a/packages/react-refresh/src/ReactFreshRuntime.js b/packages/react-refresh/src/ReactFreshRuntime.js index 669eb7c1520f0..073b0e8fea2b7 100644 --- a/packages/react-refresh/src/ReactFreshRuntime.js +++ b/packages/react-refresh/src/ReactFreshRuntime.js @@ -355,6 +355,9 @@ export function setSignature( getCustomHooks?: () => Array, ): void { if (__DEV__) { + if (allSignaturesByType.has(type)) { + return; + } allSignaturesByType.set(type, { forceReset, ownKey: key, diff --git a/packages/react-refresh/src/__tests__/ReactFreshIntegration-test.js b/packages/react-refresh/src/__tests__/ReactFreshIntegration-test.js index f3538670e3717..26366b85b5474 100644 --- a/packages/react-refresh/src/__tests__/ReactFreshIntegration-test.js +++ b/packages/react-refresh/src/__tests__/ReactFreshIntegration-test.js @@ -469,6 +469,108 @@ describe('ReactFreshIntegration', () => { } }); + it('resets state when renaming a state variable inside a HOC with direct call', () => { + if (__DEV__) { + render(` + const {useState} = React; + const S = 1; + + function hocWithDirectCall(Wrapped) { + return function Generated() { + return Wrapped(); + }; + } + + export default hocWithDirectCall(() => { + const [foo, setFoo] = useState(S); + return

A{foo}

; + }); + `); + const el = container.firstChild; + expect(el.textContent).toBe('A1'); + + patch(` + const {useState} = React; + const S = 2; + + function hocWithDirectCall(Wrapped) { + return function Generated() { + return Wrapped(); + }; + } + + export default hocWithDirectCall(() => { + const [foo, setFoo] = useState(S); + return

B{foo}

; + }); + `); + // Same state variable name, so state is preserved. + expect(container.firstChild).toBe(el); + expect(el.textContent).toBe('B1'); + + patch(` + const {useState} = React; + const S = 3; + + function hocWithDirectCall(Wrapped) { + return function Generated() { + return Wrapped(); + }; + } + + export default hocWithDirectCall(() => { + const [bar, setBar] = useState(S); + return

C{bar}

; + }); + `); + // Different state variable name, so state is reset. + expect(container.firstChild).not.toBe(el); + const newEl = container.firstChild; + expect(newEl.textContent).toBe('C3'); + } + }); + + it('does not crash when changing Hook order inside a HOC with direct call', () => { + if (__DEV__) { + render(` + const {useEffect} = React; + + function hocWithDirectCall(Wrapped) { + return function Generated() { + return Wrapped(); + }; + } + + export default hocWithDirectCall(() => { + useEffect(() => {}, []); + return

A

; + }); + `); + const el = container.firstChild; + expect(el.textContent).toBe('A'); + + patch(` + const {useEffect} = React; + + function hocWithDirectCall(Wrapped) { + return function Generated() { + return Wrapped(); + }; + } + + export default hocWithDirectCall(() => { + useEffect(() => {}, []); + useEffect(() => {}, []); + return

B

; + }); + `); + // Hook order changed, so we remount. + expect(container.firstChild).not.toBe(el); + const newEl = container.firstChild; + expect(newEl.textContent).toBe('B'); + } + }); + it('resets effects while preserving state', () => { if (__DEV__) { render(` diff --git a/packages/react-refresh/src/__tests__/__snapshots__/ReactFreshBabelPlugin-test.js.snap b/packages/react-refresh/src/__tests__/__snapshots__/ReactFreshBabelPlugin-test.js.snap index 729996f49ea3f..424cfeeaf28a0 100644 --- a/packages/react-refresh/src/__tests__/__snapshots__/ReactFreshBabelPlugin-test.js.snap +++ b/packages/react-refresh/src/__tests__/__snapshots__/ReactFreshBabelPlugin-test.js.snap @@ -37,11 +37,13 @@ const Bar = () => { _s4(Bar, "useContext{}"); _c2 = Bar; -const Baz = memo(_c3 = _s5(() => { + +const Baz = _s5(memo(_c3 = () => { _s5(); return useContext(X); -}, "useContext{}")); +}), "useContext{}"); + _c4 = Baz; const Qux = () => { @@ -108,21 +110,21 @@ exports[`ReactFreshBabelPlugin generates signatures for function expressions cal var _s = $RefreshSig$(), _s2 = $RefreshSig$(); -export const A = React.memo(_c2 = React.forwardRef(_c = _s((props, ref) => { +export const A = _s(React.memo(_c2 = React.forwardRef(_c = (props, ref) => { _s(); const [foo, setFoo] = useState(0); React.useEffect(() => {}); return

{foo}

; -}, "useState{[foo, setFoo](0)}\\nuseEffect{}"))); +})), "useState{[foo, setFoo](0)}\\nuseEffect{}"); _c3 = A; -export const B = React.memo(_c5 = React.forwardRef(_c4 = _s2(function (props, ref) { +export const B = _s2(React.memo(_c5 = React.forwardRef(_c4 = function (props, ref) { _s2(); const [foo, setFoo] = useState(0); React.useEffect(() => {}); return

{foo}

; -}, "useState{[foo, setFoo](0)}\\nuseEffect{}"))); +})), "useState{[foo, setFoo](0)}\\nuseEffect{}"); _c6 = B; function hoc() { From 38423eb1f67da1f2d4b04c317e9532e1af431d33 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 26 Mar 2021 03:24:47 +0000 Subject: [PATCH 2/8] Add a newly failing testing to demonstrate the flaw This shows why my initial approach doesn't make sense. --- .../__tests__/ReactFreshIntegration-test.js | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/packages/react-refresh/src/__tests__/ReactFreshIntegration-test.js b/packages/react-refresh/src/__tests__/ReactFreshIntegration-test.js index 26366b85b5474..60e9688661e08 100644 --- a/packages/react-refresh/src/__tests__/ReactFreshIntegration-test.js +++ b/packages/react-refresh/src/__tests__/ReactFreshIntegration-test.js @@ -571,6 +571,43 @@ describe('ReactFreshIntegration', () => { } }); + it('does not crash when changing Hook order inside a HOC returning an object', () => { + if (__DEV__) { + render(` + const {useEffect} = React; + + function hocWithDirectCall(Wrapped) { + return {Wrapped: Wrapped}; + } + + export default hocWithDirectCall(() => { + useEffect(() => {}, []); + return

A

; + }).Wrapped; + `); + const el = container.firstChild; + expect(el.textContent).toBe('A'); + + patch(` + const {useEffect} = React; + + function hocWithDirectCall(Wrapped) { + return {Wrapped: Wrapped}; + } + + export default hocWithDirectCall(() => { + useEffect(() => {}, []); + useEffect(() => {}, []); + return

B

; + }).Wrapped; + `); + // Hook order changed, so we remount. + expect(container.firstChild).not.toBe(el); + const newEl = container.firstChild; + expect(newEl.textContent).toBe('B'); + } + }); + it('resets effects while preserving state', () => { if (__DEV__) { render(` From a804fbc8bf535dc4b1fcbacfcb838d4a94a1bcc2 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 26 Mar 2021 03:53:39 +0000 Subject: [PATCH 3/8] Attach signatures at every nesting level --- .../src/ReactFreshBabelPlugin.js | 32 +++++------ .../react-refresh/src/ReactFreshRuntime.js | 56 +++++++++---------- .../ReactFreshBabelPlugin-test.js.snap | 12 ++-- 3 files changed, 47 insertions(+), 53 deletions(-) diff --git a/packages/react-refresh/src/ReactFreshBabelPlugin.js b/packages/react-refresh/src/ReactFreshBabelPlugin.js index 82f290b7321d1..7df35f967b1d3 100644 --- a/packages/react-refresh/src/ReactFreshBabelPlugin.js +++ b/packages/react-refresh/src/ReactFreshBabelPlugin.js @@ -333,12 +333,11 @@ export default function(babel, opts = {}) { return args; } - // Traverse HOC calls upwards to the rootmost one. - function findOuterCallPath(path) { - let outerCallPath = null; + function findHOCCallPathsAbove(path) { + const calls = []; while (true) { if (!path) { - return outerCallPath; + return calls; } if (path.node.type === 'AssignmentExpression') { // Ignore registrations. @@ -346,11 +345,11 @@ export default function(babel, opts = {}) { continue; } if (path.node.type === 'CallExpression') { - outerCallPath = path; + calls.push(path); path = path.parentPath; continue; } - return outerCallPath; // Stop at other types. + return calls; // Stop at other types. } } @@ -651,17 +650,16 @@ export default function(babel, opts = {}) { // Result: let Foo = () => {}; __signature(Foo, ...); } else { // let Foo = hoc(() => {}) - const outerCallPath = findOuterCallPath(path.parentPath); - if (outerCallPath) { - path = outerCallPath; - } - path.replaceWith( - t.callExpression( - sigCallID, - createArgumentsForSignature(path.node, signature, path.scope), - ), - ); - // Result: let Foo = __signature(hoc(() => {}), ...) + const paths = [path, ...findHOCCallPathsAbove(path.parentPath)]; + paths.forEach(p => { + p.replaceWith( + t.callExpression( + sigCallID, + createArgumentsForSignature(p.node, signature, p.scope), + ), + ); + }); + // Result: let Foo = __signature(hoc(__signature(() => {}, ...)), ...) } }, }, diff --git a/packages/react-refresh/src/ReactFreshRuntime.js b/packages/react-refresh/src/ReactFreshRuntime.js index 073b0e8fea2b7..d497112f420c6 100644 --- a/packages/react-refresh/src/ReactFreshRuntime.js +++ b/packages/react-refresh/src/ReactFreshRuntime.js @@ -612,57 +612,53 @@ export function _getMountedRootCount() { // function Hello() { // const [foo, setFoo] = useState(0); // const value = useCustomHook(); -// _s(); /* Second call triggers collecting the custom Hook list. +// _s(); /* Call without arguments triggers collecting the custom Hook list. // * This doesn't happen during the module evaluation because we // * don't want to change the module order with inline requires. // * Next calls are noops. */ // return

Hi

; // } // -// /* First call specifies the signature: */ +// /* Call with arguments attaches the signature to the type: */ // _s( // Hello, // 'useState{[foo, setFoo]}(0)', // () => [useCustomHook], /* Lazy to avoid triggering inline requires */ // ); -type SignatureStatus = 'needsSignature' | 'needsCustomHooks' | 'resolved'; export function createSignatureFunctionForTransform() { if (__DEV__) { - // We'll fill in the signature in two steps. - // First, we'll know the signature itself. This happens outside the component. - // Then, we'll know the references to custom Hooks. This happens inside the component. - // After that, the returned function will be a fast path no-op. - let status: SignatureStatus = 'needsSignature'; let savedType; let hasCustomHooks; + let didCollectHooks = false; return function( type: T, key: string, forceReset?: boolean, getCustomHooks?: () => Array, - ): T { - switch (status) { - case 'needsSignature': - if (type !== undefined) { - // If we received an argument, this is the initial registration call. - savedType = type; - hasCustomHooks = typeof getCustomHooks === 'function'; - setSignature(type, key, forceReset, getCustomHooks); - // The next call we expect is from inside a function, to fill in the custom Hooks. - status = 'needsCustomHooks'; - } - break; - case 'needsCustomHooks': - if (hasCustomHooks) { - collectCustomHooksForSignature(savedType); - } - status = 'resolved'; - break; - case 'resolved': - // Do nothing. Fast path for all future renders. - break; + ): T | void { + if (typeof key === 'string') { + // We're in the initial phase that associates signatures + // with the functions. Note this may be called multiple times + // in HOC chains like _s(hoc1(_s(hoc2(_s(actualFunction))))). + if (!savedType) { + // We're in the innermost call, so this is the actual type. + savedType = type; + hasCustomHooks = typeof getCustomHooks === 'function'; + } + // Set the signature for all types (even wrappers!) in case + // they have no signatures of their own. This is to prevent + // problems like https://github.com/facebook/react/issues/20417. + setSignature(type, key, forceReset, getCustomHooks); + return type; + } else { + // We're in the _s() call without arguments, which means + // this is the time to collect custom Hook signatures. + // Only do this once. This path is hot and runs *inside* every render! + if (!didCollectHooks && hasCustomHooks) { + didCollectHooks = true; + collectCustomHooksForSignature(savedType); + } } - return type; }; } else { throw new Error( diff --git a/packages/react-refresh/src/__tests__/__snapshots__/ReactFreshBabelPlugin-test.js.snap b/packages/react-refresh/src/__tests__/__snapshots__/ReactFreshBabelPlugin-test.js.snap index 424cfeeaf28a0..273d60cbe2a2a 100644 --- a/packages/react-refresh/src/__tests__/__snapshots__/ReactFreshBabelPlugin-test.js.snap +++ b/packages/react-refresh/src/__tests__/__snapshots__/ReactFreshBabelPlugin-test.js.snap @@ -38,11 +38,11 @@ _s4(Bar, "useContext{}"); _c2 = Bar; -const Baz = _s5(memo(_c3 = () => { +const Baz = _s5(memo(_c3 = _s5(() => { _s5(); return useContext(X); -}), "useContext{}"); +}, "useContext{}")), "useContext{}"); _c4 = Baz; @@ -110,21 +110,21 @@ exports[`ReactFreshBabelPlugin generates signatures for function expressions cal var _s = $RefreshSig$(), _s2 = $RefreshSig$(); -export const A = _s(React.memo(_c2 = React.forwardRef(_c = (props, ref) => { +export const A = _s(React.memo(_c2 = _s(React.forwardRef(_c = _s((props, ref) => { _s(); const [foo, setFoo] = useState(0); React.useEffect(() => {}); return

{foo}

; -})), "useState{[foo, setFoo](0)}\\nuseEffect{}"); +}, "useState{[foo, setFoo](0)}\\nuseEffect{}")), "useState{[foo, setFoo](0)}\\nuseEffect{}")), "useState{[foo, setFoo](0)}\\nuseEffect{}"); _c3 = A; -export const B = _s2(React.memo(_c5 = React.forwardRef(_c4 = function (props, ref) { +export const B = _s2(React.memo(_c5 = _s2(React.forwardRef(_c4 = _s2(function (props, ref) { _s2(); const [foo, setFoo] = useState(0); React.useEffect(() => {}); return

{foo}

; -})), "useState{[foo, setFoo](0)}\\nuseEffect{}"); +}, "useState{[foo, setFoo](0)}\\nuseEffect{}")), "useState{[foo, setFoo](0)}\\nuseEffect{}")), "useState{[foo, setFoo](0)}\\nuseEffect{}"); _c6 = B; function hoc() { From 77a9ef321c5efa81f8fc981a72538dfca96742df Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 26 Mar 2021 16:36:03 +0000 Subject: [PATCH 4/8] Sign nested memo/forwardRef too --- .../react-refresh/src/ReactFreshRuntime.js | 26 ++++-- .../__tests__/ReactFreshIntegration-test.js | 82 +++++++++++++++++++ 2 files changed, 100 insertions(+), 8 deletions(-) diff --git a/packages/react-refresh/src/ReactFreshRuntime.js b/packages/react-refresh/src/ReactFreshRuntime.js index d497112f420c6..fc4823709579c 100644 --- a/packages/react-refresh/src/ReactFreshRuntime.js +++ b/packages/react-refresh/src/ReactFreshRuntime.js @@ -355,15 +355,25 @@ export function setSignature( getCustomHooks?: () => Array, ): void { if (__DEV__) { - if (allSignaturesByType.has(type)) { - return; + if (!allSignaturesByType.has(type)) { + allSignaturesByType.set(type, { + forceReset, + ownKey: key, + fullKey: null, + getCustomHooks: getCustomHooks || (() => []), + }); + } + // Visit inner types because we might not have signed them. + if (typeof type === 'object' && type !== null) { + switch (getProperty(type, '$$typeof')) { + case REACT_FORWARD_REF_TYPE: + setSignature(type.render, key, forceReset, getCustomHooks); + break; + case REACT_MEMO_TYPE: + setSignature(type.type, key, forceReset, getCustomHooks); + break; + } } - allSignaturesByType.set(type, { - forceReset, - ownKey: key, - fullKey: null, - getCustomHooks: getCustomHooks || (() => []), - }); } else { throw new Error( 'Unexpected call to React Refresh in a production environment.', diff --git a/packages/react-refresh/src/__tests__/ReactFreshIntegration-test.js b/packages/react-refresh/src/__tests__/ReactFreshIntegration-test.js index 60e9688661e08..231dd6b04219e 100644 --- a/packages/react-refresh/src/__tests__/ReactFreshIntegration-test.js +++ b/packages/react-refresh/src/__tests__/ReactFreshIntegration-test.js @@ -571,6 +571,88 @@ describe('ReactFreshIntegration', () => { } }); + it('does not crash when changing Hook order inside a memo-ed HOC with direct call', () => { + if (__DEV__) { + render(` + const {useEffect, memo} = React; + + function hocWithDirectCall(Wrapped) { + return memo(function Generated() { + return Wrapped(); + }); + } + + export default hocWithDirectCall(() => { + useEffect(() => {}, []); + return

A

; + }); + `); + const el = container.firstChild; + expect(el.textContent).toBe('A'); + + patch(` + const {useEffect, memo} = React; + + function hocWithDirectCall(Wrapped) { + return memo(function Generated() { + return Wrapped(); + }); + } + + export default hocWithDirectCall(() => { + useEffect(() => {}, []); + useEffect(() => {}, []); + return

B

; + }); + `); + // Hook order changed, so we remount. + expect(container.firstChild).not.toBe(el); + const newEl = container.firstChild; + expect(newEl.textContent).toBe('B'); + } + }); + + it('does not crash when changing Hook order inside a memo+forwardRef-ed HOC with direct call', () => { + if (__DEV__) { + render(` + const {useEffect, memo, forwardRef} = React; + + function hocWithDirectCall(Wrapped) { + return memo(forwardRef(function Generated() { + return Wrapped(); + })); + } + + export default hocWithDirectCall(() => { + useEffect(() => {}, []); + return

A

; + }); + `); + const el = container.firstChild; + expect(el.textContent).toBe('A'); + + patch(` + const {useEffect, memo, forwardRef} = React; + + function hocWithDirectCall(Wrapped) { + return memo(forwardRef(function Generated() { + return Wrapped(); + })); + } + + export default hocWithDirectCall(() => { + useEffect(() => {}, []); + useEffect(() => {}, []); + return

B

; + }); + `); + // Hook order changed, so we remount. + expect(container.firstChild).not.toBe(el); + const newEl = container.firstChild; + expect(newEl.textContent).toBe('B'); + } + }); + it('does not crash when changing Hook order inside a HOC returning an object', () => { if (__DEV__) { render(` From d2a2045ba741ee33906ad0b4c6fd5ce6b8563fed Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 30 Mar 2021 14:41:17 +0100 Subject: [PATCH 5/8] Add an IIFE test This is not a case that is important for Fast Refresh, but we shouldn't change the code semantics. This case shows the transform isn't quite correct. It's wrapping the call at the wrong place. --- .../src/__tests__/ReactFreshBabelPlugin-test.js | 12 ++++++++++++ .../__snapshots__/ReactFreshBabelPlugin-test.js.snap | 12 ++++++++++++ 2 files changed, 24 insertions(+) diff --git a/packages/react-refresh/src/__tests__/ReactFreshBabelPlugin-test.js b/packages/react-refresh/src/__tests__/ReactFreshBabelPlugin-test.js index 320cfe17041a6..f56f16069dd77 100644 --- a/packages/react-refresh/src/__tests__/ReactFreshBabelPlugin-test.js +++ b/packages/react-refresh/src/__tests__/ReactFreshBabelPlugin-test.js @@ -524,4 +524,16 @@ describe('ReactFreshBabelPlugin', () => { '". If you want to override this check, pass {skipEnvCheck: true} as plugin options.', ); }); + + it('does not get tripped by IIFEs', () => { + expect( + transform(` + while (item) { + (item => { + useFoo(); + })(item); + } + `), + ).toMatchSnapshot(); + }); }); diff --git a/packages/react-refresh/src/__tests__/__snapshots__/ReactFreshBabelPlugin-test.js.snap b/packages/react-refresh/src/__tests__/__snapshots__/ReactFreshBabelPlugin-test.js.snap index 273d60cbe2a2a..4e2f8eec0518c 100644 --- a/packages/react-refresh/src/__tests__/__snapshots__/ReactFreshBabelPlugin-test.js.snap +++ b/packages/react-refresh/src/__tests__/__snapshots__/ReactFreshBabelPlugin-test.js.snap @@ -86,6 +86,18 @@ var _c; $RefreshReg$(_c, "App"); `; +exports[`ReactFreshBabelPlugin does not get tripped by IIFEs 1`] = ` +while (item) { + var _s = $RefreshSig$(); + + _s(_s(item => { + _s(); + + useFoo(); + }, "useFoo{}", true)(item), "useFoo{}", true); +} +`; + exports[`ReactFreshBabelPlugin generates signatures for function declarations calling hooks 1`] = ` var _s = $RefreshSig$(); From 070262b092c0925d255f907c5d187e35b4ba67b5 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 30 Mar 2021 14:55:12 +0100 Subject: [PATCH 6/8] Find HOCs above more precisely This fixes a false positive that was causing an IIFE to be wrapped in the wrong place, which made the wrapping unsafe. --- .../src/ReactFreshBabelPlugin.js | 24 ++++++++++++++----- .../ReactFreshBabelPlugin-test.js.snap | 4 ++-- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/packages/react-refresh/src/ReactFreshBabelPlugin.js b/packages/react-refresh/src/ReactFreshBabelPlugin.js index 7df35f967b1d3..306bb3ff99191 100644 --- a/packages/react-refresh/src/ReactFreshBabelPlugin.js +++ b/packages/react-refresh/src/ReactFreshBabelPlugin.js @@ -339,14 +339,26 @@ export default function(babel, opts = {}) { if (!path) { return calls; } - if (path.node.type === 'AssignmentExpression') { + let parentPath = path.parentPath; + if (!parentPath) { + return calls; + } + if ( + // hoc(_c = function() { }) + parentPath.node.type === 'AssignmentExpression' && + path.node === parentPath.node.right + ) { // Ignore registrations. - path = path.parentPath; + path = parentPath; continue; } - if (path.node.type === 'CallExpression') { - calls.push(path); - path = path.parentPath; + if ( + // hoc1(hoc2(...)) + parentPath.node.type === 'CallExpression' && + path.node !== parentPath.node.callee + ) { + calls.push(parentPath); + path = parentPath; continue; } return calls; // Stop at other types. @@ -650,7 +662,7 @@ export default function(babel, opts = {}) { // Result: let Foo = () => {}; __signature(Foo, ...); } else { // let Foo = hoc(() => {}) - const paths = [path, ...findHOCCallPathsAbove(path.parentPath)]; + const paths = [path, ...findHOCCallPathsAbove(path)]; paths.forEach(p => { p.replaceWith( t.callExpression( diff --git a/packages/react-refresh/src/__tests__/__snapshots__/ReactFreshBabelPlugin-test.js.snap b/packages/react-refresh/src/__tests__/__snapshots__/ReactFreshBabelPlugin-test.js.snap index 4e2f8eec0518c..7c1276f817880 100644 --- a/packages/react-refresh/src/__tests__/__snapshots__/ReactFreshBabelPlugin-test.js.snap +++ b/packages/react-refresh/src/__tests__/__snapshots__/ReactFreshBabelPlugin-test.js.snap @@ -90,11 +90,11 @@ exports[`ReactFreshBabelPlugin does not get tripped by IIFEs 1`] = ` while (item) { var _s = $RefreshSig$(); - _s(_s(item => { + _s(item => { _s(); useFoo(); - }, "useFoo{}", true)(item), "useFoo{}", true); + }, "useFoo{}", true)(item); } `; From 8ffb8cdbb0c3a0a2543037f8d0e5678405b14aff Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 30 Mar 2021 14:58:33 +0100 Subject: [PATCH 7/8] Be defensive against non-components being passed to setSignature --- packages/react-refresh/src/ReactFreshRuntime.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/react-refresh/src/ReactFreshRuntime.js b/packages/react-refresh/src/ReactFreshRuntime.js index fc4823709579c..a09222a076303 100644 --- a/packages/react-refresh/src/ReactFreshRuntime.js +++ b/packages/react-refresh/src/ReactFreshRuntime.js @@ -658,7 +658,12 @@ export function createSignatureFunctionForTransform() { // Set the signature for all types (even wrappers!) in case // they have no signatures of their own. This is to prevent // problems like https://github.com/facebook/react/issues/20417. - setSignature(type, key, forceReset, getCustomHooks); + if ( + type != null && + (typeof type === 'function' || typeof type === 'object') + ) { + setSignature(type, key, forceReset, getCustomHooks); + } return type; } else { // We're in the _s() call without arguments, which means From 00178dbaf7b5af453807a9d57724568b7c67759b Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 30 Mar 2021 14:58:58 +0100 Subject: [PATCH 8/8] Fix lint --- packages/react-refresh/src/ReactFreshBabelPlugin.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-refresh/src/ReactFreshBabelPlugin.js b/packages/react-refresh/src/ReactFreshBabelPlugin.js index 306bb3ff99191..64013db181a3f 100644 --- a/packages/react-refresh/src/ReactFreshBabelPlugin.js +++ b/packages/react-refresh/src/ReactFreshBabelPlugin.js @@ -339,7 +339,7 @@ export default function(babel, opts = {}) { if (!path) { return calls; } - let parentPath = path.parentPath; + const parentPath = path.parentPath; if (!parentPath) { return calls; }