diff --git a/packages/react-fresh/src/ReactFreshBabelPlugin.js b/packages/react-fresh/src/ReactFreshBabelPlugin.js index 54a372fe7891..a93656aaf398 100644 --- a/packages/react-fresh/src/ReactFreshBabelPlugin.js +++ b/packages/react-fresh/src/ReactFreshBabelPlugin.js @@ -8,7 +8,7 @@ 'use strict'; export default function(babel) { - const {types: t, template} = babel; + const {types: t} = babel; const registrationsByProgramPath = new Map(); function createRegistration(programPath, persistentID) { @@ -24,10 +24,6 @@ export default function(babel) { return handle; } - const buildRegistrationCall = template(` - __register__(HANDLE, PERSISTENT_ID); - `); - function isComponentishName(name) { return typeof name === 'string' && name[0] >= 'A' && name[0] <= 'Z'; } @@ -35,6 +31,15 @@ export default function(babel) { function findInnerComponents(inferredName, path, callback) { const node = path.node; switch (node.type) { + case 'Identifier': { + if (!isComponentishName(node.name)) { + return false; + } + // export default hoc(Foo) + // const X = hoc(Foo) + callback(inferredName, node, null); + return true; + } case 'FunctionDeclaration': { // function Foo() {} // export function Foo() {} @@ -158,6 +163,80 @@ export default function(babel) { return false; } + let hookCalls = new WeakMap(); + + function recordHookCall(functionNode, hookCallPath, hookName) { + if (!hookCalls.has(functionNode)) { + hookCalls.set(functionNode, []); + } + let hookCallsForFn = hookCalls.get(functionNode); + let key = ''; + if (hookCallPath.parent.type === 'VariableDeclarator') { + // TODO: if there is no LHS, consider some other heuristic. + key = hookCallPath.parentPath.get('id').getSource(); + } + hookCallsForFn.push({ + name: hookName, + callee: hookCallPath.node.callee, + key, + }); + } + + function isBuiltinHook(hookName) { + switch (hookName) { + case 'useState': + case 'React.useState': + case 'useReducer': + case 'React.useReducer': + case 'useEffect': + case 'React.useEffect': + case 'useLayoutEffect': + case 'React.useLayoutEffect': + case 'useMemo': + case 'React.useMemo': + case 'useCallback': + case 'React.useCallback': + case 'useRef': + case 'React.useRef': + case 'useContext': + case 'React.useContext': + case 'useImperativeMethods': + case 'React.useImperativeMethods': + case 'useDebugValue': + case 'React.useDebugValue': + return true; + default: + return false; + } + } + + function getHookCallsSignature(functionNode) { + const fnHookCalls = hookCalls.get(functionNode); + if (fnHookCalls === undefined) { + return null; + } + return { + key: fnHookCalls.map(call => call.name + '{' + call.key + '}').join('\n'), + customHooks: fnHookCalls + .filter(call => !isBuiltinHook(call.name)) + .map(call => call.callee), + }; + } + + function createArgumentsForSignature(node, signature) { + const {key, customHooks} = signature; + const args = [node, t.stringLiteral(key)]; + if (customHooks.length > 0) { + args.push(t.arrowFunctionExpression([], t.arrayExpression(customHooks))); + } + return args; + } + + let seenForRegistration = new WeakSet(); + let seenForSignature = new WeakSet(); + let seenForHookCalls = new WeakSet(); + let seenForOutro = new WeakSet(); + return { visitor: { ExportDefaultDeclaration(path) { @@ -171,6 +250,15 @@ export default function(babel) { // are currently ignored. return; } + + // Make sure we're not mutating the same tree twice. + // This can happen if another Babel plugin replaces parents. + if (seenForRegistration.has(node)) { + return; + } + seenForRegistration.add(node); + // Don't mutate the tree above this point. + // This code path handles nested cases like: // export default memo(() => {}) // In those cases it is more plausible people will omit names @@ -183,6 +271,13 @@ export default function(babel) { inferredName, declPath, (persistentID, targetExpr, targetPath) => { + if (targetPath === null) { + // For case like: + // export default hoc(Foo) + // we don't want to wrap Foo inside the call. + // Instead we assume it's registered at definition. + return; + } const handle = createRegistration(programPath, persistentID); targetPath.replaceWith( t.assignmentExpression('=', handle, targetExpr), @@ -190,7 +285,158 @@ export default function(babel) { }, ); }, - FunctionDeclaration(path) { + FunctionDeclaration: { + enter(path) { + const node = path.node; + let programPath; + let insertAfterPath; + switch (path.parent.type) { + case 'Program': + insertAfterPath = path; + programPath = path.parentPath; + break; + case 'ExportNamedDeclaration': + insertAfterPath = path.parentPath; + programPath = insertAfterPath.parentPath; + break; + case 'ExportDefaultDeclaration': + insertAfterPath = path.parentPath; + programPath = insertAfterPath.parentPath; + break; + default: + return; + } + const id = node.id; + if (id === null) { + // We don't currently handle anonymous default exports. + return; + } + const inferredName = id.name; + if (!isComponentishName(inferredName)) { + return; + } + + // Make sure we're not mutating the same tree twice. + // This can happen if another Babel plugin replaces parents. + if (seenForRegistration.has(node)) { + return; + } + seenForRegistration.add(node); + // Don't mutate the tree above this point. + + // export function Named() {} + // function Named() {} + findInnerComponents( + inferredName, + path, + (persistentID, targetExpr) => { + const handle = createRegistration(programPath, persistentID); + insertAfterPath.insertAfter( + t.expressionStatement( + t.assignmentExpression('=', handle, targetExpr), + ), + ); + }, + ); + }, + exit(path) { + const node = path.node; + const id = node.id; + if (id === null) { + return; + } + const signature = getHookCallsSignature(node); + if (signature === null) { + return; + } + + // Make sure we're not mutating the same tree twice. + // This can happen if another Babel plugin replaces parents. + if (seenForSignature.has(node)) { + return; + } + seenForSignature.add(node); + // Don't muatte the tree above this point. + + // Unlike with __register__, this needs to work for nested + // declarations too. So we need to search for a path where + // we can insert a statement rather than hardcoding it. + let insertAfterPath = null; + path.find(p => { + if (p.parentPath.isBlock()) { + insertAfterPath = p; + return true; + } + }); + if (insertAfterPath === null) { + return; + } + + insertAfterPath.insertAfter( + t.expressionStatement( + t.callExpression( + t.identifier('__signature__'), + createArgumentsForSignature(id, signature), + ), + ), + ); + }, + }, + 'ArrowFunctionExpression|FunctionExpression': { + exit(path) { + const node = path.node; + const signature = getHookCallsSignature(node); + if (signature === null) { + return; + } + + // Make sure we're not mutating the same tree twice. + // This can happen if another Babel plugin replaces parents. + if (seenForSignature.has(node)) { + return; + } + seenForSignature.add(node); + // Don't mutate the tree above this point. + + if (path.parent.type === 'VariableDeclarator') { + let insertAfterPath = null; + path.find(p => { + if (p.parentPath.isBlock()) { + insertAfterPath = p; + return true; + } + }); + if (insertAfterPath === null) { + return; + } + // Special case when a function would get an inferred name: + // let Foo = () => {} + // let Foo = function() {} + // We'll add signature it on next line so that + // we don't mess up the inferred 'Foo' function name. + insertAfterPath.insertAfter( + t.expressionStatement( + t.callExpression( + t.identifier('__signature__'), + createArgumentsForSignature(path.parent.id, signature), + ), + ), + ); + // Result: let Foo = () => {}; __signature(Foo, ...); + } else { + // let Foo = hoc(() => {}) + path.replaceWith( + t.callExpression( + t.identifier('__signature__'), + createArgumentsForSignature(node, signature), + ), + ); + // Result: let Foo = hoc(__signature(() => {}, ...)) + } + }, + }, + VariableDeclaration(path) { + const node = path.node; let programPath; let insertAfterPath; switch (path.parent.type) { @@ -209,39 +455,15 @@ export default function(babel) { default: return; } - const id = path.node.id; - if (id === null) { - // We don't currently handle anonymous default exports. - return; - } - const inferredName = id.name; - if (!isComponentishName(inferredName)) { + + // Make sure we're not mutating the same tree twice. + // This can happen if another Babel plugin replaces parents. + if (seenForRegistration.has(node)) { return; } - // export function Named() {} - // function Named() {} - findInnerComponents(inferredName, path, (persistentID, targetExpr) => { - const handle = createRegistration(programPath, persistentID); - insertAfterPath.insertAfter( - t.expressionStatement( - t.assignmentExpression('=', handle, targetExpr), - ), - ); - }); - }, - VariableDeclaration(path) { - let programPath; - switch (path.parent.type) { - case 'Program': - programPath = path.parentPath; - break; - case 'ExportNamedDeclaration': - case 'ExportDefaultDeclaration': - programPath = path.parentPath.parentPath; - break; - default: - return; - } + seenForRegistration.add(node); + // Don't mutate the tree above this point. + const declPaths = path.get('declarations'); if (declPaths.length !== 1) { return; @@ -252,29 +474,99 @@ export default function(babel) { inferredName, declPath, (persistentID, targetExpr, targetPath) => { + if (targetPath === null) { + // For case like: + // export const Something = hoc(Foo) + // we don't want to wrap Foo inside the call. + // Instead we assume it's registered at definition. + return; + } const handle = createRegistration(programPath, persistentID); - targetPath.replaceWith( - t.assignmentExpression('=', handle, targetExpr), - ); + if ( + (targetExpr.type === 'ArrowFunctionExpression' || + targetExpr.type === 'FunctionExpression') && + targetPath.parent.type === 'VariableDeclarator' + ) { + // Special case when a function would get an inferred name: + // let Foo = () => {} + // let Foo = function() {} + // We'll register it on next line so that + // we don't mess up the inferred 'Foo' function name. + insertAfterPath.insertAfter( + t.expressionStatement( + t.assignmentExpression('=', handle, declPath.node.id), + ), + ); + // Result: let Foo = () => {}; _c1 = Foo; + } else { + // let Foo = hoc(() => {}) + targetPath.replaceWith( + t.assignmentExpression('=', handle, targetExpr), + ); + // Result: let Foo = _c1 = hoc(() => {}) + } }, ); }, + CallExpression(path) { + const node = path.node; + const callee = node.callee; + + let name = null; + switch (callee.type) { + case 'Identifier': + name = callee.name; + break; + case 'MemberExpression': + name = callee.property.name; + break; + } + if (name === null || !/^use[A-Z]/.test(name)) { + return; + } + + // Make sure we're not recording the same calls twice. + // This can happen if another Babel plugin replaces parents. + if (seenForHookCalls.has(node)) { + return; + } + seenForHookCalls.add(node); + // Don't mutate the tree above this point. + + const fn = path.scope.getFunctionParent(); + if (fn === null) { + return; + } + recordHookCall(fn.block, path, name); + }, Program: { exit(path) { const registrations = registrationsByProgramPath.get(path); if (registrations === undefined) { return; } + + // Make sure we're not mutating the same tree twice. + // This can happen if another Babel plugin replaces parents. + const node = path.node; + if (seenForOutro.has(node)) { + return; + } + seenForOutro.add(node); + // Don't mutate the tree above this point. + registrationsByProgramPath.delete(path); const declarators = []; path.pushContainer('body', t.variableDeclaration('var', declarators)); registrations.forEach(({handle, persistentID}) => { path.pushContainer( 'body', - buildRegistrationCall({ - HANDLE: handle, - PERSISTENT_ID: t.stringLiteral(persistentID), - }), + t.expressionStatement( + t.callExpression(t.identifier('__register__'), [ + handle, + t.stringLiteral(persistentID), + ]), + ), ); declarators.push(t.variableDeclarator(handle)); }); diff --git a/packages/react-fresh/src/ReactFreshRuntime.js b/packages/react-fresh/src/ReactFreshRuntime.js index 0f509788ed7b..da9bd3c4d826 100644 --- a/packages/react-fresh/src/ReactFreshRuntime.js +++ b/packages/react-fresh/src/ReactFreshRuntime.js @@ -14,11 +14,16 @@ import type { import {REACT_MEMO_TYPE, REACT_FORWARD_REF_TYPE} from 'shared/ReactSymbols'; +type Signature = {| + key: string, + getCustomHooks: () => Array, +|}; + // We never remove these associations. // It's OK to reference families, but use WeakMap/Set for types. const allFamiliesByID: Map = new Map(); const allTypes: WeakSet = new WeakSet(); -const allSignaturesByType: WeakMap = new WeakMap(); +const allSignaturesByType: WeakMap = new WeakMap(); // This WeakMap is read by React, so we only put families // that have actually been edited here. This keeps checks fast. const familiesByType: WeakMap = new WeakMap(); @@ -27,6 +32,37 @@ const familiesByType: WeakMap = new WeakMap(); // It is an array of [Family, NextType] tuples. let pendingUpdates: Array<[Family, any]> = []; +function haveEqualSignatures(prevType, nextType) { + const prevSignature = allSignaturesByType.get(prevType); + const nextSignature = allSignaturesByType.get(nextType); + + if (prevSignature === undefined && nextSignature === undefined) { + return true; + } + if (prevSignature === undefined || nextSignature === undefined) { + return false; + } + if (prevSignature.key !== nextSignature.key) { + return false; + } + + // TODO: we might need to calculate previous signature earlier in practice, + // such as during the first time a component is resolved. We'll revisit this. + const prevCustomHooks = prevSignature.getCustomHooks(); + const nextCustomHooks = nextSignature.getCustomHooks(); + if (prevCustomHooks.length !== nextCustomHooks.length) { + return false; + } + + for (let i = 0; i < nextCustomHooks.length; i++) { + if (!haveEqualSignatures(prevCustomHooks[i], nextCustomHooks[i])) { + return false; + } + } + + return true; +} + export function prepareUpdate(): HotUpdate { const staleFamilies = new Set(); const updatedFamilies = new Set(); @@ -42,12 +78,10 @@ export function prepareUpdate(): HotUpdate { family.current = nextType; // Determine whether this should be a re-render or a re-mount. - const prevSignature = allSignaturesByType.get(prevType); - const nextSignature = allSignaturesByType.get(nextType); - if (prevSignature !== nextSignature) { - staleFamilies.add(family); - } else { + if (haveEqualSignatures(prevType, nextType)) { updatedFamilies.add(family); + } else { + staleFamilies.add(family); } }); @@ -98,6 +132,13 @@ export function register(type: any, id: string): void { } } -export function setSignature(type: any, signature: string): void { - allSignaturesByType.set(type, signature); +export function setSignature( + type: any, + key: string, + getCustomHooks?: () => Array, +): void { + allSignaturesByType.set(type, { + key, + getCustomHooks: getCustomHooks || (() => []), + }); } diff --git a/packages/react-fresh/src/__tests__/ReactFresh-test.js b/packages/react-fresh/src/__tests__/ReactFresh-test.js index 1c85697d84be..4cca204c000f 100644 --- a/packages/react-fresh/src/__tests__/ReactFresh-test.js +++ b/packages/react-fresh/src/__tests__/ReactFresh-test.js @@ -72,8 +72,9 @@ describe('ReactFresh', () => { ReactFreshRuntime.register(type, id); } - function __signature__(type, id) { - ReactFreshRuntime.setSignature(type, id); + function __signature__(type, key, getCustomHooks) { + ReactFreshRuntime.setSignature(type, key, getCustomHooks); + return type; } it('can preserve state for compatible types', () => { diff --git a/packages/react-fresh/src/__tests__/ReactFreshBabelPlugin-test.js b/packages/react-fresh/src/__tests__/ReactFreshBabelPlugin-test.js index cbd655d11c12..e5eb1db316f5 100644 --- a/packages/react-fresh/src/__tests__/ReactFreshBabelPlugin-test.js +++ b/packages/react-fresh/src/__tests__/ReactFreshBabelPlugin-test.js @@ -257,7 +257,9 @@ describe('ReactFreshBabelPlugin', () => { } const B = hoc(A); + // This is currently registered as a false positive: const NotAComponent = wow(A); + // We could avoid it but it also doesn't hurt. `), ).toMatchSnapshot(); }); @@ -295,7 +297,89 @@ describe('ReactFreshBabelPlugin', () => { React.createContext(Store); const B = hoc(A); + // This is currently registered as a false positive: const NotAComponent = wow(A); + // We could avoid it but it also doesn't hurt. + `), + ).toMatchSnapshot(); + }); + + it('registers capitalized identifiers in HOC calls', () => { + expect( + transform(` + function Foo() { + return

Hi

; + } + + export default hoc(Foo); + export const A = hoc(Foo); + const B = hoc(Foo); + `), + ).toMatchSnapshot(); + }); + + it('generates signatures for function declarations calling hooks', () => { + expect( + transform(` + export default function App() { + const [foo, setFoo] = useState(0); + React.useEffect(() => {}); + return

{foo}

; + } + `), + ).toMatchSnapshot(); + }); + + it('generates signatures for function expressions calling hooks', () => { + // Unlike __register__, we want to sign all functions -- not just top level. + // This lets us support editing HOCs better. + // For function declarations, __signature__ is called on next line. + // For function expressions, it wraps the expression. + // In order for this to work, __signature__ returns its first argument. + expect( + transform(` + export const A = React.memo(React.forwardRef((props, ref) => { + const [foo, setFoo] = useState(0); + React.useEffect(() => {}); + return

{foo}

; + })); + + export const B = React.memo(React.forwardRef(function(props, ref) { + const [foo, setFoo] = useState(0); + React.useEffect(() => {}); + return

{foo}

; + })); + + function hoc() { + return function Inner() { + const [foo, setFoo] = useState(0); + React.useEffect(() => {}); + return

{foo}

; + }; + } + + export let C = hoc(); + `), + ).toMatchSnapshot(); + }); + + it('includes custom hooks into the signatures', () => { + expect( + transform(` + function useFancyState() { + const [foo, setFoo] = React.useState(0); + useFancyEffect(); + return foo; + } + + const useFancyEffect = () => { + React.useEffect(() => {}); + }; + + export default function App() { + const bar = useFancyState(); + return

{bar}

; + } `), ).toMatchSnapshot(); }); diff --git a/packages/react-fresh/src/__tests__/ReactFreshIntegration-test.js b/packages/react-fresh/src/__tests__/ReactFreshIntegration-test.js index a1c025dc2c63..e50818e92362 100644 --- a/packages/react-fresh/src/__tests__/ReactFreshIntegration-test.js +++ b/packages/react-fresh/src/__tests__/ReactFreshIntegration-test.js @@ -57,10 +57,11 @@ describe('ReactFreshIntegration', () => { }).code; const exportsObj = {}; // eslint-disable-next-line no-new-func - new Function('React', 'exports', '__register__', compiled)( + new Function('React', 'exports', '__register__', '__signature__', compiled)( React, exportsObj, __register__, + __signature__, ); return exportsObj.default; } @@ -75,13 +76,20 @@ describe('ReactFreshIntegration', () => { function patch(source) { execute(source); const hotUpdate = ReactFreshRuntime.prepareUpdate(); - scheduleHotUpdate(lastRoot, hotUpdate); + act(() => { + scheduleHotUpdate(lastRoot, hotUpdate); + }); } function __register__(type, id) { ReactFreshRuntime.register(type, id); } + function __signature__(type, key, getCustomHooks) { + ReactFreshRuntime.setSignature(type, key, getCustomHooks); + return type; + } + it('reloads function declarations', () => { if (__DEV__) { render(` @@ -244,4 +252,443 @@ describe('ReactFreshIntegration', () => { expect(el.textContent).toBe('B2'); } }); + + it('resets state when renaming a state variable', () => { + if (__DEV__) { + render(` + const {useState} = React; + + export default function App() { + const [foo, setFoo] = useState(1); + return

A{foo}

; + } + `); + const el = container.firstChild; + expect(el.textContent).toBe('A1'); + + patch(` + const {useState} = React; + + export default function App() { + const [foo, setFoo] = useState('ignored'); + 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; + + export default function App() { + const [bar, setBar] = useState(2); + 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('C2'); + } + }); + + it('resets state when renaming a state variable in a HOC', () => { + if (__DEV__) { + render(` + const {useState} = React; + + function hoc(Wrapped) { + return function Generated() { + const [foo, setFoo] = useState(1); + return ; + }; + } + + export default hoc(({ value }) => { + return

A{value}

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

B{value}

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

C{value}

; + }); + `); + // Different state variable name, so state is reset. + expect(container.firstChild).not.toBe(el); + const newEl = container.firstChild; + expect(newEl.textContent).toBe('C2'); + } + }); + + it('resets state when renaming a state variable in a HOC with indirection', () => { + if (__DEV__) { + render(` + const {useState} = React; + + function hoc(Wrapped) { + return function Generated() { + const [foo, setFoo] = useState(1); + return ; + }; + } + + function Indirection({ value }) { + return

A{value}

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

B{value}

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

C{value}

; + } + + export default hoc(Indirection); + `); + // Different state variable name, so state is reset. + expect(container.firstChild).not.toBe(el); + const newEl = container.firstChild; + expect(newEl.textContent).toBe('C2'); + } + }); + + it('resets effects while preserving state', () => { + if (__DEV__) { + render(` + const {useState} = React; + + export default function App() { + const [value, setValue] = useState(0); + return

A{value}

; + } + `); + let el = container.firstChild; + expect(el.textContent).toBe('A0'); + + // Add an effect. + patch(` + const {useState} = React; + + export default function App() { + const [value, setValue] = useState(0); + React.useEffect(() => { + const id = setInterval(() => { + setValue(v => v + 1); + }, 1000); + return () => clearInterval(id); + }, []); + return

B{value}

; + } + `); + // We added an effect, thereby changing Hook order. + // This causes a remount. + expect(container.firstChild).not.toBe(el); + el = container.firstChild; + expect(el.textContent).toBe('B0'); + + act(() => { + jest.advanceTimersByTime(1000); + }); + expect(el.textContent).toBe('B1'); + + patch(` + const {useState} = React; + + export default function App() { + const [value, setValue] = useState(0); + React.useEffect(() => { + const id = setInterval(() => { + setValue(v => v + 10); + }, 1000); + return () => clearInterval(id); + }, []); + return

C{value}

; + } + `); + // Same Hooks are called, so state is preserved. + expect(container.firstChild).toBe(el); + expect(el.textContent).toBe('C1'); + + // Effects are always reset, so timer was reinstalled. + // The new version increments by 10 rather than 1. + act(() => { + jest.advanceTimersByTime(1000); + }); + expect(el.textContent).toBe('C11'); + + patch(` + const {useState} = React; + + export default function App() { + const [value, setValue] = useState(0); + return

D{value}

; + } + `); + // Removing the effect changes the signature + // and causes the component to remount. + expect(container.firstChild).not.toBe(el); + el = container.firstChild; + expect(el.textContent).toBe('D0'); + } + }); + + it('does not get confused when custom hooks are reordered', () => { + if (__DEV__) { + render(` + function useFancyState(initialState) { + return React.useState(initialState); + } + + const App = () => { + const [x, setX] = useFancyState('X'); + const [y, setY] = useFancyState('Y'); + return

A{x}{y}

; + }; + + export default App; + `); + let el = container.firstChild; + expect(el.textContent).toBe('AXY'); + + patch(` + function useFancyState(initialState) { + return React.useState(initialState); + } + + const App = () => { + const [x, setX] = useFancyState('X'); + const [y, setY] = useFancyState('Y'); + return

B{x}{y}

; + }; + + export default App; + `); + // Same state variables, so no remount. + expect(container.firstChild).toBe(el); + expect(el.textContent).toBe('BXY'); + + patch(` + function useFancyState(initialState) { + return React.useState(initialState); + } + + const App = () => { + const [y, setY] = useFancyState('Y'); + const [x, setX] = useFancyState('X'); + return

B{x}{y}

; + }; + + export default App; + `); + // Hooks were re-ordered. This causes a remount. + // Therefore, Hook calls don't accidentally share state. + expect(container.firstChild).not.toBe(el); + el = container.firstChild; + expect(el.textContent).toBe('BXY'); + } + }); + + it('remounts component if custom hook it uses changes order', () => { + if (__DEV__) { + render(` + const App = () => { + const [x, setX] = useFancyState('X'); + const [y, setY] = useFancyState('Y'); + return

A{x}{y}

; + }; + + const useFancyState = (initialState) => { + const result = useIndirection(initialState); + return result; + }; + + function useIndirection(initialState) { + return React.useState(initialState); + } + + export default App; + `); + let el = container.firstChild; + expect(el.textContent).toBe('AXY'); + + patch(` + const App = () => { + const [x, setX] = useFancyState('X'); + const [y, setY] = useFancyState('Y'); + return

B{x}{y}

; + }; + + const useFancyState = (initialState) => { + const result = useIndirection(); + return result; + }; + + function useIndirection(initialState) { + return React.useState(initialState); + } + + export default App; + `); + // We didn't change anything except the header text. + // So we don't expect a remount. + expect(container.firstChild).toBe(el); + expect(el.textContent).toBe('BXY'); + + patch(` + const App = () => { + const [x, setX] = useFancyState('X'); + const [y, setY] = useFancyState('Y'); + return

C{x}{y}

; + }; + + const useFancyState = (initialState) => { + const result = useIndirection(initialState); + return result; + }; + + function useIndirection(initialState) { + React.useEffect(() => {}); + return React.useState(initialState); + } + + export default App; + `); + // The useIndirection Hook added an affect, + // so we had to remount the component. + expect(container.firstChild).not.toBe(el); + el = container.firstChild; + expect(el.textContent).toBe('CXY'); + + patch(` + const App = () => { + const [x, setX] = useFancyState('X'); + const [y, setY] = useFancyState('Y'); + return

D{x}{y}

; + }; + + const useFancyState = (initialState) => { + const result = useIndirection(); + return result; + }; + + function useIndirection(initialState) { + React.useEffect(() => {}); + return React.useState(initialState); + } + + export default App; + `); + // We didn't change anything except the header text. + // So we don't expect a remount. + expect(container.firstChild).toBe(el); + expect(el.textContent).toBe('DXY'); + } + }); + + it('does not lose the inferred arrow names', () => { + if (__DEV__) { + render(` + const Parent = () => { + return ; + }; + + const Child = () => { + useMyThing(); + return

{Parent.name} {Child.name} {useMyThing.name}

; + }; + + const useMyThing = () => { + React.useState(); + }; + + export default Parent; + `); + expect(container.textContent).toBe('Parent Child useMyThing'); + } + }); + + it('does not lose the inferred function names', () => { + if (__DEV__) { + render(` + var Parent = function() { + return ; + }; + + var Child = function() { + useMyThing(); + return

{Parent.name} {Child.name} {useMyThing.name}

; + }; + + var useMyThing = function() { + React.useState(); + }; + + export default Parent; + `); + expect(container.textContent).toBe('Parent Child useMyThing'); + } + }); }); diff --git a/packages/react-fresh/src/__tests__/__snapshots__/ReactFreshBabelPlugin-test.js.snap b/packages/react-fresh/src/__tests__/__snapshots__/ReactFreshBabelPlugin-test.js.snap index 622d311311fe..52abe647466d 100644 --- a/packages/react-fresh/src/__tests__/__snapshots__/ReactFreshBabelPlugin-test.js.snap +++ b/packages/react-fresh/src/__tests__/__snapshots__/ReactFreshBabelPlugin-test.js.snap @@ -1,5 +1,61 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`ReactFreshBabelPlugin generates signatures for function declarations calling hooks 1`] = ` +" +export default function App() { + const [foo, setFoo] = useState(0); + React.useEffect(() => {}); + return

{foo}

; +} + +__signature__(App, \\"useState{[foo, setFoo]}\\\\nuseEffect{}\\"); + +_c = App; + +var _c; + +__register__(_c, \\"App\\");" +`; + +exports[`ReactFreshBabelPlugin generates signatures for function expressions calling hooks 1`] = ` +" +export const A = _c3 = React.memo(_c2 = React.forwardRef(_c = __signature__((props, ref) => { + const [foo, setFoo] = useState(0); + React.useEffect(() => {}); + return

{foo}

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

{foo}

; +}, \\"useState{[foo, setFoo]}\\\\nuseEffect{}\\"))); + +function hoc() { + return __signature__(function Inner() { + const [foo, setFoo] = useState(0); + React.useEffect(() => {}); + return

{foo}

; + }, \\"useState{[foo, setFoo]}\\\\nuseEffect{}\\"); +} + +export let C = hoc(); + +var _c, _c2, _c3, _c4, _c5, _c6; + +__register__(_c, \\"A$React.memo$React.forwardRef\\"); + +__register__(_c2, \\"A$React.memo\\"); + +__register__(_c3, \\"A\\"); + +__register__(_c4, \\"B$React.memo$React.forwardRef\\"); + +__register__(_c5, \\"B$React.memo\\"); + +__register__(_c6, \\"B\\");" +`; + exports[`ReactFreshBabelPlugin ignores HOC definitions 1`] = ` " let connect = () => { @@ -51,6 +107,36 @@ exports[`ReactFreshBabelPlugin ignores unnamed function declarations 1`] = ` export default function () {}" `; +exports[`ReactFreshBabelPlugin includes custom hooks into the signatures 1`] = ` +" +function useFancyState() { + const [foo, setFoo] = React.useState(0); + useFancyEffect(); + return foo; +} + +__signature__(useFancyState, \\"useState{[foo, setFoo]}\\\\nuseFancyEffect{}\\", () => [useFancyEffect]); + +const useFancyEffect = () => { + React.useEffect(() => {}); +}; + +__signature__(useFancyEffect, \\"useEffect{}\\"); + +export default function App() { + const bar = useFancyState(); + return

{bar}

; +} + +__signature__(App, \\"useFancyState{bar}\\", () => [useFancyState]); + +_c = App; + +var _c; + +__register__(_c, \\"App\\");" +`; + exports[`ReactFreshBabelPlugin only registers pascal case functions 1`] = ` " function hello() { @@ -58,6 +144,28 @@ function hello() { }" `; +exports[`ReactFreshBabelPlugin registers capitalized identifiers in HOC calls 1`] = ` +" +function Foo() { + return

Hi

; +} + +_c = Foo; +export default _c2 = hoc(Foo); +export const A = _c3 = hoc(Foo); +const B = _c4 = hoc(Foo); + +var _c, _c2, _c3, _c4; + +__register__(_c, \\"Foo\\"); + +__register__(_c2, \\"%default%\\"); + +__register__(_c3, \\"A\\"); + +__register__(_c4, \\"B\\");" +`; + exports[`ReactFreshBabelPlugin registers identifiers used in JSX at definition site 1`] = ` " import A from './A'; @@ -78,15 +186,19 @@ function Foo() { _c2 = Foo; const B = _c3 = hoc(A); -const NotAComponent = wow(A); +// This is currently registered as a false positive: +const NotAComponent = _c4 = wow(A); +// We could avoid it but it also doesn't hurt. -var _c, _c2, _c3; +var _c, _c2, _c3, _c4; __register__(_c, 'Header'); __register__(_c2, 'Foo'); -__register__(_c3, 'B');" +__register__(_c3, 'B'); + +__register__(_c4, 'NotAComponent');" `; exports[`ReactFreshBabelPlugin registers identifiers used in React.createElement at definition site 1`] = ` @@ -111,15 +223,19 @@ _c2 = Foo; React.createContext(Store); const B = _c3 = hoc(A); -const NotAComponent = wow(A); +// This is currently registered as a false positive: +const NotAComponent = _c4 = wow(A); +// We could avoid it but it also doesn't hurt. -var _c, _c2, _c3; +var _c, _c2, _c3, _c4; __register__(_c, 'Header'); __register__(_c2, 'Foo'); -__register__(_c3, 'B');" +__register__(_c3, 'B'); + +__register__(_c4, 'NotAComponent');" `; exports[`ReactFreshBabelPlugin registers likely HOCs with inline functions 1`] = ` @@ -218,13 +334,15 @@ __register__(_c3, 'Baz');" exports[`ReactFreshBabelPlugin registers top-level exported named arrow functions 1`] = ` " -export const Hello = _c = () => { +export const Hello = () => { function handleClick() {} return

Hi

; }; -export let Bar = _c2 = props => ; +_c = Hello; +export let Bar = props => ; +_c2 = Bar; export default (() => { // This one should be ignored. // You should name your components. @@ -260,14 +378,17 @@ __register__(_c2, \\"Bar\\");" exports[`ReactFreshBabelPlugin registers top-level variable declarations with arrow functions 1`] = ` " -let Hello = _c = () => { +let Hello = () => { const handleClick = () => {}; return

Hi

; }; -const Bar = _c2 = () => { +_c = Hello; +const Bar = () => { return ; }; -var Baz = _c3 = () =>
; +_c2 = Bar; +var Baz = () =>
; +_c3 = Baz; var sum = () => {}; var _c, _c2, _c3; @@ -281,13 +402,15 @@ __register__(_c3, \\"Baz\\");" exports[`ReactFreshBabelPlugin registers top-level variable declarations with function expressions 1`] = ` " -let Hello = _c = function () { +let Hello = function () { function handleClick() {} return

Hi

; }; -const Bar = _c2 = function Baz() { +_c = Hello; +const Bar = function Baz() { return ; }; +_c2 = Bar; function sum() {} let Baz = 10; var Qux;