Skip to content

Commit

Permalink
Remove client caching from cache() API (facebook#27977)
Browse files Browse the repository at this point in the history
We haven't yet decided how we want `cache` to work on the client. The
lifetime of the cache is more complex than on the server, where it only
has to live as long as a single request.

Since it's more important to ship this on the server, we're removing the
existing behavior from the client for now. On the client (i.e. not a
Server Components environment) `cache` will have not have any caching
behavior. `cache(fn)` will return the function as-is.

We intend to implement client caching in a future major release. In the
meantime, it's only exposed as an API so that Shared Components can use
per-request caching on the server without breaking on the client.
  • Loading branch information
acdlite authored and AndyPengc12 committed Apr 15, 2024
1 parent 65c10b7 commit 5ab7789
Show file tree
Hide file tree
Showing 8 changed files with 1,810 additions and 1,712 deletions.
1,841 changes: 153 additions & 1,688 deletions packages/react-reconciler/src/__tests__/ReactCache-test.js

Large diffs are not rendered by default.

1,597 changes: 1,597 additions & 0 deletions packages/react-reconciler/src/__tests__/ReactCacheElement-test.js

Large diffs are not rendered by default.

51 changes: 30 additions & 21 deletions packages/react-reconciler/src/__tests__/ReactUse-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ let useMemo;
let useEffect;
let Suspense;
let startTransition;
let cache;
let pendingTextRequests;
let waitFor;
let waitForPaint;
Expand All @@ -34,7 +33,6 @@ describe('ReactUse', () => {
useEffect = React.useEffect;
Suspense = React.Suspense;
startTransition = React.startTransition;
cache = React.cache;

const InternalTestUtils = require('internal-test-utils');
waitForAll = InternalTestUtils.waitForAll;
Expand Down Expand Up @@ -643,10 +641,10 @@ describe('ReactUse', () => {
});

test('when waiting for data to resolve, an update on a different root does not cause work to be dropped', async () => {
const getCachedAsyncText = cache(getAsyncText);
const promise = getAsyncText('Hi');

function App() {
return <Text text={use(getCachedAsyncText('Hi'))} />;
return <Text text={use(promise)} />;
}

const root1 = ReactNoop.createRoot();
Expand Down Expand Up @@ -998,39 +996,46 @@ describe('ReactUse', () => {
);

test('load multiple nested Suspense boundaries', async () => {
const getCachedAsyncText = cache(getAsyncText);
const promiseA = getAsyncText('A');
const promiseB = getAsyncText('B');
const promiseC = getAsyncText('C');
assertLog([
'Async text requested [A]',
'Async text requested [B]',
'Async text requested [C]',
]);

function AsyncText({text}) {
return <Text text={use(getCachedAsyncText(text))} />;
function AsyncText({promise}) {
return <Text text={use(promise)} />;
}

const root = ReactNoop.createRoot();
await act(() => {
root.render(
<Suspense fallback={<Text text="(Loading A...)" />}>
<AsyncText text="A" />
<AsyncText promise={promiseA} />
<Suspense fallback={<Text text="(Loading B...)" />}>
<AsyncText text="B" />
<AsyncText promise={promiseB} />
<Suspense fallback={<Text text="(Loading C...)" />}>
<AsyncText text="C" />
<AsyncText promise={promiseC} />
</Suspense>
</Suspense>
</Suspense>,
);
});
assertLog(['Async text requested [A]', '(Loading A...)']);
assertLog(['(Loading A...)']);
expect(root).toMatchRenderedOutput('(Loading A...)');

await act(() => {
resolveTextRequests('A');
});
assertLog(['A', 'Async text requested [B]', '(Loading B...)']);
assertLog(['A', '(Loading B...)']);
expect(root).toMatchRenderedOutput('A(Loading B...)');

await act(() => {
resolveTextRequests('B');
});
assertLog(['B', 'Async text requested [C]', '(Loading C...)']);
assertLog(['B', '(Loading C...)']);
expect(root).toMatchRenderedOutput('AB(Loading C...)');

await act(() => {
Expand Down Expand Up @@ -1584,34 +1589,38 @@ describe('ReactUse', () => {
});

test('regression test: updates while component is suspended should not be mistaken for render phase updates', async () => {
const getCachedAsyncText = cache(getAsyncText);
const promiseA = getAsyncText('A');
const promiseB = getAsyncText('B');
const promiseC = getAsyncText('C');
assertLog([
'Async text requested [A]',
'Async text requested [B]',
'Async text requested [C]',
]);

let setState;
function App() {
const [state, _setState] = useState('A');
const [state, _setState] = useState(promiseA);
setState = _setState;
return <Text text={use(getCachedAsyncText(state))} />;
return <Text text={use(state)} />;
}

// Initial render
const root = ReactNoop.createRoot();
await act(() => root.render(<App />));
assertLog(['Async text requested [A]']);
expect(root).toMatchRenderedOutput(null);
await act(() => resolveTextRequests('A'));
assertLog(['A']);
expect(root).toMatchRenderedOutput('A');

// Update to B. This will suspend.
await act(() => startTransition(() => setState('B')));
assertLog(['Async text requested [B]']);
await act(() => startTransition(() => setState(promiseB)));
expect(root).toMatchRenderedOutput('A');

// While B is suspended, update to C. This should immediately interrupt
// the render for B. In the regression, this update was mistakenly treated
// as a render phase update.
ReactNoop.flushSync(() => setState('C'));
assertLog(['Async text requested [C]']);
ReactNoop.flushSync(() => setState(promiseC));

// Finish rendering.
await act(() => resolveTextRequests('C'));
Expand Down
27 changes: 27 additions & 0 deletions packages/react/src/ReactCacheClient.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/**
* Copyright (c) Meta Platforms, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/

export function cache<A: Iterable<mixed>, T>(fn: (...A) => T): (...A) => T {
// On the client (i.e. not a Server Components environment) `cache` has
// no caching behavior. We just return the function as-is.
//
// We intend to implement client caching in a future major release. In the
// meantime, it's only exposed as an API so that Shared Components can use
// per-request caching on the server without breaking on the client. But it
// does mean they need to be aware of the behavioral difference.
//
// The rest of the behavior is the same as the server implementation — it
// returns a new reference, extra properties like `displayName` are not
// preserved, the length of the new function is 0, etc. That way apps can't
// accidentally depend on those details.
return function () {
// $FlowFixMe[incompatible-call]: We don't want to use rest arguments since we transpile the code.
return fn.apply(null, arguments);
};
}
File renamed without changes.
2 changes: 1 addition & 1 deletion packages/react/src/ReactClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import {createContext} from './ReactContext';
import {lazy} from './ReactLazy';
import {forwardRef} from './ReactForwardRef';
import {memo} from './ReactMemo';
import {cache} from './ReactCache';
import {cache} from './ReactCacheClient';
import {postpone} from './ReactPostpone';
import {
getCacheSignal,
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/ReactServer.experimental.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import {
import {forwardRef} from './ReactForwardRef';
import {lazy} from './ReactLazy';
import {memo} from './ReactMemo';
import {cache} from './ReactCache';
import {cache} from './ReactCacheServer';
import {startTransition} from './ReactStartTransition';
import {postpone} from './ReactPostpone';
import version from 'shared/ReactVersion';
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/ReactServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import {
import {forwardRef} from './ReactForwardRef';
import {lazy} from './ReactLazy';
import {memo} from './ReactMemo';
import {cache} from './ReactCache';
import {cache} from './ReactCacheServer';
import {startTransition} from './ReactStartTransition';
import version from 'shared/ReactVersion';

Expand Down

0 comments on commit 5ab7789

Please sign in to comment.