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.

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

[react-dom] Remove findDOMNode from OSS builds #28267

Merged
merged 1 commit into from
Mar 27, 2024

Conversation

gnoff
Copy link
Collaborator

@gnoff gnoff commented Feb 7, 2024

In the next major findDOMNode is being removed. This PR removes the API from the react-dom entrypoints for OSS builds and re-exposes the implementation as part of internals.

findDOMNode is being retained for Meta builds and so all tests that currently use it will continue to do so by accessing it from internals. Once the replacement API ships in an upcoming minor any tests that were using this API incidentally can be updated to use the new API and any tests asserting findDOMNode's behavior directly can stick around until we remove it entirely (once Meta has moved away from it)

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Feb 7, 2024
Comment on lines -705 to -718

it('supports drilling through to the DOM using findDOMNode', function() {
const ref = React.createRef();
test(React.createElement(Inner, {name: 'foo', ref: ref}), 'DIV', 'foo');
const node = ReactDOM.findDOMNode(ref.current);
expect(node).toBe(container.firstChild);
});
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I had trouble accessing internals from these tests so I just deleted the test. I can undo if there is pushback here but I'd argue that this no longer needs assertions since the API will be removed from public builds

@react-sizebot
Copy link

react-sizebot commented Feb 7, 2024

Comparing: 8436bcc...9928ca5

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 176.92 kB 176.93 kB = 54.99 kB 54.99 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 173.10 kB 173.11 kB +0.02% 53.94 kB 53.95 kB
facebook-www/ReactDOM-prod.classic.js +0.01% 592.31 kB 592.39 kB +0.02% 103.83 kB 103.85 kB
facebook-www/ReactDOM-prod.modern.js +0.13% 573.20 kB 573.97 kB +0.14% 100.71 kB 100.86 kB
test_utils/ReactAllWarnings.js Deleted 65.19 kB 0.00 kB Deleted 16.30 kB 0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-experimental/react-dom/cjs/react-dom.react-server.production.min.js +0.63% 2.69 kB 2.71 kB +1.15% 0.87 kB 0.88 kB
oss-stable-semver/react-dom/cjs/react-dom.react-server.production.min.js +0.63% 2.69 kB 2.71 kB +1.15% 0.87 kB 0.88 kB
oss-stable/react-dom/cjs/react-dom.react-server.production.min.js +0.63% 2.69 kB 2.71 kB +1.15% 0.87 kB 0.88 kB
oss-stable-semver/react-dom/cjs/react-dom-server-rendering-stub.production.min.js +0.47% 3.64 kB 3.66 kB +0.71% 1.27 kB 1.28 kB
oss-stable/react-dom/cjs/react-dom-server-rendering-stub.production.min.js +0.46% 3.67 kB 3.69 kB +0.70% 1.29 kB 1.30 kB
oss-experimental/react-dom/cjs/react-dom-server-rendering-stub.production.min.js +0.46% 3.68 kB 3.69 kB +0.78% 1.29 kB 1.30 kB
oss-stable-semver/react-dom/umd/react-dom-server-rendering-stub.production.min.js +0.45% 3.78 kB 3.80 kB +0.96% 1.36 kB 1.37 kB
oss-stable/react-dom/umd/react-dom-server-rendering-stub.production.min.js +0.45% 3.81 kB 3.82 kB +0.94% 1.38 kB 1.40 kB
oss-experimental/react-dom/umd/react-dom-server-rendering-stub.production.min.js +0.45% 3.81 kB 3.83 kB +1.01% 1.38 kB 1.40 kB
oss-experimental/react-dom/cjs/react-dom.react-server.production.js +0.33% 6.39 kB 6.41 kB +0.86% 1.29 kB 1.30 kB
oss-stable-semver/react-dom/cjs/react-dom.react-server.production.js +0.33% 6.39 kB 6.41 kB +0.86% 1.29 kB 1.30 kB
oss-stable/react-dom/cjs/react-dom.react-server.production.js +0.33% 6.39 kB 6.41 kB +0.86% 1.29 kB 1.30 kB
facebook-www/ReactDOM-dev.modern.js +0.24% 1,650.60 kB 1,654.53 kB +0.25% 328.33 kB 329.16 kB
facebook-www/ReactDOMTesting-dev.modern.js +0.24% 1,671.08 kB 1,675.01 kB +0.25% 332.76 kB 333.58 kB
oss-stable-semver/react-dom/cjs/react-dom-server-rendering-stub.production.js +0.23% 9.17 kB 9.19 kB +0.44% 2.29 kB 2.30 kB
oss-stable/react-dom/cjs/react-dom-server-rendering-stub.production.js +0.23% 9.20 kB 9.22 kB +0.43% 2.32 kB 2.33 kB
oss-experimental/react-dom/cjs/react-dom-server-rendering-stub.production.js +0.23% 9.20 kB 9.22 kB +0.43% 2.32 kB 2.33 kB
test_utils/ReactAllWarnings.js Deleted 65.19 kB 0.00 kB Deleted 16.30 kB 0.00 kB

Generated by 🚫 dangerJS against 9928ca5

@gnoff gnoff force-pushed the move-find-dom-node-to-internals branch from 85d4602 to cd81c4a Compare February 7, 2024 05:24
@@ -14,7 +14,6 @@ export {
createPortal,
createRoot,
hydrateRoot,
findDOMNode,
Copy link
Member

Choose a reason for hiding this comment

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

Should we provide an error like "findDOMNode has been removed, see react.dev/finddomnode for more info"?

@gnoff gnoff added the React 19 label Mar 13, 2024
@gnoff gnoff force-pushed the move-find-dom-node-to-internals branch 3 times, most recently from 9924aa7 to e17f49f Compare March 27, 2024 21:24
Exposes `findDOMNode` on internals and updates tests to read from internals
@gnoff gnoff force-pushed the move-find-dom-node-to-internals branch from e17f49f to 9928ca5 Compare March 27, 2024 21:31
@gnoff gnoff merged commit 9ad40b1 into facebook:main Mar 27, 2024
38 checks passed
@gnoff gnoff deleted the move-find-dom-node-to-internals branch March 27, 2024 21:43
github-actions bot pushed a commit that referenced this pull request Mar 27, 2024
In the next major `findDOMNode` is being removed. This PR removes the
API from the react-dom entrypoints for OSS builds and re-exposes the
implementation as part of internals.

`findDOMNode` is being retained for Meta builds and so all tests that
currently use it will continue to do so by accessing it from internals.
Once the replacement API ships in an upcoming minor any tests that were
using this API incidentally can be updated to use the new API and any
tests asserting `findDOMNode`'s behavior directly can stick around until
we remove it entirely (once Meta has moved away from it)

DiffTrain build for [9ad40b1](9ad40b1)
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
In the next major `findDOMNode` is being removed. This PR removes the
API from the react-dom entrypoints for OSS builds and re-exposes the
implementation as part of internals.

`findDOMNode` is being retained for Meta builds and so all tests that
currently use it will continue to do so by accessing it from internals.
Once the replacement API ships in an upcoming minor any tests that were
using this API incidentally can be updated to use the new API and any
tests asserting `findDOMNode`'s behavior directly can stick around until
we remove it entirely (once Meta has moved away from it)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team React 19
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants