Skip to content

Commit

Permalink
Pass Suspense config to startTransition
Browse files Browse the repository at this point in the history 
...instead of `useTransition`.

This avoids the problem of having to bind the config object to
`startTransition`, invalidating downstream memoizations.
  • Loading branch information
@acdlite
acdlite committed Nov 15, 2019
1 parent 8e74a31 commit d6db4ec
Show file tree
Hide file tree
Showing 3 changed files with 231 additions and 77 deletions.
98 changes: 34 additions & 64 deletions packages/react-reconciler/src/ReactFiberHooks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,7 @@ export type Dispatcher = {
props: Object,
): ReactEventResponderListener<E, C>,
useDeferredValue<T>(value: T, config: TimeoutConfig | void | null): T,
useTransition(
config: SuspenseConfig | void | null,
): [(() => void) => void, boolean],
useTransition(): [(() => void) => void, boolean],
};

type Update<S, A> = {
Expand Down Expand Up @@ -1172,50 +1170,32 @@ function updateDeferredValue<T>(
return prevValue;
}

function mountTransition(
config: SuspenseConfig | void | null,
): [(() => void) => void, boolean] {
function startTransition(setPending, callback, config) {
setPending(true);
Scheduler.unstable_next(() => {
const previousConfig = ReactCurrentBatchConfig.suspense;
ReactCurrentBatchConfig.suspense = config === undefined ? null : config;
try {
setPending(false);
callback();
} finally {
ReactCurrentBatchConfig.suspense = previousConfig;
}
});
}

function mountTransition(): [(() => void) => void, boolean] {
const [isPending, setPending] = mountState(false);
const startTransition = mountCallback(
callback => {
setPending(true);
Scheduler.unstable_next(() => {
const previousConfig = ReactCurrentBatchConfig.suspense;
ReactCurrentBatchConfig.suspense = config === undefined ? null : config;
try {
setPending(false);
callback();
} finally {
ReactCurrentBatchConfig.suspense = previousConfig;
}
});
},
[config, isPending],
);
return [startTransition, isPending];
const hook = mountWorkInProgressHook();
const start = (hook.memoizedState = startTransition.bind(null, setPending));
return [start, isPending];
}

function updateTransition(
config: SuspenseConfig | void | null,
): [(() => void) => void, boolean] {
const [isPending, setPending] = updateState(false);
const startTransition = updateCallback(
callback => {
setPending(true);
Scheduler.unstable_next(() => {
const previousConfig = ReactCurrentBatchConfig.suspense;
ReactCurrentBatchConfig.suspense = config === undefined ? null : config;
try {
setPending(false);
callback();
} finally {
ReactCurrentBatchConfig.suspense = previousConfig;
}
});
},
[config, isPending],
);
return [startTransition, isPending];
function updateTransition(): [(() => void) => void, boolean] {
const [isPending] = updateState(false);
const hook = updateWorkInProgressHook();
const start = hook.memoizedState;
return [start, isPending];
}

function dispatchAction<S, A>(
Expand Down Expand Up @@ -1553,12 +1533,10 @@ if (__DEV__) {
mountHookTypesDev();
return mountDeferredValue(value, config);
},
useTransition(
config: SuspenseConfig | void | null,
): [(() => void) => void, boolean] {
useTransition(): [(() => void) => void, boolean] {
currentHookNameInDev = 'useTransition';
mountHookTypesDev();
return mountTransition(config);
return mountTransition();
},
};

Expand Down Expand Up @@ -1670,12 +1648,10 @@ if (__DEV__) {
updateHookTypesDev();
return mountDeferredValue(value, config);
},
useTransition(
config: SuspenseConfig | void | null,
): [(() => void) => void, boolean] {
useTransition(): [(() => void) => void, boolean] {
currentHookNameInDev = 'useTransition';
updateHookTypesDev();
return mountTransition(config);
return mountTransition();
},
};

Expand Down Expand Up @@ -1787,12 +1763,10 @@ if (__DEV__) {
updateHookTypesDev();
return updateDeferredValue(value, config);
},
useTransition(
config: SuspenseConfig | void | null,
): [(() => void) => void, boolean] {
useTransition(): [(() => void) => void, boolean] {
currentHookNameInDev = 'useTransition';
updateHookTypesDev();
return updateTransition(config);
return updateTransition();
},
};

Expand Down Expand Up @@ -1917,13 +1891,11 @@ if (__DEV__) {
mountHookTypesDev();
return mountDeferredValue(value, config);
},
useTransition(
config: SuspenseConfig | void | null,
): [(() => void) => void, boolean] {
useTransition(): [(() => void) => void, boolean] {
currentHookNameInDev = 'useTransition';
warnInvalidHookAccess();
mountHookTypesDev();
return mountTransition(config);
return mountTransition();
},
};

Expand Down Expand Up @@ -2048,13 +2020,11 @@ if (__DEV__) {
updateHookTypesDev();
return updateDeferredValue(value, config);
},
useTransition(
config: SuspenseConfig | void | null,
): [(() => void) => void, boolean] {
useTransition(): [(() => void) => void, boolean] {
currentHookNameInDev = 'useTransition';
warnInvalidHookAccess();
updateHookTypesDev();
return updateTransition(config);
return updateTransition();
},
};
}
204 changes: 195 additions & 9 deletions packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1977,16 +1977,18 @@ describe('ReactHooksWithNoopRenderer', () => {
it.experimental(
'delays showing loading state until after timeout',
async () => {
const SUSPENSE_CONFIG = {
timeoutMs: 1000,
};

let transition;
function App() {
const [show, setShow] = useState(false);
const [startTransition, isPending] = useTransition({
timeoutMs: 1000,
});
const [startTransition, isPending] = useTransition();
transition = () => {
startTransition(() => {
setShow(true);
});
}, SUSPENSE_CONFIG);
};
return (
<Suspense
Expand Down Expand Up @@ -2043,17 +2045,19 @@ describe('ReactHooksWithNoopRenderer', () => {
it.experimental(
'delays showing loading state until after busyDelayMs + busyMinDurationMs',
async () => {
const SUSPENSE_CONFIG = {
busyDelayMs: 1000,
busyMinDurationMs: 2000,
};

let transition;
function App() {
const [show, setShow] = useState(false);
const [startTransition, isPending] = useTransition({
busyDelayMs: 1000,
busyMinDurationMs: 2000,
});
const [startTransition, isPending] = useTransition();
transition = () => {
startTransition(() => {
setShow(true);
});
}, SUSPENSE_CONFIG);
};
return (
<Suspense
Expand Down Expand Up @@ -2111,6 +2115,188 @@ describe('ReactHooksWithNoopRenderer', () => {
]);
},
);

it.experimental('always returns the same startTransition', async () => {
const SUSPENSE_CONFIG = {
busyDelayMs: 1000,
busyMinDurationMs: 2000,
};

let transition;
function App() {
const [step, setStep] = useState(0);
const [startTransition, isPending] = useTransition();
// Log whenever startTransition changes
useEffect(
() => {
Scheduler.unstable_yieldValue('New startTransition function');
},
[startTransition],
);
transition = () => {
startTransition(() => {
setStep(n => n + 1);
}, SUSPENSE_CONFIG);
};
return (
<Suspense fallback={<Text text="Loading..." />}>
<AsyncText key={step} ms={2000} text={`Step: ${step}`} />
{isPending && <Text text="(pending...)" />}
</Suspense>
);
}

const root = ReactNoop.createRoot();
await ReactNoop.act(async () => {
root.render(<App />);
});
expect(Scheduler).toHaveYielded([
'Suspend! [Step: 0]',
'Loading...',
// Initial mount. This should never be logged again.
'New startTransition function',
]);
await ReactNoop.act(async () => {
await advanceTimers(2000);
});
expect(Scheduler).toHaveYielded([
'Promise resolved [Step: 0]',
'Step: 0',
]);

// Update. The effect should not fire.
await ReactNoop.act(async () => {
Scheduler.unstable_runWithPriority(
Scheduler.unstable_UserBlockingPriority,
transition,
);
});
expect(Scheduler).toHaveYielded([
'Step: 0',
'(pending...)',
'Suspend! [Step: 1]',
'Loading...',
// No log effect, because startTransition did not change
]);
});

it.experimental(
'can pass different suspense configs per transition',
async () => {
let transition;
function App({timeoutMs}) {
const [step, setStep] = useState(0);
const [startTransition, isPending] = useTransition();
// Log whenever startTransition changes
useEffect(
() => {
Scheduler.unstable_yieldValue('New startTransition function');
},
[startTransition],
);
transition = () => {
startTransition(
() => {
setStep(n => n + 1);
},
{timeoutMs},
);
};
return (
<Suspense fallback={<Text text="Loading..." />}>
<AsyncText key={step} ms={2000} text={`Step: ${step}`} />
{isPending && <Text text="(pending...)" />}
</Suspense>
);
}

const root = ReactNoop.createRoot();
await ReactNoop.act(async () => {
root.render(<App timeoutMs={500} />);
});
expect(Scheduler).toHaveYielded([
'Suspend! [Step: 0]',
'Loading...',
// Initial mount. This should never be logged again.
'New startTransition function',
]);
await ReactNoop.act(async () => {
await advanceTimers(2000);
});
expect(Scheduler).toHaveYielded([
'Promise resolved [Step: 0]',
'Step: 0',
]);

// Schedule a transition. Should timeout quickly.
await ReactNoop.act(async () => {
Scheduler.unstable_runWithPriority(
Scheduler.unstable_UserBlockingPriority,
transition,
);

expect(Scheduler).toFlushAndYield([
'Step: 0',
'(pending...)',
'Suspend! [Step: 1]',
'Loading...',
// No log effect, because startTransition did not change
]);

// Advance time. This should be sufficient to timeout.
await advanceTimers(1000);
expect(Scheduler).toFlushAndYield([]);
// Show placeholder.
expect(root).toMatchRenderedOutput(
<>
<span prop="Step: 0" hidden={true} />
<span prop="(pending...)" hidden={true} />
<span prop="Loading..." />
</>,
);
// Resolve the promise
await advanceTimers(10000);
});
expect(Scheduler).toHaveYielded([
'Promise resolved [Step: 1]',
'Step: 1',
]);

// Increase the timeout threshold
await ReactNoop.act(async () => {
root.render(<App timeoutMs={5000} />);
});
expect(Scheduler).toHaveYielded(['Step: 1']);

// Schedule a transition again. This time it should take longer
// to timeout.
await ReactNoop.act(async () => {
Scheduler.unstable_runWithPriority(
Scheduler.unstable_UserBlockingPriority,
transition,
);

expect(Scheduler).toFlushAndYield([
'Step: 1',
'(pending...)',
'Suspend! [Step: 2]',
'Loading...',
// No log effect, because startTransition did not change
]);

// Advance time. This should *not* be sufficient to timeout.
await advanceTimers(1000);
expect(Scheduler).toFlushAndYield([]);
// Still showing pending state, no placeholder.
expect(root).toMatchRenderedOutput(
<>
<span prop="Step: 1" />
<span prop="(pending...)" />
</>,
);
});
},
);
});
describe('useDeferredValue', () => {
it.experimental('defers text value until specified timeout', async () => {
Expand Down
Loading

0 comments on commit d6db4ec

Please sign in to comment.