Skip to content

Commit

Permalink
Move ReactDOMLegacy implementation into RootFB (#28656)
Browse files Browse the repository at this point in the history
Only the FB entry point has legacy mode now so we can move the remaining
code in there.

Also enable disableLegacyMode in modern www builds since it doesn't
expose those entry points.

Now dependent on #28709.

---------

Co-authored-by: Josh Story <story@hey.com>
  • Loading branch information
sebmarkbage and gnoff authored Apr 3, 2024
1 parent 5de8703 commit 8f55a6a
Show file tree
Hide file tree
Showing 21 changed files with 206 additions and 539 deletions.
3 changes: 3 additions & 0 deletions packages/react-devtools-shell/src/app/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ function mountStrictApp(App) {
}

function mountLegacyApp(App: () => React$Node) {
// $FlowFixMe[prop-missing]: These are removed in 19.
const {render, unmountComponentAtNode} = require('react-dom');

function LegacyRender() {
Expand All @@ -77,8 +78,10 @@ function mountLegacyApp(App: () => React$Node) {

const container = createContainer();

// $FlowFixMe[not-a-function]: These are removed in 19.
render(createElement(LegacyRender), container);

// $FlowFixMe: These are removed in 19.
unmountFunctions.push(() => unmountComponentAtNode(container));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ function mountApp(App: () => React$Node) {

((document.body: any): HTMLBodyElement).appendChild(container);

// $FlowFixMe[prop-missing]: These are removed in 19.
ReactDOM.render(<App />, container);
}
function mountTestApp() {
Expand Down
6 changes: 3 additions & 3 deletions packages/react-dom/index.classic.fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,8 @@ Object.assign((Internals: any), {

export {
createPortal,
findDOMNode,
flushSync,
unmountComponentAtNode,
unstable_createEventHandle,
unstable_renderSubtreeIntoContainer,
unstable_runWithPriority, // DO NOT USE: Temporarily exposed to migrate off of Scheduler.runWithPriority.
useFormStatus,
useFormState,
Expand All @@ -42,6 +39,9 @@ export {
hydrateRoot,
render,
unstable_batchedUpdates,
findDOMNode,
unstable_renderSubtreeIntoContainer,
unmountComponentAtNode,
} from './src/client/ReactDOMRootFB';

export {Internals as __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED};
3 changes: 0 additions & 3 deletions packages/react-dom/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,8 @@ export {
createRoot,
hydrateRoot,
flushSync,
render,
unmountComponentAtNode,
unstable_batchedUpdates,
unstable_createEventHandle,
unstable_renderSubtreeIntoContainer,
unstable_runWithPriority, // DO NOT USE: Temporarily exposed to migrate off of Scheduler.runWithPriority.
useFormStatus,
useFormState,
Expand Down
3 changes: 0 additions & 3 deletions packages/react-dom/index.stable.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,7 @@ export {
createRoot,
hydrateRoot,
flushSync,
render,
unmountComponentAtNode,
unstable_batchedUpdates,
unstable_renderSubtreeIntoContainer,
useFormStatus,
useFormState,
prefetchDNS,
Expand Down
7 changes: 5 additions & 2 deletions packages/react-dom/src/ReactDOMSharedInternals.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
* @flow
*/

import type {FindDOMNodeType} from './client/ReactDOMLegacy.js';
import type {HostDispatcher} from './shared/ReactDOMTypes';

type InternalsType = {
Expand All @@ -16,7 +15,11 @@ type InternalsType = {
ReactDOMCurrentDispatcher: {
current: HostDispatcher,
},
findDOMNode: null | FindDOMNodeType,
findDOMNode:
| null
| ((
componentOrElement: React$Component<any, any>,
) => null | Element | Text),
};

function noop() {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ let act;
let React;
let ReactDOM;
let ReactDOMClient;
let findDOMNode;

const clone = function (o) {
return JSON.parse(JSON.stringify(o));
Expand Down Expand Up @@ -95,8 +94,6 @@ describe('ReactComponentLifeCycle', () => {

React = require('react');
ReactDOM = require('react-dom');
findDOMNode =
ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.findDOMNode;
ReactDOMClient = require('react-dom/client');
});

Expand Down Expand Up @@ -376,6 +373,7 @@ describe('ReactComponentLifeCycle', () => {
expect(instance.updater.isMounted(instance)).toBe(false);
});

// @gate www && !disableLegacyMode
it('warns if legacy findDOMNode is used inside render', async () => {
class Component extends React.Component {
state = {isMounted: false};
Expand All @@ -384,7 +382,7 @@ describe('ReactComponentLifeCycle', () => {
}
render() {
if (this.state.isMounted) {
expect(findDOMNode(this).tagName).toBe('DIV');
expect(ReactDOM.findDOMNode(this).tagName).toBe('DIV');
}
return <div />;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,9 @@ describe('ReactDeprecationWarnings', () => {
});
}
}
expect(() => {
ReactNoop.renderLegacySyncRoot(<Component />);
}).toErrorDev([

ReactNoop.render(<Component />);
await expect(async () => await waitForAll([])).toErrorDev([
'Component "Component" contains the string ref "refComponent". Support for string refs will be removed in a future major release.',
]);
await waitForAll([]);
Expand Down
1 change: 1 addition & 0 deletions packages/react-dom/src/__tests__/ReactTestUtils-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,7 @@ describe('ReactTestUtils', () => {
});

// @gate !disableDOMTestUtils
// @gate !disableLegacyMode
it('should call setState callback with no arguments', async () => {
let mockArgs;
class Component extends React.Component {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,15 @@

const React = require('react');
const ReactDOM = require('react-dom');
const findDOMNode =
ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.findDOMNode;
const StrictMode = React.StrictMode;

describe('findDOMNode', () => {
// @gate www && !disableLegacyMode
it('findDOMNode should return null if passed null', () => {
expect(findDOMNode(null)).toBe(null);
expect(ReactDOM.findDOMNode(null)).toBe(null);
});

// @gate !disableLegacyMode
// @gate www && !disableLegacyMode
it('findDOMNode should find dom element', () => {
class MyNode extends React.Component {
render() {
Expand All @@ -34,13 +33,13 @@ describe('findDOMNode', () => {

const container = document.createElement('div');
const myNode = ReactDOM.render(<MyNode />, container);
const myDiv = findDOMNode(myNode);
const mySameDiv = findDOMNode(myDiv);
const myDiv = ReactDOM.findDOMNode(myNode);
const mySameDiv = ReactDOM.findDOMNode(myDiv);
expect(myDiv.tagName).toBe('DIV');
expect(mySameDiv).toBe(myDiv);
});

// @gate !disableLegacyMode
// @gate www && !disableLegacyMode
it('findDOMNode should find dom element after an update from null', () => {
function Bar({flag}) {
if (flag) {
Expand All @@ -57,23 +56,24 @@ describe('findDOMNode', () => {
const container = document.createElement('div');

const myNodeA = ReactDOM.render(<MyNode />, container);
const a = findDOMNode(myNodeA);
const a = ReactDOM.findDOMNode(myNodeA);
expect(a).toBe(null);

const myNodeB = ReactDOM.render(<MyNode flag={true} />, container);
expect(myNodeA === myNodeB).toBe(true);

const b = findDOMNode(myNodeB);
const b = ReactDOM.findDOMNode(myNodeB);
expect(b.tagName).toBe('SPAN');
});

// @gate www && !disableLegacyMode
it('findDOMNode should reject random objects', () => {
expect(function () {
findDOMNode({foo: 'bar'});
ReactDOM.findDOMNode({foo: 'bar'});
}).toThrowError('Argument appears to not be a ReactComponent. Keys: foo');
});

// @gate !disableLegacyMode
// @gate www && !disableLegacyMode
it('findDOMNode should reject unmounted objects with render func', () => {
class Foo extends React.Component {
render() {
Expand All @@ -85,16 +85,16 @@ describe('findDOMNode', () => {
const inst = ReactDOM.render(<Foo />, container);
ReactDOM.unmountComponentAtNode(container);

expect(() => findDOMNode(inst)).toThrowError(
expect(() => ReactDOM.findDOMNode(inst)).toThrowError(
'Unable to find node on an unmounted component.',
);
});

// @gate !disableLegacyMode
// @gate www && !disableLegacyMode
it('findDOMNode should not throw an error when called within a component that is not mounted', () => {
class Bar extends React.Component {
UNSAFE_componentWillMount() {
expect(findDOMNode(this)).toBeNull();
expect(ReactDOM.findDOMNode(this)).toBeNull();
}

render() {
Expand All @@ -107,7 +107,7 @@ describe('findDOMNode', () => {
}).not.toThrow();
});

// @gate !disableLegacyMode
// @gate www && !disableLegacyMode
it('findDOMNode should warn if used to find a host component inside StrictMode', () => {
let parent = undefined;
let child = undefined;
Expand All @@ -129,7 +129,7 @@ describe('findDOMNode', () => {
);

let match;
expect(() => (match = findDOMNode(parent))).toErrorDev([
expect(() => (match = ReactDOM.findDOMNode(parent))).toErrorDev([
'Warning: findDOMNode is deprecated in StrictMode. ' +
'findDOMNode was passed an instance of ContainsStrictModeChild which renders StrictMode children. ' +
'Instead, add a ref directly to the element you want to reference. ' +
Expand All @@ -141,7 +141,7 @@ describe('findDOMNode', () => {
expect(match).toBe(child);
});

// @gate !disableLegacyMode
// @gate www && !disableLegacyMode
it('findDOMNode should warn if passed a component that is inside StrictMode', () => {
let parent = undefined;
let child = undefined;
Expand All @@ -162,7 +162,7 @@ describe('findDOMNode', () => {
);

let match;
expect(() => (match = findDOMNode(parent))).toErrorDev([
expect(() => (match = ReactDOM.findDOMNode(parent))).toErrorDev([
'Warning: findDOMNode is deprecated in StrictMode. ' +
'findDOMNode was passed an instance of IsInStrictMode which is inside StrictMode. ' +
'Instead, add a ref directly to the element you want to reference. ' +
Expand Down
37 changes: 7 additions & 30 deletions packages/react-dom/src/client/ReactDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,12 @@
*/

import type {ReactNodeList} from 'shared/ReactTypes';
import type {
Container,
PublicInstance,
} from 'react-dom-bindings/src/client/ReactFiberConfigDOM';
import type {
RootType,
HydrateRootOptions,
CreateRootOptions,
} from './ReactDOMRoot';

import {
findDOMNode,
render,
unstable_renderSubtreeIntoContainer,
unmountComponentAtNode,
} from './ReactDOMLegacy';
import {
createRoot as createRootImpl,
hydrateRoot as hydrateRootImpl,
Expand All @@ -35,6 +25,7 @@ import {
flushSync as flushSyncWithoutWarningIfAlreadyRendering,
isAlreadyRendering,
injectIntoDevTools,
findHostInstance,
} from 'react-reconciler/src/ReactFiberReconciler';
import {runWithPriority} from 'react-reconciler/src/ReactEventPriorities';
import {createPortal as createPortalImpl} from 'react-reconciler/src/ReactPortal';
Expand Down Expand Up @@ -99,20 +90,6 @@ function createPortal(
return createPortalImpl(children, container, null, key);
}

function renderSubtreeIntoContainer(
parentComponent: React$Component<any, any>,
element: React$Element<any>,
containerNode: Container,
callback: ?Function,
): React$Component<any, any> | PublicInstance | null {
return unstable_renderSubtreeIntoContainer(
parentComponent,
element,
containerNode,
callback,
);
}

function createRoot(
container: Element | Document | DocumentFragment,
options?: CreateRootOptions,
Expand Down Expand Up @@ -163,6 +140,12 @@ function flushSync<R>(fn: (() => R) | void): R | void {
return flushSyncWithoutWarningIfAlreadyRendering(fn);
}

function findDOMNode(
componentOrElement: React$Component<any, any>,
): null | Element | Text {
return findHostInstance(componentOrElement);
}

// Expose findDOMNode on internals
Internals.findDOMNode = findDOMNode;

Expand All @@ -178,15 +161,9 @@ export {
unstable_batchedUpdates,
flushSync,
ReactVersion as version,
// Disabled behind disableLegacyReactDOMAPIs
findDOMNode,
render,
unmountComponentAtNode,
// exposeConcurrentModeAPIs
createRoot,
hydrateRoot,
// Disabled behind disableUnstableRenderSubtreeIntoContainer
renderSubtreeIntoContainer as unstable_renderSubtreeIntoContainer,
// enableCreateEventHandleAPI
createEventHandle as unstable_createEventHandle,
// TODO: Remove this once callers migrate to alternatives.
Expand Down
Loading

0 comments on commit 8f55a6a

Please sign in to comment.