-
Notifications
You must be signed in to change notification settings - Fork 47.2k
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
Conversation
|
||
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); | ||
}); |
There was a problem hiding this comment.
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
85d4602
to
cd81c4a
Compare
@@ -14,7 +14,6 @@ export { | |||
createPortal, | |||
createRoot, | |||
hydrateRoot, | |||
findDOMNode, |
There was a problem hiding this comment.
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"
?
9924aa7
to
e17f49f
Compare
Exposes `findDOMNode` on internals and updates tests to read from internals
e17f49f
to
9928ca5
Compare
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)
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)
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 assertingfindDOMNode
's behavior directly can stick around until we remove it entirely (once Meta has moved away from it)