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.

Sign up for GitHub

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

Jump to bottom

[be] Remove unused, experimental getCacheSignal API #28706

Merged
merged 2 commits into from
Apr 2, 2024
Merged

[be] Remove unused, experimental getCacheSignal API #28706

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5c7d6c3
[be] Remove unused, experimental getCacheSignal API
josephsavona Apr 1, 2024
084402b
fix accidental removal of other export
josephsavona Apr 1, 2024
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
9 changes: 0 additions & 9 deletions packages/react-reconciler/src/ReactFiberCache.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,6 @@ import {enableCache} from 'shared/ReactFeatureFlags';
import {readContext} from './ReactFiberNewContext';
import {CacheContext} from './ReactFiberCacheComponent';

function getCacheSignal(): AbortSignal {
if (!enableCache) {
throw new Error('Not implemented.');
}
const cache: Cache = readContext(CacheContext);
return cache.controller.signal;
}

function getCacheForType<T>(resourceType: () => T): T {
if (!enableCache) {
throw new Error('Not implemented.');
Expand All @@ -36,6 +28,5 @@ function getCacheForType<T>(resourceType: () => T): T {
}

export const DefaultCacheDispatcher: CacheDispatcher = {
getCacheSignal,
getCacheForType,
};
1 change: 0 additions & 1 deletion packages/react-reconciler/src/ReactInternalTypes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,5 @@ export type Dispatcher = {
};

export type CacheDispatcher = {
getCacheSignal: () => AbortSignal,
getCacheForType: <T>(resourceType: () => T) => T,
};
5 changes: 0 additions & 5 deletions packages/react-server/src/ReactFizzCache.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,10 @@

import type {CacheDispatcher} from 'react-reconciler/src/ReactInternalTypes';

function getCacheSignal(): AbortSignal {
throw new Error('Not implemented.');
}

function getCacheForType<T>(resourceType: () => T): T {
throw new Error('Not implemented.');
}

export const DefaultCacheDispatcher: CacheDispatcher = {
getCacheSignal,
getCacheForType,
};
13 changes: 0 additions & 13 deletions packages/react-server/src/flight/ReactFlightServerCache.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,6 @@ import type {CacheDispatcher} from 'react-reconciler/src/ReactInternalTypes';

import {resolveRequest, getCache} from '../ReactFlightServer';

function createSignal(): AbortSignal {
return new AbortController().signal;
}

function resolveCache(): Map<Function, mixed> {
const request = resolveRequest();
if (request) {
Expand All @@ -24,15 +20,6 @@ function resolveCache(): Map<Function, mixed> {
}

export const DefaultCacheDispatcher: CacheDispatcher = {
getCacheSignal(): AbortSignal {
const cache = resolveCache();
let entry: AbortSignal | void = (cache.get(createSignal): any);
if (entry === undefined) {
entry = createSignal();
cache.set(createSignal, entry);
}
return entry;
},
getCacheForType<T>(resourceType: () => T): T {
const cache = resolveCache();
let entry: T | void = (cache.get(resourceType): any);
Expand Down
5 changes: 0 additions & 5 deletions packages/react-suspense-test-utils/src/ReactSuspenseTestUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,9 @@ import ReactSharedInternals from 'shared/ReactSharedInternals';

const ReactCurrentCache = ReactSharedInternals.ReactCurrentCache;

function unsupported() {
throw new Error('This feature is not supported by ReactSuspenseTestUtils.');
}

export function waitForSuspense<T>(fn: () => T): Promise<T> {
const cache: Map<Function, mixed> = new Map();
const testDispatcher: CacheDispatcher = {
getCacheSignal: unsupported,
getCacheForType<R>(resourceType: () => R): R {
let entry: R | void = (cache.get(resourceType): any);
if (entry === undefined) {
Expand Down
1 change: 0 additions & 1 deletion packages/react/index.classic.fb.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ export {
unstable_LegacyHidden,
unstable_Scope,
unstable_SuspenseList,
unstable_getCacheSignal,
unstable_getCacheForType,
unstable_useCacheRefresh,
unstable_useMemoCache,
Expand Down
1 change: 0 additions & 1 deletion packages/react/index.experimental.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ export {
unstable_DebugTracingMode,
unstable_Activity,
unstable_postpone,
unstable_getCacheSignal,
unstable_getCacheForType,
unstable_SuspenseList,
unstable_useCacheRefresh,
Expand Down
1 change: 0 additions & 1 deletion packages/react/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ export {
unstable_Scope,
unstable_SuspenseList,
unstable_TracingMarker,
unstable_getCacheSignal,
unstable_getCacheForType,
unstable_useCacheRefresh,
unstable_useMemoCache,
Expand Down
1 change: 0 additions & 1 deletion packages/react/index.modern.fb.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ export {
unstable_Activity,
unstable_Scope,
unstable_SuspenseList,
unstable_getCacheSignal,
unstable_getCacheForType,
unstable_useCacheRefresh,
unstable_useMemoCache,
Expand Down
2 changes: 0 additions & 2 deletions packages/react/src/ReactClient.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ import {memo} from './ReactMemo';
import {cache} from './ReactCacheClient';
import {postpone} from './ReactPostpone';
import {
getCacheSignal,
getCacheForType,
useCallback,
useContext,
Expand Down Expand Up @@ -115,7 +114,6 @@ export {
REACT_SUSPENSE_LIST_TYPE as unstable_SuspenseList,
REACT_LEGACY_HIDDEN_TYPE as unstable_LegacyHidden,
REACT_OFFSCREEN_TYPE as unstable_Activity,
getCacheSignal as unstable_getCacheSignal,
getCacheForType as unstable_getCacheForType,
useCacheRefresh as unstable_useCacheRefresh,
use,
Expand Down
8 changes: 2 additions & 6 deletions packages/react/src/ReactFetch.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,8 @@ if (enableCache && enableFetchInstrumentation) {
// We're outside a cached scope.
return originalFetch(resource, options);
}
if (
options &&
options.signal &&
options.signal !== dispatcher.getCacheSignal()
Copy link
Contributor Author

@josephsavona josephsavona Apr 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This suggests that maybe someone (Next?) is passing the signal returned from getCacheSignal() back to React. But since getCacheSignal isn't exposed, we should be able to just not do that?

@gnoff is this change cool?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

confirmed offline that Next is not using this API, so it's safe to remove this

) {
// If we're passed a signal that is not ours, then we assume that
if (options && options.signal) {
// If we're passed a signal, then we assume that
// someone else controls the lifetime of this object and opts out of
// caching. It's effectively the opt-out mechanism.
// Ideally we should be able to check this on the Request but
Expand Down
20 changes: 0 additions & 20 deletions packages/react/src/ReactHooks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,26 +43,6 @@ function resolveDispatcher() {
return ((dispatcher: any): Dispatcher);
}

export function getCacheSignal(): AbortSignal {
const dispatcher = ReactCurrentCache.current;
if (!dispatcher) {
// If we have no cache to associate with this call, then we don't know
// its lifetime. We abort early since that's safer than letting it live
// for ever. Unlike just caching which can be a functional noop outside
// of React, these should generally always be associated with some React
// render but we're not limiting quite as much as making it a Hook.
// It's safer than erroring early at runtime.
const controller = new AbortController();
const reason = new Error(
'This CacheSignal was requested outside React which means that it is ' +
'immediately aborted.',
);
controller.abort(reason);
return controller.signal;
}
return dispatcher.getCacheSignal();
}

export function getCacheForType<T>(resourceType: () => T): T {
const dispatcher = ReactCurrentCache.current;
if (!dispatcher) {
Expand Down
2 changes: 0 additions & 2 deletions packages/react/src/ReactServer.experimental.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import {
useDebugValue,
useMemo,
useActionState,
getCacheSignal,
getCacheForType,
} from './ReactHooks';
import {forwardRef} from './ReactForwardRef';
Expand Down Expand Up @@ -78,7 +77,6 @@ export {
startTransition,
REACT_DEBUG_TRACING_MODE_TYPE as unstable_DebugTracingMode,
REACT_SUSPENSE_TYPE as unstable_SuspenseList,
getCacheSignal as unstable_getCacheSignal,
getCacheForType as unstable_getCacheForType,
postpone as unstable_postpone,
useId,
Expand Down