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

Merged
merged 1 commit into from Nov 19, 2015

Conversation

Projects
None yet
4 participants
@jimfb
Copy link
Contributor

commented Nov 17, 2015

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.

@jimfb jimfb force-pushed the jimfb:remove-public-dom-instance branch from add3892 to f074de1 Nov 17, 2015

jim

@jimfb jimfb force-pushed the jimfb:remove-public-dom-instance branch from f074de1 to 538d0b0 Nov 17, 2015

@sophiebits

This comment has been minimized.

Copy link
Collaborator

commented Nov 17, 2015

k

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

This comment has been minimized.

Copy link
@probablyup

probablyup Nov 18, 2015

Contributor

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

This comment has been minimized.

Copy link
@jimfb

jimfb Nov 18, 2015

Author Contributor

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.

jimfb added a commit that referenced this pull request Nov 19, 2015

Merge pull request #5495 from jimfb/remove-public-dom-instance
Remove legacy dom node/ref stuff.

@jimfb jimfb merged commit c643ecd into facebook:master Nov 19, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@renovate renovate bot referenced this pull request Feb 2, 2018

Open

Update dependency react to v0.14.9 #29

0 of 1 task complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.