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

Remove legacy dom node/ref stuff. #5495

Conversation

jimfb
Copy link
Contributor

Remove legacy dom node/ref stuff.

As per conversation with @spicyj, the only warning that appears to be firing internally is the .props access, and we think that's because of devtools (filtering out Object.InjectedScript results in zero results), so I think we're good-to-go internally.

force-pushed the remove-public-dom-instance branch from f074de1 to 538d0b0 Compare November 17, 2015 23:30
@sophiebits
Copy link
Collaborator

k

@@ -146,7 +146,7 @@ describe('ReactCompositeComponent', function() {
var anchor = instance.getAnchor();
var actualDOMAnchorNode = ReactDOM.findDOMNode(anchor);
Copy link
Contributor

Choose a reason for hiding this comment

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

you shouldn't need ReactDOM.findDOMNode here, if I'm reading this right? IIRC since the ref is a non-composite, it should directly return the node

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think I agree, we don't need any of the actualDOMAnchorNode stuff. But I didn't want to confuse the diff by modifying test logic unnecessarily.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants