Added findDOMNode, as we move toward deprecating g#2646
Uh oh!
There was an error while loading. Please reload this page.
Conversation
syranide commented Dec 3, 2014
Nevermind me... or? |
jimfb commented Dec 3, 2014
The bigger thing is that we're moving it off the component and into a static helper/utility function. That's the structural change, the name change is more of an "oh, while we're making the api change here, let's fix the name also, so users don't need to do two migrations" I don't remember all the reasons we didn't like "get", but I think one of them might have been related to the fact that we may not have a 1-to-1 mapping from react components to dom nodes ([might be 1-to-0 today, might be extended to 1-to-n in future] - maybe I just made that up), and "get" has a connotation of being an instance method with a 1-to-1 mapping. Regardless, I know we were talking about "queryDOMNode" and "selectDOMNode", both of which were too easily confused with jquery selectors. Sema indicated we should go with "findDOMNode" |
zpao commented Dec 3, 2014
@syranide no, you didn't miss any explicit discussion. This is part of the decoupling that @sebmarkbage has been working on but some of the details haven't been well communicated. Specifically in this case, I'm not entirely happy with naming so we could probably bikeshed some more if you want. I'm personally leaning towards |
src/core/findDOMNode.js Outdated
There was a problem hiding this comment.
We'll want a better message here. Also, we need to use printf format (with only %s), not string concat. invariant(condition, 'some %s string %s', 1, 2)
jimfbforce-pushed the getDOMNode-becomes-findDOMNode branch 3 times, most recently from e419094 to 414717cComparesrc/core/findDOMNode.js Outdated
jimfbforce-pushed the getDOMNode-becomes-findDOMNode branch from 414717c to 1d55163Comparejimfbforce-pushed the getDOMNode-becomes-findDOMNode branch 2 times, most recently from fe77ef8 to 0308e03Compare
sebmarkbage commented Dec 18, 2014
This should move into the browser folder since it's DOM specific. |
src/core/findDOMNode.js Outdated
There was a problem hiding this comment.
I think you meant to check typeof componentOrElement.render === 'function', not the componentOrElement itself. Please add a unit test that covers this error message case.
jimfbforce-pushed the getDOMNode-becomes-findDOMNode branch 3 times, most recently from 903f056 to 0810b08Comparejimfbforce-pushed the getDOMNode-becomes-findDOMNode branch from 0810b08 to 8bbc60cComparejimfbforce-pushed the getDOMNode-becomes-findDOMNode branch from 8bbc60c to b46a6ceComparesebmarkbage commented Dec 22, 2014
👍 |
Added findDOMNode, as we move toward deprecating getDOMNode
There was a problem hiding this comment.
There is no %s so these won't be inserted anywhere.
jimfb commented Jan 2, 2015
Updated diff: #2800 |
Summary: Currently Forget bails on mutations to globals within any callback function. However, callbacks passed to useEffect should not bail and are not subject to the rules of react in the same way. We allow this by instead of immediately raising errors when we see illegal writes, storing the error as part of the function. When the function is called, or passed to a position that could call it during rendering, we bail as before; but if it's passed to `useEffect`, we don't raise the errors.
4 participants
Added findDOMNode, as we move toward deprecating getDOMNode