Skip to content

[refresh] Fix default export signatures for memo/forwardRef/hoc #32705

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions packages/react-refresh/src/ReactFreshBabelPlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,20 @@ export default function (babel, opts = {}) {
if (!foundInside) {
return false;
}
// const Foo = hoc1(hoc2(() => {}))
// export default memo(React.forwardRef(function() {}))
callback(inferredName, node, path);

// TODO: this is a hack, we should find a better way to detect this.
Copy link
Member Author

Choose a reason for hiding this comment

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

I suck at babel, and this call is recursive. Consider this a proof of concept of a fix.

if (
firstArgPath.node.type === 'Identifier' &&
inferredName === '%default%'
) {
// export default memo(function() {}))
// export default forwardRef(function() {}))
callback(innerName, node, path);
} else {
// const Foo = hoc1(hoc2(() => {}))
// export default memo(React.forwardRef(function() {}))
callback(inferredName, node, path);
}
return true;
}
default: {
Expand Down
38 changes: 38 additions & 0 deletions packages/react-refresh/src/__tests__/ReactFreshBabelPlugin-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,44 @@
).toMatchSnapshot();
});

it('registers memo default export', () => {
expect(
transform(`
function Component() {}
export default React.memo(Component);
`),
).toMatchSnapshot();
});

it('registers forwardRef default export', () => {
expect(
transform(`
function Component() {}
export default React.forwardRef(Component);
`),
).toMatchSnapshot();
});

it('registers memo default export', () => {

Check failure on line 227 in packages/react-refresh/src/__tests__/ReactFreshBabelPlugin-test.js

View workflow job for this annotation

GitHub Actions / Run eslint

Test title is used multiple times in the same describe block
expect(
transform(`
import {memo} from 'react';
function Component() {}
export default memo(Component);
`),
).toMatchSnapshot();
});

it('registers forwardRef default export', () => {

Check failure on line 237 in packages/react-refresh/src/__tests__/ReactFreshBabelPlugin-test.js

View workflow job for this annotation

GitHub Actions / Run eslint

Test title is used multiple times in the same describe block
expect(
transform(`
import {forwardRef} from 'react';
function Component() {}
export default forwardRef(Component);
`),
).toMatchSnapshot();
});

it('registers likely HOCs with inline functions', () => {
expect(
transform(`
Expand Down
117 changes: 117 additions & 0 deletions packages/react-refresh/src/__tests__/ReactFreshIntegration-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,123 @@ describe('ReactFreshIntegration', () => {
}
});

it('switches memo to forwardRef with different inner', async () => {
if (__DEV__) {
await render(`
const Child = () => {
return <h1>A1</h1>;
};

export default Child;
`);
let lastElement = container.firstChild;
expect(lastElement.textContent).toBe('A1');
await patch(`
const {memo} = React;

const Child2 = () => {
return <h1>A2</h1>;
};

export default memo(Child2);
`);
expect(container.firstChild !== lastElement).toBe(true);
expect(container.firstChild.textContent).toBe('A2');
lastElement = container.firstChild;

await patch(`
const {forwardRef} = React;

const Child3 = () => {
return <h1>A3</h1>;
};

export default forwardRef(Child3);
`);

expect(container.firstChild !== lastElement).toBe(true);
expect(container.firstChild.textContent).toBe('A3');
lastElement = container.firstChild;

await patch(`
const {memo} = React;

const Child4 = () => {
return <h1>A4</h1>;
};

export default memo(Child4);
`);

expect(container.firstChild !== lastElement).toBe(true);
expect(container.firstChild.textContent).toBe('A4');
lastElement = container.firstChild;

await patch(`
const Child5 = () => {
return <h1>A5</h1>;
};

export default Child5;
`);

expect(container.firstChild !== lastElement).toBe(true);
expect(container.firstChild.textContent).toBe('A5');
}
});

it('switches memo to forwardRef with same inner', async () => {
if (__DEV__) {
await render(`
const Child = () => {
return <h1>A1</h1>;
};

export default Child;
`);
let lastElement = container.firstChild;
expect(lastElement.textContent).toBe('A1');
await patch(`
const {memo} = React;

const Child = () => {
return <h1>A1</h1>;
};

export default memo(Child);
`);

expect(container.firstChild !== lastElement).toBe(true);
expect(container.firstChild.textContent).toBe('A1');
lastElement = container.firstChild;

await patch(`
const {forwardRef} = React;

const Child = () => {
return <h1>A1</h1>;
};

export default forwardRef(Child);
`);

expect(container.firstChild !== lastElement).toBe(true);
expect(container.firstChild.textContent).toBe('A1');
lastElement = container.firstChild;

await patch(`
const Child = () => {
return <h1>A1</h1>;
};

export default Child;
`);

expect(container.firstChild !== lastElement).toBe(true);
expect(container.firstChild.textContent).toBe('A1');
}
});

it('reloads default export with named memo', async () => {
if (__DEV__) {
await render(`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,11 +268,30 @@ const B = hoc(Foo);
_c4 = B;
var _c, _c2, _c3, _c4;
$RefreshReg$(_c, "Foo");
$RefreshReg$(_c2, "%default%");
$RefreshReg$(_c2, "%default%$hoc");
$RefreshReg$(_c3, "A");
$RefreshReg$(_c4, "B");
`;

exports[`ReactFreshBabelPlugin registers forwardRef default export 1`] = `
function Component() {}
_c = Component;
export default _c2 = React.forwardRef(Component);
var _c, _c2;
$RefreshReg$(_c, "Component");
$RefreshReg$(_c2, "%default%$React.forwardRef");
`;

exports[`ReactFreshBabelPlugin registers forwardRef default export 2`] = `
import { forwardRef } from 'react';
function Component() {}
_c = Component;
export default _c2 = forwardRef(Component);
var _c, _c2;
$RefreshReg$(_c, "Component");
$RefreshReg$(_c2, "%default%$forwardRef");
`;

exports[`ReactFreshBabelPlugin registers identifiers used in JSX at definition site 1`] = `
import A from './A';
import Store from './Store';
Expand Down Expand Up @@ -397,6 +416,25 @@ $RefreshReg$(_c2, "%default%$React.memo");
$RefreshReg$(_c3, "%default%");
`;

exports[`ReactFreshBabelPlugin registers memo default export 1`] = `
function Component() {}
_c = Component;
export default _c2 = React.memo(Component);
var _c, _c2;
$RefreshReg$(_c, "Component");
$RefreshReg$(_c2, "%default%$React.memo");
`;

exports[`ReactFreshBabelPlugin registers memo default export 2`] = `
import { memo } from 'react';
function Component() {}
_c = Component;
export default _c2 = memo(Component);
var _c, _c2;
$RefreshReg$(_c, "Component");
$RefreshReg$(_c2, "%default%$memo");
`;

exports[`ReactFreshBabelPlugin registers top-level exported function declarations 1`] = `
export function Hello() {
function handleClick() {}
Expand Down
Loading