Uh oh!
There was an error while loading. Please reload this page.
feat: add nodeRef alternative instead of internal findDOMNode - #559
Conversation
ref instead of findDOMNoderef instead of findDOMNode
taion
left a comment
There was a problem hiding this comment.
isn't this going to break stuff when rendering functional children that can't accept refs?
@taion I'll add some tests with different types of components, and see whats happening.
They mean that cannot use it inside a function component. |
einarq
commented
Nov 12, 2019
Any ideas on when (and if) this will be merged? |
einarq
commented
Nov 12, 2019
The way I understand this PR, I would say it is going to break things. Whatever you want to transition would have to either accept a ref (a forwardRef component), or be a plain dom element. Or am I missing something? It's happened before, many times... :) |
@einarq you are absolutely right. Whatever you want to transition would have to resolve to a DOM ref. So this would be a breaking change. @taion what are your thoughts on creating a new major version, using |
+1 for a new major version |
Or I'll try to find a solution that will |
Hi, there :) I see that these changes lead to the fact that I need to wrap my component to What about provide this: If user provide I can create PR. |
@inomdzhon I also thought of this, user should pas to functionApp(){constref=React.useRef()return(<TransitiondomRef={ref}><MaybeDOMmaybeFunctionMaybeClassrefThatComponentAccepts={ref}/></Transition>)}Using the method above, we can make it non breaking change, and use I am also thinking of writing a experimental hook. Maybe something like this functionApp(){constref=React.useRef()useTransition(ref)return(<MaybeDOMmaybeFunctionMaybeClassrefThatComponentAccepts={ref}/>)} |
inomdzhon
commented
Nov 21, 2019
Hm, but what about list of component? It can be hard to collect all refs and provide to May be we can use render props (like |
@inomdzhon in this case we can have this maybe? functionApp(){return(<TransitionGroup><CSSTransitiondomRef="hostRef"><div>1</div></CSSTransition><CSSTransitiondomRef="hostRef"><div>2</div></CSSTransition><CSSTransitiondomRef="hostRef"><div>3</div></CSSTransition></TransitionGroup>);}or even better this functionApp(){return(<TransitionGroupdomRef="hostRef"><CSSTransition><div>1</div></CSSTransition><CSSTransition><div>2</div></CSSTransition><CSSTransition><div>3</div></CSSTransition></TransitionGroup>);}And |
inomdzhon
commented
Nov 21, 2019
I think For don't repeat users can customize Than |
This are all use cases I can see for the API right now. importReactfrom'react'import{Transition,TransitionGroup}from'react-transition-group'functionMaybeDomOrFunctionOrClass(props){const{ refThatResolvesToDOM }=propsreturn<divref={refThatResolvesToDOM}>666</div>}functionApp(){constref=React.useRef(null)constref1=React.useRef(null)constref2=React.useRef(null)constref3=React.useRef(null)return(<React.Fragment>
/* it will use ref to make transition */
<TransitiondomRef={ref}><MaybeDomOrFunctionOrClassrefThatResolvesToDOM={ref}/></Transition>
/* it will try to find and use prop `refThatResolvesToDOM` */
/* otherwise fallback to `findDOMNode` */
<TransitiondomRef="refThatResolvesToDOM"><MaybeDomOrFunctionOrClassrefThatResolvesToDOM={ref}/></Transition>
/* it will try to find and use prop `refThatResolvesToDOM` */
/* otherwise fallback to `findDOMNode` */
/* ??? maybe create a ref internally in Transition and pass as `refThatResolvesToDOM` to child */
<TransitiondomRef="refThatResolvesToDOM"><MaybeDomOrFunctionOrClass/></Transition><TransitionGroup><TransitiondomRef={ref1}><MaybeDomOrFunctionOrClassrefThatResolvesToDOM={ref1}/></Transition><TransitiondomRef={ref2}><MaybeDomOrFunctionOrClassrefThatResolvesToDOM={ref2}/></Transition><TransitiondomRef={ref3}><MaybeDomOrFunctionOrClassrefThatResolvesToDOM={ref3}/></Transition></TransitionGroup><TransitionGroup><TransitiondomRef="refThatResolvesToDOM"><MaybeDomOrFunctionOrClassrefThatResolvesToDOM={ref1}/></Transition><TransitiondomRef="refThatResolvesToDOM"><MaybeDomOrFunctionOrClassrefThatResolvesToDOM={ref2}/></Transition><TransitiondomRef="refThatResolvesToDOM"><MaybeDomOrFunctionOrClassrefThatResolvesToDOM={ref3}/></Transition></TransitionGroup><TransitionGroup><TransitiondomRef="refThatResolvesToDOM"><MaybeDomOrFunctionOrClass/></Transition><TransitiondomRef="refThatResolvesToDOM"><MaybeDomOrFunctionOrClass/></Transition><TransitiondomRef="refThatResolvesToDOM"><MaybeDomOrFunctionOrClass/></Transition></TransitionGroup><TransitionGroupdomRef="refThatResolvesToDOM"><Transition><MaybeDomOrFunctionOrClass/></Transition><Transition><MaybeDomOrFunctionOrClass/></Transition><Transition><MaybeDomOrFunctionOrClass/></Transition></TransitionGroup></React.Fragment>)} |
Ok, I see. What about use The fewer use cases, the easier it is to use and maintain code. Over the years, I realized that the principle the easier the better. I mean we shouldn't complicate. |
jRichardeau
commented
Jan 24, 2020
Hey, |
camflan
commented
Feb 25, 2020
Any updates on release schedule? |
inomdzhon
commented
Feb 25, 2020
Hi, @iamandrewluca 👋 How I can help you? |
ref instead of findDOMNoderef instead of findDOMNodeStill thinking how to provide the API for the end user. The limitation of creating the ref internally is that we don't know what kind of children the |
unrevised6419
commented
Feb 27, 2020
If taking the example above from #559 (comment) And changing functionMaybeDomOrFunctionOrClass(props){const{ refThatResolvesToDOM }=propsreturn<divref={refThatResolvesToDOM}>666</div>}to functionMaybeDomOrFunctionOrClass(){constref=React.useRef(null)return(<TransitiondomRef={ref}><divref={ref}>666</div></Transition>)}then our functionApp(){return(<React.Fragment><MaybeDomOrFunctionOrClass/><TransitionGroup><MaybeDomOrFunctionOrClass/><MaybeDomOrFunctionOrClass/><MaybeDomOrFunctionOrClass/></TransitionGroup></React.Fragment>)} |
unrevised6419
commented
Feb 27, 2020
If functionMaybeDomOrFunctionOrClass({ innerRef =React.useRef(null)}){return(<TransitiondomRef={innerRef}><divref={innerRef}>666</div></Transition>)}functionApp(){constref=React.useRef(null)constref1=React.useRef(null)constref2=React.useRef(null)constref3=React.useRef(null)return(<React.Fragment><MaybeDomOrFunctionOrClassinnerRef={ref}/><TransitionGroup><MaybeDomOrFunctionOrClassinnerRef={ref1}/><MaybeDomOrFunctionOrClassinnerRef={ref2}/><MaybeDomOrFunctionOrClassinnerRef={ref3}/></TransitionGroup></React.Fragment>)} |
ref instead of findDOMNodedomRef alternative instead of findDOMNodeunrevised6419
commented
Feb 27, 2020
I'll do some tests using this PR and reactstrap components. |
inomdzhon
commented
Feb 27, 2020
Yeah, I understood 👌Waiting tests. Furthermore, I realized on this point |
domRef alternative instead of findDOMNodedomRef alternative instead of findDOMNode# [4.4.0](v4.3.0...v4.4.0) (2020-05-05) ### Features * add `nodeRef` alternative instead of internal `findDOMNode` ([#559](#559)) ([85016bf](85016bf))
jquense
commented
May 5, 2020
🎉 This PR is included in version 4.4.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
I didn't realize we have semantic-release set up 😅 I'll update release notes and the documentation. |
fkhadra
commented
May 5, 2020
Guys, thank you so much for all the work you did for addressing this issue. Also, migrating my library to the new api was a breeze. 🙏 |
silvenon
commented
May 5, 2020
I'm glad that you're already enjoying the update! ❤️ Let us know if there are any issues. |
I feel like the typing of nodeRef should be extended. Right now it only accommodates for the type 'Element' which is limiting in case of nested routes. Please, let me know if I misunderstood #559. Here's my situation: Level 1 router is a <Switch> handling switching between high-level website sections which, themselves are routers handling level 2 navigation (<Switch>'es). Right now, I'm drilling nodeRef down to div's through nested <Switch>'es and it: a) feels hacky, b) interferes with level 2 CSSTransitions. Am I missing something? Is the only way around this to wrap all of my <Switch>'es in <div ref={nodeRef}>'s? |
unrevised6419
commented
May 20, 2020
@antikvarBE can you provide a simple example? I kind of understand what are you trying to do, and I think I know what should you do, but I don't see entire picture. |
Thanks for reaching back @iamandrewluca . In Private.tsx constnodeRef=React.useRef(null);return(<divid={'private'}className={styles.wrapper}><MainMenuitems={items}/><mainclassName={styles.content}><SwitchTransitionmode={'out-in'}><CSSTransitionkey={location.pathname.split('/',3).join('/')}timeout={500}classNames={{ ...styles}}nodeRef={nodeRef}><PrivateRouterlocation={location}ref={nodeRef}/></CSSTransition></SwitchTransition></main></div>);In PrivateRouter.routes.tsx constPrivateRouter: React.FC<SwitchProps&{nodeRef: React.MutableRefObject<null>}>=({ location })=>(<Switchlocation={location}ref={nodeRef}><Routepath={'/favorites'}><Favorites/></Route><Routepath={'/support'}><Support/></Route><Routepath={'/account'}><Account/></Route></Switch>);Doing this would throw the aforementioned error: So, currently, I'm doing this instead: constnodeRef=React.useRef(null);return(<divid={'private'}className={styles.wrapper}><MainMenuitems={items}/><mainclassName={styles.content}><SwitchTransitionmode={'out-in'}><CSSTransitionkey={location.pathname.split('/',3).join('/')}timeout={500}classNames={{ ...styles}}nodeRef={nodeRef}><divref={nodeRef}>// <-- This is my current workaround<PrivateRouterlocation={location}/></div></CSSTransition></SwitchTransition></main></div>);It works perfectly fine but I'm the kind of person who thinks twice before introducing another DOM level and if I can get away without doing it and keep things nice and clean semantically, I'd rather do that. What do you think? |
silvenon
commented
May 20, 2020
Is |
malikalimoekhamedov
commented
May 20, 2020
@silvenon , it's from react-router-dom. (JSXattribute)React.ClassAttributes<Switch>.ref?: string |((instance: Switch|null)=>void)|React.RefObject<Switch>|null|undefined |
Oh, then |
@antikvarBE in example above if I'm not mistaken you want to transition between private routes? What version of react router do you use? |
@iamandrewluca , I'm using "react-router": "^5.2.0" and "react-router-dom": "^5.2.0". Yes, I'm transitioning between private routes as they're described in Private.routes.tsx (see example). Essencially, <Switch> displays one and only one <Route> at a time. If one route only contains one component, it also means <Switch> displays one and only one component at a time. That's the way I understand it. |
@silvenon : Got it. That makes sense. Just wanted to double-check. Thanks a bunch. |
# [4.4.0](reactjs/react-transition-group@v4.3.0...v4.4.0) (2020-05-05) ### Features * add `nodeRef` alternative instead of internal `findDOMNode` ([#559](reactjs/react-transition-group#559)) ([85016bf](reactjs/react-transition-group@85016bf))
# [4.4.0](reactjs/react-transition-group@v4.3.0...v4.4.0) (2020-05-05) ### Features * add `nodeRef` alternative instead of internal `findDOMNode` ([#559](reactjs/react-transition-group#559)) ([85016bf](reactjs/react-transition-group@85016bf))
# [4.4.0](reactjs/react-transition-group@v4.3.0...v4.4.0) (2020-05-05) ### Features * add `nodeRef` alternative instead of internal `findDOMNode` ([#559](reactjs/react-transition-group#559)) ([85016bf](reactjs/react-transition-group@85016bf))
# [4.4.0](reactjs/react-transition-group@v4.3.0...v4.4.0) (2020-05-05) ### Features * add `nodeRef` alternative instead of internal `findDOMNode` ([#559](reactjs/react-transition-group#559)) ([85016bf](reactjs/react-transition-group@85016bf))
RFC: An alternative to ReactDOM.findDOMNode
nodeRefalternative for internalfindDOMNodeWe can name prop one of this:
domRefnodeRef✓guestReftransitionRefref(withforwardRefbut BREAKING CHANGE)PRs:
Closes#457
Closes#486
Closes#514
Issues:
Closes#287
Closes#429