Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Allow more hooks to be added when replaying mount
Currently, if you call setState in render, you must render the exact
same hooks as during the first render pass.

I'm about to add a behavior where if something suspends, we can reuse
the hooks from the previous attempt. That means during initial render,
if something suspends, we should be able to reuse the hooks that were
already created and continue adding more after that. This will error
in the current implementation because of the expectation that every
render produces the same list of hooks.

In this commit, I've changed the logic to allow more hooks to be added
when replaying. But only during a mount — if there's already a current
fiber, then the logic is unchanged, because we shouldn't add any
additional hooks that aren't in the current fiber's list. Mounts are
special because there's no current fiber to compare to.

I haven't change any other behavior yet. The reason I've put this into
its own step is there are a couple tests that intentionally break the
Hook rule, to assert that React errors in these cases, and those happen
to be coupled to the behavior. This is undefined behavior that is always
accompanied by a warning and/or error. So the change should be safe.
  • Loading branch information
acdlite committed Nov 17, 2022
commit f1186578cfcaf78e802aaa29b68189e2d986e4be
Original file line number Diff line number Diff line change
Expand Up @@ -430,26 +430,6 @@ describe('ReactDOMServerHooks', () => {
expect(domNode.textContent).toEqual('hi');
});

itThrowsWhenRendering(
'with a warning for useRef inside useReducer',
async render => {
function App() {
const [value, dispatch] = useReducer((state, action) => {
useRef(0);
return state + 1;
}, 0);
if (value === 0) {
dispatch();
}
return value;
}

const domNode = await render(<App />, 1);
expect(domNode.textContent).toEqual('1');
},
'Rendered more hooks than during the previous render',
);

itRenders('with a warning for useRef inside useState', async render => {
function App() {
const [value] = useState(() => {
Expand Down Expand Up @@ -686,6 +666,32 @@ describe('ReactDOMServerHooks', () => {
);
});

describe('invalid hooks', () => {
it('warns when calling useRef inside useReducer', async () => {
function App() {
const [value, dispatch] = useReducer((state, action) => {
useRef(0);
return state + 1;
}, 0);
if (value === 0) {
dispatch();
}
return value;
}

let error;
try {
await serverRender(<App />);
} catch (x) {
error = x;
}
expect(error).not.toBe(undefined);
expect(error.message).toContain(
'Rendered more hooks than during the previous render',
);
});
});

itRenders(
'can use the same context multiple times in the same function',
async render => {
Expand Down
19 changes: 18 additions & 1 deletion packages/react-reconciler/src/ReactFiberHooks.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -839,7 +839,24 @@ function updateWorkInProgressHook(): Hook {
// Clone from the current hook.

if (nextCurrentHook === null) {
throw new Error('Rendered more hooks than during the previous render.');
const currentFiber = currentlyRenderingFiber.alternate;
if (currentFiber === null) {
// This is the initial render. This branch is reached when the component
// suspends, resumes, then renders an additional hook.
const newHook: Hook = {
memoizedState: null,

baseState: null,
baseQueue: null,
queue: null,

next: null,
};
nextCurrentHook = newHook;
} else {
// This is an update. We should always have a current hook.
throw new Error('Rendered more hooks than during the previous render.');
}
}

currentHook = nextCurrentHook;
Expand Down
19 changes: 18 additions & 1 deletion packages/react-reconciler/src/ReactFiberHooks.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -839,7 +839,24 @@ function updateWorkInProgressHook(): Hook {
// Clone from the current hook.

if (nextCurrentHook === null) {
throw new Error('Rendered more hooks than during the previous render.');
const currentFiber = currentlyRenderingFiber.alternate;
if (currentFiber === null) {
// This is the initial render. This branch is reached when the component
// suspends, resumes, then renders an additional hook.
const newHook: Hook = {
memoizedState: null,

baseState: null,
baseQueue: null,
queue: null,

next: null,
};
nextCurrentHook = newHook;
} else {
// This is an update. We should always have a current hook.
throw new Error('Rendered more hooks than during the previous render.');
}
}

currentHook = nextCurrentHook;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1071,7 +1071,9 @@ describe('ReactHooks', () => {
expect(() => {
expect(() => {
ReactTestRenderer.create(<App />);
}).toThrow('Rendered more hooks than during the previous render.');
}).toThrow(
'Should have a queue. This is likely a bug in React. Please file an issue.',
);
}).toErrorDev([
'Do not call Hooks inside useEffect(...), useMemo(...), or other built-in Hooks',
'Do not call Hooks inside useEffect(...), useMemo(...), or other built-in Hooks',
Expand Down