Skip to content

Commit

Permalink
Flatten ReactSharedInternals (#28783)
Browse files Browse the repository at this point in the history
This is similar to #28771 but for isomorphic. We need a make over for
these dispatchers anyway so this is the first step. Also helps flush out
some internals usage that will break anyway.

It flattens the inner mutable objects onto the ReactSharedInternals.
  • Loading branch information
sebmarkbage authored Apr 8, 2024
1 parent acef2f8 commit d50323e
Show file tree
Hide file tree
Showing 65 changed files with 652 additions and 791 deletions.
7 changes: 3 additions & 4 deletions packages/react-cache/src/ReactCacheOld.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,11 @@ const Pending = 0;
const Resolved = 1;
const Rejected = 2;

const ReactCurrentDispatcher =
React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED
.ReactCurrentDispatcher;
const SharedInternals =
React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED;

function readContext(Context: ReactContext<mixed>) {
const dispatcher = ReactCurrentDispatcher.current;
const dispatcher = SharedInternals.H;
if (dispatcher === null) {
// This wasn't being minified but we're going to retire this package anyway.
// eslint-disable-next-line react-internal/prod-error-codes
Expand Down
18 changes: 9 additions & 9 deletions packages/react-debug-tools/src/ReactDebugHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import {
} from 'shared/ReactSymbols';
import hasOwnProperty from 'shared/hasOwnProperty';

type CurrentDispatcherRef = typeof ReactSharedInternals.ReactCurrentDispatcher;
type CurrentDispatcherRef = typeof ReactSharedInternals;

// Used to track hooks called during a render

Expand Down Expand Up @@ -1075,11 +1075,11 @@ export function inspectHooks<Props>(
// DevTools will pass the current renderer's injected dispatcher.
// Other apps might compile debug hooks as part of their app though.
if (currentDispatcher == null) {
currentDispatcher = ReactSharedInternals.ReactCurrentDispatcher;
currentDispatcher = ReactSharedInternals;
}

const previousDispatcher = currentDispatcher.current;
currentDispatcher.current = DispatcherProxy;
const previousDispatcher = currentDispatcher.H;
currentDispatcher.H = DispatcherProxy;

let readHookLog;
let ancestorStackError;
Expand All @@ -1093,7 +1093,7 @@ export function inspectHooks<Props>(
readHookLog = hookLog;
hookLog = [];
// $FlowFixMe[incompatible-use] found when upgrading Flow
currentDispatcher.current = previousDispatcher;
currentDispatcher.H = previousDispatcher;
}
const rootStack = ErrorStackParser.parse(ancestorStackError);
return buildTree(rootStack, readHookLog);
Expand Down Expand Up @@ -1129,9 +1129,9 @@ function inspectHooksOfForwardRef<Props, Ref>(
ref: Ref,
currentDispatcher: CurrentDispatcherRef,
): HooksTree {
const previousDispatcher = currentDispatcher.current;
const previousDispatcher = currentDispatcher.H;
let readHookLog;
currentDispatcher.current = DispatcherProxy;
currentDispatcher.H = DispatcherProxy;
let ancestorStackError;
try {
ancestorStackError = new Error();
Expand All @@ -1141,7 +1141,7 @@ function inspectHooksOfForwardRef<Props, Ref>(
} finally {
readHookLog = hookLog;
hookLog = [];
currentDispatcher.current = previousDispatcher;
currentDispatcher.H = previousDispatcher;
}
const rootStack = ErrorStackParser.parse(ancestorStackError);
return buildTree(rootStack, readHookLog);
Expand Down Expand Up @@ -1169,7 +1169,7 @@ export function inspectHooksOfFiber(
// DevTools will pass the current renderer's injected dispatcher.
// Other apps might compile debug hooks as part of their app though.
if (currentDispatcher == null) {
currentDispatcher = ReactSharedInternals.ReactCurrentDispatcher;
currentDispatcher = ReactSharedInternals;
}

if (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -453,39 +453,6 @@ describe('ReactHooksInspection', () => {
`);
});

it('should support an injected dispatcher', () => {
const initial = {
useState() {
throw new Error("Should've been proxied");
},
};
let current = initial;
let getterCalls = 0;
const setterCalls = [];
const FakeDispatcherRef = {
get current() {
getterCalls++;
return current;
},
set current(value) {
setterCalls.push(value);
current = value;
},
};

function Foo(props) {
const [state] = FakeDispatcherRef.current.useState('hello world');
return <div>{state}</div>;
}

ReactDebugTools.inspectHooks(Foo, {}, FakeDispatcherRef);

expect(getterCalls).toBe(2);
expect(setterCalls).toHaveLength(2);
expect(setterCalls[0]).not.toBe(initial);
expect(setterCalls[1]).toBe(initial);
});

it('should inspect use() calls for Promise and Context', async () => {
const MyContext = React.createContext('hi');
const promise = Promise.resolve('world');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2345,61 +2345,6 @@ describe('ReactHooksInspectionIntegration', () => {
`);
});

it('should support an injected dispatcher', async () => {
function Foo(props) {
const [state] = React.useState('hello world');
return <div>{state}</div>;
}

const initial = {};
let current = initial;
let getterCalls = 0;
const setterCalls = [];
const FakeDispatcherRef = {
get current() {
getterCalls++;
return current;
},
set current(value) {
setterCalls.push(value);
current = value;
},
};

let renderer;
await act(() => {
renderer = ReactTestRenderer.create(<Foo />, {
unstable_isConcurrent: true,
});
});
const childFiber = renderer.root._currentFiber();

let didCatch = false;

try {
ReactDebugTools.inspectHooksOfFiber(childFiber, FakeDispatcherRef);
} catch (error) {
expect(error.message).toBe('Error rendering inspected component');
expect(error.cause).toBeInstanceOf(Error);
expect(error.cause.message).toBe(
'Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for' +
' one of the following reasons:\n' +
'1. You might have mismatching versions of React and the renderer (such as React DOM)\n' +
'2. You might be breaking the Rules of Hooks\n' +
'3. You might have more than one copy of React in the same app\n' +
'See https://react.dev/link/invalid-hook-call for tips about how to debug and fix this problem.',
);
didCatch = true;
}
// avoid false positive if no error was thrown at all
expect(didCatch).toBe(true);

expect(getterCalls).toBe(1);
expect(setterCalls).toHaveLength(2);
expect(setterCalls[0]).not.toBe(initial);
expect(setterCalls[1]).toBe(initial);
});

// This test case is based on an open source bug report:
// https://github.com/facebookincubator/redux-react-hook/issues/34#issuecomment-466693787
it('should properly advance the current hook for useContext', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ export function describeNativeComponentFrame(
// Note that unlike the code this was forked from (in ReactComponentStackFrame)
// DevTools should override the dispatcher even when DevTools is compiled in production mode,
// because the app itself may be in development mode and log errors/warnings.
const previousDispatcher = currentDispatcherRef.current;
currentDispatcherRef.current = null;
const previousDispatcher = currentDispatcherRef.H;
currentDispatcherRef.H = null;
disableLogs();

// NOTE: keep in sync with the implementation in ReactComponentStackFrame
Expand Down Expand Up @@ -270,7 +270,7 @@ export function describeNativeComponentFrame(

Error.prepareStackTrace = previousPrepareStackTrace;

currentDispatcherRef.current = previousDispatcher;
currentDispatcherRef.H = previousDispatcher;
reenableLogs();
}
// Fallback to just using the name if we couldn't make it throw.
Expand Down
16 changes: 7 additions & 9 deletions packages/react-devtools-shared/src/backend/console.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,15 @@

import type {Fiber} from 'react-reconciler/src/ReactInternalTypes';
import type {
LegacyDispatcherRef,
CurrentDispatcherRef,
ReactRenderer,
WorkTagMap,
ConsolePatchSettings,
} from './types';
import {format, formatWithStyles} from './utils';

import {getInternalReactConstants} from './renderer';
import {getInternalReactConstants, getDispatcherRef} from './renderer';
import {getStackByFiberInDevAndProd} from './DevToolsFiberComponentStack';
import {consoleManagedByDevToolsDuringStrictMode} from 'react-devtools-feature-flags';
import {castBool, castBrowserTheme} from '../utils';
Expand Down Expand Up @@ -75,7 +76,7 @@ type OnErrorOrWarning = (
const injectedRenderers: Map<
ReactRenderer,
{
currentDispatcherRef: CurrentDispatcherRef,
currentDispatcherRef: LegacyDispatcherRef | CurrentDispatcherRef,
getCurrentFiber: () => Fiber | null,
onErrorOrWarning: ?OnErrorOrWarning,
workTagMap: WorkTagMap,
Expand Down Expand Up @@ -215,12 +216,9 @@ export function patch({
// Search for the first renderer that has a current Fiber.
// We don't handle the edge case of stacks for more than one (e.g. interleaved renderers?)
// eslint-disable-next-line no-for-of-loops/no-for-of-loops
for (const {
currentDispatcherRef,
getCurrentFiber,
onErrorOrWarning,
workTagMap,
} of injectedRenderers.values()) {
for (const renderer of injectedRenderers.values()) {
const currentDispatcherRef = getDispatcherRef(renderer);
const {getCurrentFiber, onErrorOrWarning, workTagMap} = renderer;
const current: ?Fiber = getCurrentFiber();
if (current != null) {
try {
Expand All @@ -241,7 +239,7 @@ export function patch({
const componentStack = getStackByFiberInDevAndProd(
workTagMap,
current,
currentDispatcherRef,
(currentDispatcherRef: any),
);
if (componentStack !== '') {
if (isStrictModeOverride(args, method)) {
Expand Down
36 changes: 30 additions & 6 deletions packages/react-devtools-shared/src/backend/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ import type {
RendererInterface,
SerializedElement,
WorkTagMap,
CurrentDispatcherRef,
LegacyDispatcherRef,
} from './types';
import type {
ComponentFilter,
Expand All @@ -140,6 +142,31 @@ type ReactPriorityLevelsType = {
NoPriority: number,
};

export function getDispatcherRef(renderer: {
+currentDispatcherRef?: LegacyDispatcherRef | CurrentDispatcherRef,
...
}): void | CurrentDispatcherRef {
if (renderer.currentDispatcherRef === undefined) {
return undefined;
}
const injectedRef = renderer.currentDispatcherRef;
if (
typeof injectedRef.H === 'undefined' &&
typeof injectedRef.current !== 'undefined'
) {
// We got a legacy dispatcher injected, let's create a wrapper proxy to translate.
return {
get H() {
return (injectedRef: any).current;
},
set H(value) {
(injectedRef: any).current = value;
},
};
}
return (injectedRef: any);
}

function getFiberFlags(fiber: Fiber): number {
// The name of this field changed from "effectTag" to "flags"
return fiber.flags !== undefined ? fiber.flags : (fiber: any).effectTag;
Expand Down Expand Up @@ -694,7 +721,7 @@ export function attach(
getDisplayNameForFiber,
getIsProfiling: () => isProfiling,
getLaneLabelMap,
currentDispatcherRef: renderer.currentDispatcherRef,
currentDispatcherRef: getDispatcherRef(renderer),
workTagMap: ReactTypeOfWork,
reactVersion: version,
});
Expand Down Expand Up @@ -3344,10 +3371,7 @@ export function attach(
}

try {
hooks = inspectHooksOfFiber(
fiber,
(renderer.currentDispatcherRef: any),
);
hooks = inspectHooksOfFiber(fiber, getDispatcherRef(renderer));
} finally {
// Restore original console functionality.
for (const method in originalConsoleMethods) {
Expand Down Expand Up @@ -4571,7 +4595,7 @@ export function attach(
function getComponentStackForFiber(fiber: Fiber): string | null {
let componentStack = fiberToComponentStackMap.get(fiber);
if (componentStack == null) {
const dispatcherRef = renderer.currentDispatcherRef;
const dispatcherRef = getDispatcherRef(renderer);
if (dispatcherRef == null) {
return null;
}
Expand Down
9 changes: 7 additions & 2 deletions packages/react-devtools-shared/src/backend/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,12 @@ export type NativeType = Object;
export type RendererID = number;

type Dispatcher = any;
export type CurrentDispatcherRef = {current: null | Dispatcher};
export type LegacyDispatcherRef = {current: null | Dispatcher};
type SharedInternalsSubset = {
H: null | Dispatcher,
...
};
export type CurrentDispatcherRef = SharedInternalsSubset;

export type GetDisplayNameForFiberID = (
id: number,
Expand Down Expand Up @@ -155,7 +160,7 @@ export type ReactRenderer = {
scheduleUpdate?: ?(fiber: Object) => void,
setSuspenseHandler?: ?(shouldSuspend: (fiber: Object) => boolean) => void,
// Only injected by React v16.8+ in order to support hooks inspection.
currentDispatcherRef?: CurrentDispatcherRef,
currentDispatcherRef?: LegacyDispatcherRef | CurrentDispatcherRef,
// Only injected by React v16.9+ in DEV mode.
// Enables DevTools to append owners-only component stack to error messages.
getCurrentFiber?: () => Fiber | null,
Expand Down
6 changes: 3 additions & 3 deletions packages/react-devtools-shared/src/devtools/cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,11 @@ const Pending = 0;
const Resolved = 1;
const Rejected = 2;

const ReactCurrentDispatcher = (React: any)
.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.ReactCurrentDispatcher;
const ReactSharedInternals = (React: any)
.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED;

function readContext(Context: ReactContext<null>) {
const dispatcher = ReactCurrentDispatcher.current;
const dispatcher = ReactSharedInternals.H;
if (dispatcher === null) {
throw new Error(
'react-cache: read and preload may only be called from within a ' +
Expand Down
Loading

0 comments on commit d50323e

Please sign in to comment.