Skip to content

Commit

Permalink
DevTools resets Store after Fast Refresh force unmount
Browse files Browse the repository at this point in the history
Fast Refresh sometimes force remounts a component in a way that DevTools can't handle. It could throw but that's a bad user experience. Or it could ignore the unmount but then Store might end up with a duplicate node. So instead, a fallback is to completely reset the Store. This is costly but since Fast Refresh is only used in DEV builds, it should be okay.
  • Loading branch information
Brian Vaughn committed May 18, 2021
1 parent 5dd3c57 commit 723f44a
Show file tree
Hide file tree
Showing 13 changed files with 480 additions and 517 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -131,25 +131,48 @@ describe('Fast Refresh', () => {
};
function Child() {
return null;
return <div />;
};
export default Parent;
`);
expect(store).toMatchInlineSnapshot(`
[root]
▾ <Parent>
<Child key="A">
`);

let element = container.firstChild;
expect(container.firstChild).not.toBe(null);

patch(`
function Parent() {
return <Child key="A" />;
};
function Child() {
return <div />;
};
export default Parent;
`);
expect(store).toMatchInlineSnapshot(`
[root]
▾ <Parent>
<Child key="A">
`);

// State is preserved; this verifies that Fast Refresh is wired up.
expect(container.firstChild).toBe(element);
element = container.firstChild;

patch(`
function Parent() {
return <Child key="B" />;
};
function Child() {
return null;
return <div />;
};
export default Parent;
Expand All @@ -159,6 +182,9 @@ describe('Fast Refresh', () => {
▾ <Parent>
<Child key="B">
`);

// State is reset because hooks changed.
expect(container.firstChild).not.toBe(element);
});

it('should not break when there are warnings in between patching', () => {
Expand All @@ -168,10 +194,8 @@ describe('Fast Refresh', () => {
export default function Component() {
const [state, setState] = useState(1);
console.warn("Expected warning during render");
return <div />;
return null;
}
`);
});
Expand All @@ -181,56 +205,56 @@ describe('Fast Refresh', () => {
<Component> ⚠
`);

let element = container.firstChild;

withErrorsOrWarningsIgnored(['Expected warning during render'], () => {
patch(`
const {useState} = React;
const {useEffect, useState} = React;
export default function Component() {
const [state, setState] = useState(1);
console.warn("Expected warning during render");
return <div id="one" />;
return null;
}
`);
});

// This is the same component type, so the warning count carries over.
expect(store).toMatchInlineSnapshot(`
✕ 0, ⚠ 2
[root]
<Component> ⚠
`);

// State is preserved; this verifies that Fast Refresh is wired up.
expect(container.firstChild).toBe(element);
element = container.firstChild;

withErrorsOrWarningsIgnored(['Expected warning during render'], () => {
patch(`
const {useEffect, useState} = React;
export default function Component() {
const [state, setState] = useState(3);
const [state, setState] = useState(1);
useEffect(() => {});
console.warn("Expected warning during render");
return <div id="one" />;
return null;
}
`);
});

// This is a new component type, so the warning count has been reset.
expect(store).toMatchInlineSnapshot(`
✕ 0, ⚠ 1
[root]
<Component> ⚠
`);

// State is reset because hooks changed.
expect(container.firstChild).not.toBe(element);
withErrorsOrWarningsIgnored(['Expected warning during render'], () => {
patch(`
const {useEffect, useState} = React;
export default function Component() {
const [state, setState] = useState(1);
console.warn("Expected warning during render");
return null;
}
`);
});
expect(store).toMatchInlineSnapshot(`
✕ 0, ⚠ 1
[root]
<Component> ⚠
`);
});
});
Loading

0 comments on commit 723f44a

Please sign in to comment.