Skip to content
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

[nocommit] Test showing eager state rebase behavior #31592

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
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
7 changes: 6 additions & 1 deletion packages/react-reconciler/src/ReactFiberHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import {
disableLegacyMode,
enableNoCloningMemoCache,
enableContextProfiling,
testingOnlyDisableEagerState,
} from 'shared/ReactFeatureFlags';
import {
REACT_CONTEXT_TYPE,
Expand Down Expand Up @@ -1527,7 +1528,11 @@ function updateReducerImpl<S, A>(
if (update.hasEagerState) {
// If this update is a state update (not a reducer) and was processed eagerly,
// we can use the eagerly computed state
newState = ((update.eagerState: any): S);
if (testingOnlyDisableEagerState) {
newState = reducer(newState, action);
} else {
newState = ((update.eagerState: any): S);
}
} else {
newState = reducer(newState, action);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ let waitFor;
let waitForThrow;
let waitForPaint;
let assertLog;
let flushSync;

describe('ReactHooksWithNoopRenderer', () => {
beforeEach(() => {
Expand All @@ -62,6 +63,7 @@ describe('ReactHooksWithNoopRenderer', () => {
useImperativeHandle = React.useImperativeHandle;
forwardRef = React.forwardRef;
memo = React.memo;
flushSync = React.flushSync;
useTransition = React.useTransition;
useDeferredValue = React.useDeferredValue;
Suspense = React.Suspense;
Expand Down Expand Up @@ -4009,6 +4011,197 @@ describe('ReactHooksWithNoopRenderer', () => {
expect(ReactNoop).toMatchRenderedOutput('1');
});

it('useState early bail out behavior', async () => {
let notify;
let hasUpdates = false;
let aValue = 1;
let bValue = 2;

function handleUpdates(selector) {
if (selector === 'A') {
return aValue;
} else {
return bValue;
}
}

function App({selector}) {
const [state, setState] = useState(1);
const [previousSelector, setSelector] = useState(selector);
Scheduler.log('Render: ' + state);
if (selector !== previousSelector) {
const newState = handleUpdates(selector);
Scheduler.log('Set state in render ' + state + ' -> ' + newState);
setState(newState);
setSelector(selector);
}

notify = latestSelector => {
setState(prevState => {
if (previousSelector !== latestSelector) {
if (hasUpdates) {
const newState = prevState + 1;
Scheduler.log(
`Set state in updater (${latestSelector}): ${prevState} -> ${newState}`,
);
return newState;
}
}
Scheduler.log(
`No change in updater (${latestSelector}): ${prevState}`,
);

return prevState;
});
};

return state;
}

ReactNoop.render(<App selector="A" />);
await waitForAll(['Render: 1']);
expect(ReactNoop).toMatchRenderedOutput('1');

await act(async () => {
ReactNoop.render(<App selector="B" />);

React.startTransition(() => {
notify('A');
});

React.startTransition(() => {
notify('B');
});

hasUpdates = true;
});

if (gate(flags => !flags.testingOnlyDisableEagerState)) {
// If we kept the existing behavior

assertLog([
// Transition updater eagerly runs
'No change in updater (A): 1',
'No change in updater (B): 1',

// Sync update with setState in render
'Render: 1',
'Set state in render 1 -> 2',
'Render: 2',

// Transition rebased on top without re-running reducer
'Render: 1',
]);

expect(ReactNoop).toMatchRenderedOutput('1');
} else {
// If we turned off using the eager state

assertLog([
// Transition updater eagerly runs
'No change in updater (A): 1',
'No change in updater (B): 1',

// Sync update with setState in render
'Render: 1',
'Set state in render 1 -> 2',
'Render: 2',

// Transition rebased on top with reducer
'No change in updater (A): 1',
'Set state in updater (B): 1 -> 2',
'Render: 2',
]);

expect(ReactNoop).toMatchRenderedOutput('2');
}
});

it('useReducer early bail out behavior', async () => {
let notify;
let hasUpdates = false;
let aValue = 1;
let bValue = 2;

function handleUpdates(selector) {
if (selector === 'A') {
return aValue;
} else {
return bValue;
}
}

function App({selector}) {
const [state, setState] = useReducer((state, action) => {
if (typeof action === 'function') {
return action(state);
}
return action;
}, 1);
const [previousSelector, setSelector] = useState(selector);
Scheduler.log('Render: ' + state);
if (selector !== previousSelector) {
const newState = handleUpdates(selector);
Scheduler.log('Set state in render ' + state + ' -> ' + newState);
setState(newState);
setSelector(selector);
}

notify = latestSelector => {
setState(prevState => {
if (previousSelector !== latestSelector) {
if (hasUpdates) {
const newState = prevState + 1;
Scheduler.log(
`Set state in updater (${latestSelector}): ${prevState} -> ${newState}`,
);
return newState;
}
}
Scheduler.log(
`No change in updater (${latestSelector}): ${prevState}`,
);

return prevState;
});
};

return state;
}

ReactNoop.render(<App selector="A" />);
await waitForAll(['Render: 1']);
expect(ReactNoop).toMatchRenderedOutput('1');

await act(async () => {
ReactNoop.render(<App selector="B" />);

React.startTransition(() => {
notify('A');
});

React.startTransition(() => {
notify('B');
});

hasUpdates = true;
});

assertLog([
// Sync update with setState in render
'Render: 1',
'Set state in render 1 -> 2',
'Render: 2',

// Run transitions
'No change in updater (A): 1',
'Set state in updater (B): 1 -> 2',
'Render: 2',
]);

expect(ReactNoop).toMatchRenderedOutput('2');
});

// Regression test. Covers a case where an internal state variable
// (`didReceiveUpdate`) is not reset properly.
it('state bail out edge case (#16359)', async () => {
Expand Down
2 changes: 2 additions & 0 deletions packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -300,3 +300,5 @@ export const enableGetInspectorDataForInstanceInProduction = false;
export const consoleManagedByDevToolsDuringStrictMode = true;

export const enableDO_NOT_USE_disableStrictPassiveEffect = false;

export const testingOnlyDisableEagerState = false;
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,4 @@ export const enableShallowPropDiffing = __VARIANT__;
export const passChildrenWhenCloningPersistedNodes = __VARIANT__;
export const enableFabricCompleteRootInCommitPhase = __VARIANT__;
export const enableSiblingPrerendering = __VARIANT__;
export const testingOnlyDisableEagerState = false;
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ export const retryLaneExpirationMs = 5000;
export const syncLaneExpirationMs = 250;
export const transitionLaneExpirationMs = 5000;
export const useModernStrictMode = true;
export const testingOnlyDisableEagerState = false;

// Flow magic to verify the exports of this file match the original version.
((((null: any): ExportsType): FeatureFlagsType): ExportsType);
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-oss.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ export const enableProfilerTimer = __PROFILE__;
export const enableProfilerCommitHooks = __PROFILE__;
export const enableProfilerNestedUpdatePhase = __PROFILE__;
export const enableUpdaterTracking = __PROFILE__;
export const testingOnlyDisableEagerState = false;

// Flow magic to verify the exports of this file match the original version.
((((null: any): ExportsType): FeatureFlagsType): ExportsType);
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.test-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ export const disableDefaultPropsExceptForClasses = true;

export const enableObjectFiber = false;
export const enableOwnerStacks = false;
export const testingOnlyDisableEagerState = false;

// Flow magic to verify the exports of this file match the original version.
((((null: any): ExportsType): FeatureFlagsType): ExportsType);
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ export const transitionLaneExpirationMs = 5000;
export const useModernStrictMode = true;
export const enableFabricCompleteRootInCommitPhase = false;
export const enableSiblingPrerendering = false;
export const testingOnlyDisableEagerState = false;

// Flow magic to verify the exports of this file match the original version.
((((null: any): ExportsType): FeatureFlagsType): ExportsType);
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ export const enableObjectFiber = false;
export const enableOwnerStacks = false;
export const enableShallowPropDiffing = false;
export const enableSiblingPrerendering = false;
export const testingOnlyDisableEagerState = false;

// Flow magic to verify the exports of this file match the original version.
((((null: any): ExportsType): FeatureFlagsType): ExportsType);
2 changes: 2 additions & 0 deletions packages/shared/forks/ReactFeatureFlags.www-dynamic.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,5 @@ export const enableSiblingPrerendering = __VARIANT__;
export const enableTrustedTypesIntegration = false;
// You probably *don't* want to add more hardcoded ones.
// Instead, try to add them above with the __VARIANT__ value.

export const testingOnlyDisableEagerState = __VARIANT__;
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export const {
retryLaneExpirationMs,
syncLaneExpirationMs,
transitionLaneExpirationMs,
testingOnlyDisableEagerState,
} = dynamicFeatureFlags;

// On WWW, __EXPERIMENTAL__ is used for a new modern build.
Expand Down