Skip to content

Commit

Permalink
Use dispatcheronupdate for concurrent mode
Browse files Browse the repository at this point in the history
  • Loading branch information
@tyao1
tyao1 committed Sep 9, 2022
1 parent c80e541 commit ef8c6e6
Show file tree
Hide file tree
Showing 3 changed files with 183 additions and 17 deletions.
33 changes: 25 additions & 8 deletions packages/react-reconciler/src/ReactFiberHooks.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,6 @@ let ignorePreviousDependencies: boolean = false;
function mountHookTypesDev() {
if (__DEV__) {
const hookName = ((currentHookNameInDev: any): HookType);

if (hookTypesDev === null) {
hookTypesDev = [hookName];
} else {
Expand Down Expand Up @@ -269,7 +268,7 @@ function checkDepsAreArrayDev(deps: mixed) {
}
}

function warnOnHookMismatchInDev(currentHookName: HookType) {
function warnOnHookMismatchInDev(currentHookName: ?HookType) {
if (__DEV__) {
const componentName = getComponentNameFromFiber(currentlyRenderingFiber);
if (!didWarnAboutMismatchedHooksForComponent.has(componentName)) {
Expand All @@ -280,12 +279,18 @@ function warnOnHookMismatchInDev(currentHookName: HookType) {

const secondColumnStart = 30;

for (let i = 0; i <= ((hookTypesUpdateIndexDev: any): number); i++) {
const endIndex =
hookTypesUpdateIndexDev < 0
? hookTypesDev.length - 1
: hookTypesUpdateIndexDev;
for (let i = 0; i <= endIndex; i++) {
const oldHookName = hookTypesDev[i];
const newHookName =
i === ((hookTypesUpdateIndexDev: any): number)
(hookTypesUpdateIndexDev < 0
? null
: i === ((hookTypesUpdateIndexDev: any): number)
? currentHookName
: oldHookName;
: oldHookName) ?? undefined;

let row = `${i + 1}. ${oldHookName}`;

Expand Down Expand Up @@ -416,7 +421,10 @@ export function renderWithHooks<Props, SecondArg>(
// Non-stateful hooks (e.g. context) don't get added to memoizedState,
// so memoizedState would be null during updates and mounts.
if (__DEV__) {
if (current !== null && current.memoizedState !== null) {
if (
current !== null &&
(current.mode & ConcurrentMode || current.memoizedState !== null)
) {
ReactCurrentDispatcher.current = HooksDispatcherOnUpdateInDEV;
} else if (hookTypesDev !== null) {
// This dispatcher handles an edge case where a component is updating,
Expand All @@ -430,7 +438,9 @@ export function renderWithHooks<Props, SecondArg>(
}
} else {
ReactCurrentDispatcher.current =
current === null || current.memoizedState === null
current === null ||
((current.mode & ConcurrentMode) === NoMode &&
current.memoizedState === null)
? HooksDispatcherOnMount
: HooksDispatcherOnUpdate;
}
Expand Down Expand Up @@ -485,7 +495,14 @@ export function renderWithHooks<Props, SecondArg>(
ReactCurrentDispatcher.current = ContextOnlyDispatcher;

if (__DEV__) {
workInProgress._debugHookTypes = hookTypesDev;
workInProgress._debugHookTypes = hookTypesDev ?? [];

if (hookTypesDev !== null) {
if (currentHookNameInDev == null && hookTypesDev.length) {
// Renders no hook from some hooks
warnOnHookMismatchInDev(null);
}
}
}

// This check uses currentHook so that it works the same in DEV and prod bundles.
Expand Down
33 changes: 25 additions & 8 deletions packages/react-reconciler/src/ReactFiberHooks.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,6 @@ let ignorePreviousDependencies: boolean = false;
function mountHookTypesDev() {
if (__DEV__) {
const hookName = ((currentHookNameInDev: any): HookType);

if (hookTypesDev === null) {
hookTypesDev = [hookName];
} else {
Expand Down Expand Up @@ -269,7 +268,7 @@ function checkDepsAreArrayDev(deps: mixed) {
}
}

function warnOnHookMismatchInDev(currentHookName: HookType) {
function warnOnHookMismatchInDev(currentHookName: ?HookType) {
if (__DEV__) {
const componentName = getComponentNameFromFiber(currentlyRenderingFiber);
if (!didWarnAboutMismatchedHooksForComponent.has(componentName)) {
Expand All @@ -280,12 +279,18 @@ function warnOnHookMismatchInDev(currentHookName: HookType) {

const secondColumnStart = 30;

for (let i = 0; i <= ((hookTypesUpdateIndexDev: any): number); i++) {
const endIndex =
hookTypesUpdateIndexDev < 0
? hookTypesDev.length - 1
: hookTypesUpdateIndexDev;
for (let i = 0; i <= endIndex; i++) {
const oldHookName = hookTypesDev[i];
const newHookName =
i === ((hookTypesUpdateIndexDev: any): number)
(hookTypesUpdateIndexDev < 0
? null
: i === ((hookTypesUpdateIndexDev: any): number)
? currentHookName
: oldHookName;
: oldHookName) ?? undefined;

let row = `${i + 1}. ${oldHookName}`;

Expand Down Expand Up @@ -416,7 +421,10 @@ export function renderWithHooks<Props, SecondArg>(
// Non-stateful hooks (e.g. context) don't get added to memoizedState,
// so memoizedState would be null during updates and mounts.
if (__DEV__) {
if (current !== null && current.memoizedState !== null) {
if (
current !== null &&
(current.mode & ConcurrentMode || current.memoizedState !== null)
) {
ReactCurrentDispatcher.current = HooksDispatcherOnUpdateInDEV;
} else if (hookTypesDev !== null) {
// This dispatcher handles an edge case where a component is updating,
Expand All @@ -430,7 +438,9 @@ export function renderWithHooks<Props, SecondArg>(
}
} else {
ReactCurrentDispatcher.current =
current === null || current.memoizedState === null
current === null ||
((current.mode & ConcurrentMode) === NoMode &&
current.memoizedState === null)
? HooksDispatcherOnMount
: HooksDispatcherOnUpdate;
}
Expand Down Expand Up @@ -485,7 +495,14 @@ export function renderWithHooks<Props, SecondArg>(
ReactCurrentDispatcher.current = ContextOnlyDispatcher;

if (__DEV__) {
workInProgress._debugHookTypes = hookTypesDev;
workInProgress._debugHookTypes = hookTypesDev ?? [];

if (hookTypesDev !== null) {
if (currentHookNameInDev == null && hookTypesDev.length) {
// Renders no hook from some hooks
warnOnHookMismatchInDev(null);
}
}
}

// This check uses currentHook so that it works the same in DEV and prod bundles.
Expand Down
134 changes: 133 additions & 1 deletion packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3847,6 +3847,39 @@ describe('ReactHooksWithNoopRenderer', () => {
// expect(ReactNoop.getChildren()).toEqual([span('A: 2, B: 3, C: 4')]);
});

it('mount first state', () => {
function App(props) {
let A;
if (props.loadA) {
const [_A] = useState(0);
A = _A;
} else {
A = '[not loaded]';
}

return <Text text={`A: ${A}`} />;
}

ReactNoop.render(<App loadA={false} />);
expect(Scheduler).toFlushAndYield(['A: [not loaded]']);
expect(ReactNoop.getChildren()).toEqual([span('A: [not loaded]')]);

ReactNoop.render(<App loadA={true} />);
expect(() => {
expect(() => {
expect(Scheduler).toFlushAndYield(['A: 0']);
}).toThrow('Rendered more hooks than during the previous render');
}).toErrorDev([
'Warning: React has detected a change in the order of Hooks called by App. ' +
'This will lead to bugs and errors if not fixed. For more information, ' +
'read the Rules of Hooks: https://reactjs.org/link/rules-of-hooks\n\n' +
' Previous render Next render\n' +
' ------------------------------------------------------\n' +
'1. undefined useState\n' +
' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n',
]);
});

it('unmount state', () => {
let updateA;
let updateB;
Expand Down Expand Up @@ -3887,7 +3920,106 @@ describe('ReactHooksWithNoopRenderer', () => {
);
});

it('unmount effects', () => {
it('unmount last state', () => {
function App(props) {
let A;
if (props.loadA) {
const [_A] = useState(0);
A = _A;
} else {
A = '[not loaded]';
}

return <Text text={`A: ${A}`} />;
}

ReactNoop.render(<App loadA={true} />);
expect(Scheduler).toFlushAndYield(['A: 0']);
expect(ReactNoop.getChildren()).toEqual([span('A: 0')]);
ReactNoop.render(<App loadA={false} />);
expect(() => {
expect(Scheduler).toFlushAndYield(['A: [not loaded]']);
}).toErrorDev([
'Warning: React has detected a change in the order of Hooks called by App. ' +
'This will lead to bugs and errors if not fixed. For more information, ' +
'read the Rules of Hooks: https://reactjs.org/link/rules-of-hooks\n\n' +
' Previous render Next render\n' +
' ------------------------------------------------------\n' +
'1. useState undefined\n' +
' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n',
]);
});

it('mount effect', () => {
function App(props) {
if (props.showMore) {
useEffect(() => {
Scheduler.unstable_yieldValue('Mount A');
return () => {
Scheduler.unstable_yieldValue('Unmount A');
};
}, []);
}

return null;
}

ReactNoop.render(<App showMore={false} />);
expect(Scheduler).toFlushAndYield([]);

act(() => {
ReactNoop.render(<App showMore={true} />);
expect(() => {
expect(() => {
expect(Scheduler).toFlushAndYield(['Mount A']);
}).toThrow('Rendered more hooks than during the previous render');
}).toErrorDev([
'Warning: React has detected a change in the order of Hooks called by App. ' +
'This will lead to bugs and errors if not fixed. For more information, ' +
'read the Rules of Hooks: https://reactjs.org/link/rules-of-hooks\n\n' +
' Previous render Next render\n' +
' ------------------------------------------------------\n' +
'1. undefined useEffect\n' +
' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n',
]);
});
});

it('unmount effect', () => {
function App(props) {
if (props.showMore) {
useEffect(() => {
Scheduler.unstable_yieldValue('Mount A');
return () => {
Scheduler.unstable_yieldValue('Unmount A');
};
}, []);
}

return null;
}

ReactNoop.render(<App showMore={true} />);
expect(Scheduler).toFlushAndYield(['Mount A']);
ReactNoop.render(<App loadA={false} />);
expect(() => {
// We don't throw because it would require an additional prod check.
expect(Scheduler).not.toFlushAndThrow(
'Rendered fewer hooks than expected. This may be caused by an ' +
'accidental early return statement.',
);
}).toErrorDev([
'Warning: React has detected a change in the order of Hooks called by App. ' +
'This will lead to bugs and errors if not fixed. For more information, ' +
'read the Rules of Hooks: https://reactjs.org/link/rules-of-hooks\n\n' +
' Previous render Next render\n' +
' ------------------------------------------------------\n' +
'1. useEffect undefined\n' +
' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n',
]);
});

it('unmount additional effects', () => {
function App(props) {
useEffect(() => {
Scheduler.unstable_yieldValue('Mount A');
Expand Down

0 comments on commit ef8c6e6

Please sign in to comment.