From 1f53b093ec77bd7183fae06b3be2c56eea88bcb0 Mon Sep 17 00:00:00 2001 From: Andrew Luca Date: Tue, 8 Oct 2019 09:32:06 +0300 Subject: [PATCH 1/7] feat: add `nodeRef` alternative instead of internal `findDOMNode` --- src/CSSTransition.js | 41 ++++-- src/ReplaceTransition.js | 17 ++- src/Transition.js | 62 +++++--- src/TransitionGroup.js | 1 + test/CSSTransition-test.js | 250 +++++++++++++++++--------------- test/CSSTransitionGroup-test.js | 21 ++- test/SwitchTransition-test.js | 26 ++-- test/Transition-test.js | 117 ++++++++------- test/TransitionGroup-test.js | 11 +- test/setup.js | 6 +- 10 files changed, 332 insertions(+), 220 deletions(-) diff --git a/src/CSSTransition.js b/src/CSSTransition.js index 58de70ad..cd61d6a2 100644 --- a/src/CSSTransition.js +++ b/src/CSSTransition.js @@ -90,61 +90,72 @@ class CSSTransition extends React.Component { exit: {}, } - onEnter = (node, appearing) => { + onEnter = (maybeNode, maybeAppearing) => { + const [node, appearing] = this.resolveArguments(maybeNode, maybeAppearing) this.removeClasses(node, 'exit'); this.addClass(node, appearing ? 'appear' : 'enter', 'base'); if (this.props.onEnter) { - this.props.onEnter(node, appearing) + this.props.onEnter(maybeNode, maybeAppearing) } } - onEntering = (node, appearing) => { + onEntering = (maybeNode, maybeAppearing) => { + const [node, appearing] = this.resolveArguments(maybeNode, maybeAppearing) const type = appearing ? 'appear' : 'enter'; this.addClass(node, type, 'active') if (this.props.onEntering) { - this.props.onEntering(node, appearing) + this.props.onEntering(maybeNode, maybeAppearing) } } - onEntered = (node, appearing) => { + onEntered = (maybeNode, maybeAppearing) => { + const [node, appearing] = this.resolveArguments(maybeNode, maybeAppearing) const type = appearing ? 'appear' : 'enter' this.removeClasses(node, type); this.addClass(node, type, 'done'); if (this.props.onEntered) { - this.props.onEntered(node, appearing) + this.props.onEntered(maybeNode, maybeAppearing) } } - onExit = (node) => { + onExit = (maybeNode) => { + const [node] = this.resolveArguments(maybeNode) this.removeClasses(node, 'appear'); this.removeClasses(node, 'enter'); this.addClass(node, 'exit', 'base') if (this.props.onExit) { - this.props.onExit(node) + this.props.onExit(maybeNode) } } - onExiting = (node) => { + onExiting = (maybeNode) => { + const [node] = this.resolveArguments(maybeNode) this.addClass(node, 'exit', 'active') if (this.props.onExiting) { - this.props.onExiting(node) + this.props.onExiting(maybeNode) } } - onExited = (node) => { + onExited = (maybeNode) => { + const [node] = this.resolveArguments(maybeNode) this.removeClasses(node, 'exit'); this.addClass(node, 'exit', 'done'); if (this.props.onExited) { - this.props.onExited(node) + this.props.onExited(maybeNode) } } + // when prop `nodeRef` is provided `node` is excluded + resolveArguments = (maybeNode, maybeAppearing) => this.props.nodeRef + ? [this.props.nodeRef.current, maybeNode] // here `maybeNode` is actually `appearing` + : [maybeNode, maybeAppearing] // `findDOMNode` was used + getClassNames = (type) => { const { classNames } = this.props; const isStringClassNames = typeof classNames === 'string'; @@ -305,6 +316,7 @@ CSSTransition.propTypes = { /** * A `` callback fired immediately after the 'enter' or 'appear' class is * applied. + * Note: when `nodeRef` prop is passed, `node` is not passed * * @type Function(node: HtmlElement, isAppearing: bool) */ @@ -313,6 +325,7 @@ CSSTransition.propTypes = { /** * A `` callback fired immediately after the 'enter-active' or * 'appear-active' class is applied. + * Note: when `nodeRef` prop is passed, `node` is not passed * * @type Function(node: HtmlElement, isAppearing: bool) */ @@ -321,6 +334,7 @@ CSSTransition.propTypes = { /** * A `` callback fired immediately after the 'enter' or * 'appear' classes are **removed** and the `done` class is added to the DOM node. + * Note: when `nodeRef` prop is passed, `node` is not passed * * @type Function(node: HtmlElement, isAppearing: bool) */ @@ -329,6 +343,7 @@ CSSTransition.propTypes = { /** * A `` callback fired immediately after the 'exit' class is * applied. + * Note: when `nodeRef` prop is passed, `node` is not passed * * @type Function(node: HtmlElement) */ @@ -336,6 +351,7 @@ CSSTransition.propTypes = { /** * A `` callback fired immediately after the 'exit-active' is applied. + * Note: when `nodeRef` prop is passed, `node` is not passed * * @type Function(node: HtmlElement) */ @@ -344,6 +360,7 @@ CSSTransition.propTypes = { /** * A `` callback fired immediately after the 'exit' classes * are **removed** and the `exit-done` class is added to the DOM node. + * Note: when `nodeRef` prop is passed, `node` is not passed * * @type Function(node: HtmlElement) */ diff --git a/src/ReplaceTransition.js b/src/ReplaceTransition.js index 1eea0259..4a3fb7c4 100644 --- a/src/ReplaceTransition.js +++ b/src/ReplaceTransition.js @@ -28,7 +28,13 @@ class ReplaceTransition extends React.Component { const child = React.Children.toArray(children)[idx]; if (child.props[handler]) child.props[handler](...originalArgs) - if (this.props[handler]) this.props[handler](ReactDOM.findDOMNode(this)) + if (this.props[handler]) { + const nodeRef = idx === 0 + ? this.props.firstNodeRef + : this.props.secondNodeRef + + this.props[handler](nodeRef ? undefined : ReactDOM.findDOMNode(this)) + } } render() { @@ -45,6 +51,8 @@ class ReplaceTransition extends React.Component { delete props.onExit; delete props.onExiting; delete props.onExited; + delete props.firstNodeRef; + delete props.secondNodeRef; return ( @@ -76,6 +84,13 @@ ReplaceTransition.propTypes = { return null; }, + + /** + * A react reference to DOM element that need to transition + * https://stackoverflow.com/a/51127130/4671932 + */ + firstNodeRef: PropTypes.shape({ current: PropTypes.instanceOf(Element) }), + secondNodeRef: PropTypes.shape({ current: PropTypes.instanceOf(Element) }) }; export default ReplaceTransition; diff --git a/src/Transition.js b/src/Transition.js index d3c7a87e..bb8fd7fb 100644 --- a/src/Transition.js +++ b/src/Transition.js @@ -169,7 +169,7 @@ class Transition extends React.Component { this.updateStatus(true, this.appearStatus) } - componentDidUpdate(prevProps) { + componentDidUpdate(prevProps, _prevState, _snapshot) { let nextStatus = null if (prevProps !== this.props) { const { status } = this.state @@ -210,21 +210,23 @@ class Transition extends React.Component { if (nextStatus !== null) { // nextStatus will always be ENTERING or EXITING. this.cancelNextCallback() - const node = ReactDOM.findDOMNode(this) if (nextStatus === ENTERING) { - this.performEnter(node, mounting) + this.performEnter(mounting) } else { - this.performExit(node) + this.performExit() } } else if (this.props.unmountOnExit && this.state.status === EXITED) { this.setState({ status: UNMOUNTED }) } } - performEnter(node, mounting) { + performEnter(mounting) { const { enter } = this.props const appearing = this.context ? this.context.isMounting : mounting + const [maybeNode, maybeAppearing] = this.props.nodeRef + ? [appearing] + : [ReactDOM.findDOMNode(this), appearing] const timeouts = this.getTimeouts() const enterTimeout = appearing ? timeouts.appear : timeouts.enter @@ -232,43 +234,47 @@ class Transition extends React.Component { // if we are mounting and running this it means appear _must_ be set if ((!mounting && !enter) || config.disabled) { this.safeSetState({ status: ENTERED }, () => { - this.props.onEntered(node) + this.props.onEntered(maybeNode) }) return } - this.props.onEnter(node, appearing) + this.props.onEnter(maybeNode, maybeAppearing) this.safeSetState({ status: ENTERING }, () => { - this.props.onEntering(node, appearing) + this.props.onEntering(maybeNode, maybeAppearing) - this.onTransitionEnd(node, enterTimeout, () => { + this.onTransitionEnd(enterTimeout, () => { this.safeSetState({ status: ENTERED }, () => { - this.props.onEntered(node, appearing) + this.props.onEntered(maybeNode, maybeAppearing) }) }) }) } - performExit(node) { + performExit() { const { exit } = this.props const timeouts = this.getTimeouts() + const maybeNode = this.props.nodeRef + ? undefined + : ReactDOM.findDOMNode(this) // no exit animation skip right to EXITED if (!exit || config.disabled) { this.safeSetState({ status: EXITED }, () => { - this.props.onExited(node) + this.props.onExited(maybeNode) }) return } - this.props.onExit(node) + + this.props.onExit(maybeNode) this.safeSetState({ status: EXITING }, () => { - this.props.onExiting(node) + this.props.onExiting(maybeNode) - this.onTransitionEnd(node, timeouts.exit, () => { + this.onTransitionEnd(timeouts.exit, () => { this.safeSetState({ status: EXITED }, () => { - this.props.onExited(node) + this.props.onExited(maybeNode) }) }) }) @@ -308,8 +314,11 @@ class Transition extends React.Component { return this.nextCallback } - onTransitionEnd(node, timeout, handler) { + onTransitionEnd(timeout, handler) { this.setNextCallback(handler) + const node = this.props.nodeRef + ? this.props.nodeRef.current + : ReactDOM.findDOMNode(this) const doesNotHaveTimeoutOrListener = timeout == null && !this.props.addEndListener @@ -319,7 +328,10 @@ class Transition extends React.Component { } if (this.props.addEndListener) { - this.props.addEndListener(node, this.nextCallback) + const [maybeNode, maybeNextCallback] = this.props.nodeRef + ? [this.nextCallback] + : [node, this.nextCallback] + this.props.addEndListener(maybeNode, maybeNextCallback) } if (timeout != null) { @@ -349,6 +361,7 @@ class Transition extends React.Component { delete childProps.onExit delete childProps.onExiting delete childProps.onExited + delete childProps.nodeRef if (typeof children === 'function') { // allows for nested Transitions @@ -467,6 +480,7 @@ Transition.propTypes = { * Add a custom transition end trigger. Called with the transitioning * DOM node and a `done` callback. Allows for more fine grained transition end * logic. **Note:** Timeouts are still used as a fallback if provided. + * Note: when `nodeRef` prop is passed, `node` is not passed * * ```jsx * addEndListener={(node, done) => { @@ -480,6 +494,7 @@ Transition.propTypes = { /** * Callback fired before the "entering" status is applied. An extra parameter * `isAppearing` is supplied to indicate if the enter stage is occurring on the initial mount + * Note: when `nodeRef` prop is passed, `node` is not passed * * @type Function(node: HtmlElement, isAppearing: bool) -> void */ @@ -488,6 +503,7 @@ Transition.propTypes = { /** * Callback fired after the "entering" status is applied. An extra parameter * `isAppearing` is supplied to indicate if the enter stage is occurring on the initial mount + * Note: when `nodeRef` prop is passed, `node` is not passed * * @type Function(node: HtmlElement, isAppearing: bool) */ @@ -496,6 +512,7 @@ Transition.propTypes = { /** * Callback fired after the "entered" status is applied. An extra parameter * `isAppearing` is supplied to indicate if the enter stage is occurring on the initial mount + * Note: when `nodeRef` prop is passed, `node` is not passed * * @type Function(node: HtmlElement, isAppearing: bool) -> void */ @@ -503,6 +520,7 @@ Transition.propTypes = { /** * Callback fired before the "exiting" status is applied. + * Note: when `nodeRef` prop is passed, `node` is not passed * * @type Function(node: HtmlElement) -> void */ @@ -510,6 +528,7 @@ Transition.propTypes = { /** * Callback fired after the "exiting" status is applied. + * Note: when `nodeRef` prop is passed, `node` is not passed * * @type Function(node: HtmlElement) -> void */ @@ -517,10 +536,17 @@ Transition.propTypes = { /** * Callback fired after the "exited" status is applied. + * Note: when `nodeRef` prop is passed, `node` is not passed * * @type Function(node: HtmlElement) -> void */ onExited: PropTypes.func, + + /** + * A react reference to DOM element that need to transition + * https://stackoverflow.com/a/51127130/4671932 + */ + nodeRef: PropTypes.shape({ current: PropTypes.instanceOf(Element) }) } // Name the function so it is clearer in the documentation diff --git a/src/TransitionGroup.js b/src/TransitionGroup.js index eec05197..bda2f82b 100644 --- a/src/TransitionGroup.js +++ b/src/TransitionGroup.js @@ -66,6 +66,7 @@ class TransitionGroup extends React.Component { } } + // node is `undefined` when user provided `nodeRef` prop handleExited(child, node) { let currentChildMapping = getChildMapping(this.props.children) diff --git a/test/CSSTransition-test.js b/test/CSSTransition-test.js index 3e5f9761..9d8aa1af 100644 --- a/test/CSSTransition-test.js +++ b/test/CSSTransition-test.js @@ -7,40 +7,43 @@ import TransitionGroup from '../src/TransitionGroup'; describe('CSSTransition', () => { it('should flush new props to the DOM before initiating a transition', (done) => { - mount( + const nodeRef = React.createRef() + const wrapper = mount( { - expect(node.classList.contains('test-class')).toEqual(true) - expect(node.classList.contains('test-entering')).toEqual(false) + onEnter={() => { + expect(nodeRef.current.classList.contains('test-class')).toEqual(true) + expect(nodeRef.current.classList.contains('test-entering')).toEqual(false) done() }} > -
+
) - .tap(inst => { - expect(inst.getDOMNode().classList.contains('test-class')).toEqual(false) - }) - .setProps({ + expect(nodeRef.current.classList.contains('test-class')).toEqual(false) + + wrapper.setProps({ in: true, className: 'test-class' }) }); describe('entering', () => { - let instance; + let wrapper, nodeRef; beforeEach(() => { - instance = mount( + nodeRef = React.createRef() + wrapper = mount( -
+
) }); @@ -48,21 +51,21 @@ describe('CSSTransition', () => { it('should apply classes at each transition state', done => { let count = 0; - instance.setProps({ + wrapper.setProps({ in: true, - onEnter(node) { + onEnter() { count++; - expect(node.className).toEqual('test-enter'); + expect(nodeRef.current.className).toEqual('test-enter'); }, - onEntering(node){ + onEntering() { count++; - expect(node.className).toEqual('test-enter test-enter-active'); + expect(nodeRef.current.className).toEqual('test-enter test-enter-active'); }, - onEntered(node){ - expect(node.className).toEqual('test-enter-done'); + onEntered() { + expect(nodeRef.current.className).toEqual('test-enter-done'); expect(count).toEqual(2); done(); } @@ -71,34 +74,36 @@ describe('CSSTransition', () => { it('should apply custom classNames names', done => { let count = 0; - instance = mount( + const nodeRef = React.createRef() + wrapper = mount( -
+
); - instance.setProps({ + wrapper.setProps({ in: true, - onEnter(node){ + onEnter(){ count++; - expect(node.className).toEqual('custom'); + expect(nodeRef.current.className).toEqual('custom'); }, - onEntering(node){ + onEntering(){ count++; - expect(node.className).toEqual('custom custom-super-active'); + expect(nodeRef.current.className).toEqual('custom custom-super-active'); }, - onEntered(node){ - expect(node.className).toEqual('custom-super-done'); + onEntered(){ + expect(nodeRef.current.className).toEqual('custom-super-done'); expect(count).toEqual(2); done(); } @@ -109,39 +114,43 @@ describe('CSSTransition', () => { describe('appearing', () => { it('should apply appear classes at each transition state', done => { let count = 0; + const nodeRef = React.createRef() mount( { + onEnter={(isAppearing) => { count++; expect(isAppearing).toEqual(true); - expect(node.className).toEqual('appear-test-appear'); + expect(nodeRef.current.className).toEqual('appear-test-appear'); }} - onEntering={(node, isAppearing) => { + onEntering={(isAppearing) => { count++; expect(isAppearing).toEqual(true); - expect(node.className).toEqual('appear-test-appear appear-test-appear-active'); + expect(nodeRef.current.className).toEqual('appear-test-appear appear-test-appear-active'); }} - onEntered={(node, isAppearing) => { + onEntered={(isAppearing) => { expect(isAppearing).toEqual(true); - expect(node.className).toEqual('appear-test-appear-done appear-test-enter-done'); + expect(nodeRef.current.className).toEqual('appear-test-appear-done appear-test-enter-done'); expect(count).toEqual(2); done(); }} > -
+
); }); it('should lose the "*-appear-done" class after leaving and entering again', (done) => { + const nodeRef = React.createRef() const wrapper = mount( { wrapper.setProps({ in: false, onEntered: () => {}, - onExited: (node) => { - expect(node.className).toBe('appear-test-exit-done') + onExited: () => { + expect(nodeRef.current.className).toBe('appear-test-exit-done') wrapper.setProps({ in: true, onEntered: () => { - expect(node.className).toBe('appear-test-enter-done') + expect(nodeRef.current.className).toBe('appear-test-enter-done') done() } }) @@ -162,7 +171,7 @@ describe('CSSTransition', () => { }) }} > -
+
) }); @@ -191,32 +200,34 @@ describe('CSSTransition', () => { it('should not be appearing in normal enter mode', done => { let count = 0; + const nodeRef = React.createRef() mount( -
+
).setProps({ in: true, - onEnter(node, isAppearing){ + onEnter(isAppearing){ count++; expect(isAppearing).toEqual(false); - expect(node.className).toEqual('not-appear-test-enter'); + expect(nodeRef.current.className).toEqual('not-appear-test-enter'); }, - onEntering(node, isAppearing){ + onEntering(isAppearing){ count++; expect(isAppearing).toEqual(false); - expect(node.className).toEqual('not-appear-test-enter not-appear-test-enter-active'); + expect(nodeRef.current.className).toEqual('not-appear-test-enter not-appear-test-enter-active'); }, - onEntered(node, isAppearing){ + onEntered(isAppearing){ expect(isAppearing).toEqual(false); - expect(node.className).toEqual('not-appear-test-enter-done'); + expect(nodeRef.current.className).toEqual('not-appear-test-enter-done'); expect(count).toEqual(2); done(); } @@ -224,9 +235,11 @@ describe('CSSTransition', () => { }); it('should not enter the transition states when appear=false', () => { + const nodeRef = React.createRef() mount( { throw Error('Entred called!') }} > -
+
); }); @@ -249,16 +262,18 @@ describe('CSSTransition', () => { }); describe('exiting', ()=> { - let instance; + let wrapper, nodeRef; beforeEach(() => { - instance = mount( + nodeRef = React.createRef() + wrapper = mount( -
+
) }); @@ -266,21 +281,21 @@ describe('CSSTransition', () => { it('should apply classes at each transition state', done => { let count = 0; - instance.setProps({ + wrapper.setProps({ in: false, - onExit(node){ + onExit(){ count++; - expect(node.className).toEqual('test-exit'); + expect(nodeRef.current.className).toEqual('test-exit'); }, - onExiting(node){ + onExiting(){ count++; - expect(node.className).toEqual('test-exit test-exit-active'); + expect(nodeRef.current.className).toEqual('test-exit test-exit-active'); }, - onExited(node){ - expect(node.className).toEqual('test-exit-done'); + onExited(){ + expect(nodeRef.current.className).toEqual('test-exit-done'); expect(count).toEqual(2); done(); } @@ -289,9 +304,11 @@ describe('CSSTransition', () => { it('should apply custom classNames names', done => { let count = 0; - instance = mount( + const nodeRef = React.createRef() + wrapper = mount( { exitDone: 'custom-super-done', }} > -
+
); - instance.setProps({ + wrapper.setProps({ in: false, - onExit(node){ + onExit() { count++; - expect(node.className).toEqual('custom'); + expect(nodeRef.current.className).toEqual('custom'); }, - onExiting(node){ + onExiting() { count++; - expect(node.className).toEqual('custom custom-super-active'); + expect(nodeRef.current.className).toEqual('custom custom-super-active'); }, - onExited(node){ - expect(node.className).toEqual('custom-super-done'); + onExited() { + expect(nodeRef.current.className).toEqual('custom-super-done'); expect(count).toEqual(2); done(); } @@ -327,30 +344,32 @@ describe('CSSTransition', () => { it('should support empty prefix', done => { let count = 0; - const instance = mount( + const nodeRef = React.createRef() + const wrapper = mount( -
+
) - instance.setProps({ + wrapper.setProps({ in: false, - onExit(node){ + onExit() { count++; - expect(node.className).toEqual('exit'); + expect(nodeRef.current.className).toEqual('exit'); }, - onExiting(node){ + onExiting() { count++; - expect(node.className).toEqual('exit exit-active'); + expect(nodeRef.current.className).toEqual('exit exit-active'); }, - onExited(node){ - expect(node.className).toEqual('exit-done'); + onExited() { + expect(nodeRef.current.className).toEqual('exit-done'); expect(count).toEqual(2); done(); } @@ -359,11 +378,11 @@ describe('CSSTransition', () => { }); describe('reentering', () => { - it('should remove dynamically applied classes', done => { + it('should remove dynamically applied classes', async () => { let count = 0; class Test extends React.Component { render() { - const { direction, text, ...props } = this.props; + const { direction, text, nodeRef, ...props } = this.props; return ( { - {text} + {text} ) } } - const instance = mount() + let nodeRef = React.createRef() + const wrapper = mount() const rerender = getProps => new Promise(resolve => - instance.setProps({ + wrapper.setProps({ onEnter: undefined, onEntering: undefined, onEntered: undefined, @@ -400,40 +421,43 @@ describe('CSSTransition', () => { }) ); - Promise.resolve().then(() => - rerender(resolve => ({ - direction: 'up', - text: 'bar', + nodeRef = React.createRef() - onEnter(node) { - count++; - expect(node.className).toEqual('up-enter'); - }, - onEntering(node) { - count++; - expect(node.className).toEqual('up-enter up-enter-active'); - resolve() - } - })) - ).then(() => { - return rerender(resolve => ({ - direction: 'down', - text: 'foo', - - onEntering(node) { - count++; - expect(node.className).toEqual('down-enter down-enter-active'); - }, - onEntered(node) { - count++; - expect(node.className).toEqual('down-enter-done'); - resolve(); - } - })) - }).then(() => { - expect(count).toEqual(4); - done(); - }); + await rerender(resolve => ({ + direction: 'up', + text: 'bar', + nodeRef, + + onEnter() { + count++; + expect(nodeRef.current.className).toEqual('up-enter'); + }, + onEntering() { + count++; + expect(nodeRef.current.className).toEqual('up-enter up-enter-active'); + resolve() + } + })) + + nodeRef = React.createRef() + + await rerender(resolve => ({ + direction: 'down', + text: 'foo', + nodeRef, + + onEntering() { + count++; + expect(nodeRef.current.className).toEqual('down-enter down-enter-active'); + }, + onEntered() { + count++; + expect(nodeRef.current.className).toEqual('down-enter-done'); + resolve(); + } + })) + + expect(count).toEqual(4); }); }); }); diff --git a/test/CSSTransitionGroup-test.js b/test/CSSTransitionGroup-test.js index 4d0decbf..dae08b59 100644 --- a/test/CSSTransitionGroup-test.js +++ b/test/CSSTransitionGroup-test.js @@ -10,14 +10,7 @@ let TransitionGroup; describe('CSSTransitionGroup', () => { let container; let consoleErrorSpy; - - function YoloTransition({ id, ...props }) { - return ( - - - - ) - } + let YoloTransition; beforeEach(() => { jest.resetModuleRegistry(); @@ -28,6 +21,18 @@ describe('CSSTransitionGroup', () => { TransitionGroup = require('../src/TransitionGroup'); + YoloTransition = class extends React.Component { + nodeRef = React.createRef() + render() { + let { id, ...props } = this.props + return ( + + + + ) + } + } + container = document.createElement('div'); consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); }); diff --git a/test/SwitchTransition-test.js b/test/SwitchTransition-test.js index 744e2967..76d2a527 100644 --- a/test/SwitchTransition-test.js +++ b/test/SwitchTransition-test.js @@ -10,20 +10,21 @@ describe('SwitchTransition', () => { beforeEach(() => { log = []; let events = { - onEnter: (_, m) => log.push(m ? 'appear' : 'enter'), - onEntering: (_, m) => log.push(m ? 'appearing' : 'entering'), - onEntered: (_, m) => log.push(m ? 'appeared' : 'entered'), + onEnter: (m) => log.push(m ? 'appear' : 'enter'), + onEntering: (m) => log.push(m ? 'appearing' : 'entering'), + onEntered: (m) => log.push(m ? 'appeared' : 'entered'), onExit: () => log.push('exit'), onExiting: () => log.push('exiting'), onExited: () => log.push('exited') }; + const nodeRef = React.createRef() Parent = function Parent({ on, rendered = true }) { return ( {rendered ? ( - - + + ) : null} @@ -32,10 +33,11 @@ describe('SwitchTransition', () => { }); it('should have default status ENTERED', () => { + const nodeRef = React.createRef() const wrapper = mount( - - + + ); @@ -44,10 +46,11 @@ describe('SwitchTransition', () => { }); it('should have default mode: out-in', () => { + const nodeRef = React.createRef() const wrapper = mount( - - + + ); @@ -56,11 +59,12 @@ describe('SwitchTransition', () => { }); it('should work without childs', () => { + const nodeRef = React.createRef() expect(() => { mount( - - + + ); diff --git a/test/Transition-test.js b/test/Transition-test.js index bc31c926..a1ad8c6c 100644 --- a/test/Transition-test.js +++ b/test/Transition-test.js @@ -25,15 +25,17 @@ expect.extend({ describe('Transition', () => { it('should not transition on mount', () => { + const nodeRef = React.createRef() let wrapper = mount( { throw new Error('should not Enter') }} > -
+
) @@ -41,21 +43,23 @@ describe('Transition', () => { }) it('should transition on mount with `appear`', done => { + const nodeRef = React.createRef() mount( { throw Error('Animated!') }} > -
+
) mount( - done()}> -
+ done()}> +
) }) @@ -63,14 +67,16 @@ describe('Transition', () => { it('should pass filtered props to children', () => { class Child extends React.Component { render() { - return
child
+ return
child
} } + const nodeRef = React.createRef() const child = mount( { onExiting={() => {}} onExited={() => {}} > - + ).find(Child) - expect(child.props()).toEqual({ foo: 'foo', bar: 'bar' }) + expect(child.props()).toEqual({ foo: 'foo', bar: 'bar', nodeRef }) }) it('should allow addEndListener instead of timeouts', done => { - let listener = jest.fn((node, end) => setTimeout(end, 0)) + let listener = jest.fn(end => setTimeout(end, 0)) - let inst = mount( + const nodeRef = React.createRef() + let wrapper = mount( { expect(listener).toHaveBeenCalledTimes(1) done() }} > -
+
) - inst.setProps({ in: true }) + wrapper.setProps({ in: true }) }) it('should fallback to timeouts with addEndListener', done => { let calledEnd = false - let listener = (node, end) => + let listener = (end) => setTimeout(() => { calledEnd = true end() }, 100) - let inst = mount( + const nodeRef = React.createRef() + let wrapper = mount( { expect(calledEnd).toEqual(false) done() }} > -
+
) - inst.setProps({ in: true }) + wrapper.setProps({ in: true }) }) it('should mount/unmount immediately if not have enter/exit timeout', (done) => { + const nodeRef = React.createRef() const wrapper = mount( - -
+ +
) @@ -164,9 +175,10 @@ describe('Transition', () => { setTimeout(() => { calledBeforeEntered = true }, 10) + const nodeRef = React.createRef() const wrapper = mount( - -
+ +
) @@ -182,9 +194,10 @@ describe('Transition', () => { }) it('should use appear timeout if appear is set', done => { + const nodeRef = React.createRef() const wrapper = mount( - -
+ +
) @@ -206,12 +219,13 @@ describe('Transition', () => { }) describe('entering', () => { - let wrapper + let wrapper, nodeRef; beforeEach(() => { + nodeRef = React.createRef() wrapper = mount( - -
+ +
) }) @@ -267,12 +281,13 @@ describe('Transition', () => { }) describe('exiting', () => { - let wrapper + let wrapper, nodeRef; beforeEach(() => { + nodeRef = React.createRef() wrapper = mount( - -
+ +
) }) @@ -329,10 +344,8 @@ describe('Transition', () => { describe('mountOnEnter', () => { class MountTransition extends React.Component { - constructor(props) { - super(props) - this.state = { in: props.initialIn } - } + nodeRef = React.createRef() + state = { in: this.props.initialIn } render() { const { ...props } = this.props @@ -341,12 +354,13 @@ describe('Transition', () => { return ( this.transition = this.transition || transition} + nodeRef={this.nodeRef} mountOnEnter in={this.state.in} timeout={10} {...props} > -
+
) } @@ -362,7 +376,7 @@ describe('Transition', () => { initialIn={false} onEnter={() => { expect(wrapper.instance().getStatus()).toEqual(EXITED) - expect(wrapper.getDOMNode()).toExist() + expect(wrapper.instance().nodeRef.current).toExist() done() }} /> @@ -370,7 +384,7 @@ describe('Transition', () => { expect(wrapper.instance().getStatus()).toEqual(UNMOUNTED) - expect(wrapper.getDOMNode()).not.toExist() + expect(wrapper.instance().nodeRef.current).not.toExist() wrapper.setProps({ in: true }) }) @@ -381,27 +395,27 @@ describe('Transition', () => { initialIn={false} onEntered={() => { expect(wrapper.instance().getStatus()).toEqual(ENTERED) - expect(wrapper.getDOMNode()).toExist() + expect(wrapper.instance().nodeRef.current).toExist() wrapper.setState({ in: false }) }} onExited={() => { expect(wrapper.instance().getStatus()).toEqual(EXITED) - expect(wrapper.getDOMNode()).toExist() + expect(wrapper.instance().nodeRef.current).toExist() done() }} /> ) - expect(wrapper.getDOMNode()).not.toExist() + expect(wrapper.instance().nodeRef.current).not.toExist() wrapper.setState({ in: true }) }) }) describe('unmountOnExit', () => { class UnmountTransition extends React.Component { - divRef = React.createRef() + nodeRef = React.createRef() state = { in: this.props.initialIn } render() { @@ -411,12 +425,13 @@ describe('Transition', () => { return ( this.transition = this.transition || transition} + nodeRef={this.nodeRef} unmountOnExit in={this.state.in} timeout={10} {...props} > -
+
) } @@ -427,42 +442,42 @@ describe('Transition', () => { } it('should mount when entering', done => { - const wrapper = mount( + const instance = mount( { - expect(wrapper.getStatus()).toEqual(EXITED) - expect(wrapper.divRef.current).toExist() + expect(instance.getStatus()).toEqual(EXITED) + expect(instance.nodeRef.current).toExist() done() }} /> ).instance() - expect(wrapper.getStatus()).toEqual(UNMOUNTED) - expect(wrapper.divRef.current).toBeNull() + expect(instance.getStatus()).toEqual(UNMOUNTED) + expect(instance.nodeRef.current).toBeNull() - wrapper.setState({ in: true }) + instance.setState({ in: true }) }) it('should unmount after exiting', done => { - const wrapper = mount( + const instance = mount( { setTimeout(() => { - expect(wrapper.getStatus()).toEqual(UNMOUNTED) - expect(wrapper.divRef.current).not.toExist() + expect(instance.getStatus()).toEqual(UNMOUNTED) + expect(instance.nodeRef.current).not.toExist() done() }) }} /> ).instance() - expect(wrapper.getStatus()).toEqual(ENTERED) - expect(wrapper.divRef.current).toExist() + expect(instance.getStatus()).toEqual(ENTERED) + expect(instance.nodeRef.current).toExist() - wrapper.setState({ in: false }) + instance.setState({ in: false }) }) }) }) diff --git a/test/TransitionGroup-test.js b/test/TransitionGroup-test.js index 91ed167d..f921ae40 100644 --- a/test/TransitionGroup-test.js +++ b/test/TransitionGroup-test.js @@ -20,18 +20,19 @@ describe('TransitionGroup', () => { log = [] let events = { - onEnter: (_, m) => log.push(m ? 'appear' : 'enter'), - onEntering: (_, m) => log.push(m ? 'appearing' : 'entering'), - onEntered: (_, m) => log.push(m ? 'appeared' : 'entered'), + onEnter: (m) => log.push(m ? 'appear' : 'enter'), + onEntering: (m) => log.push(m ? 'appearing' : 'entering'), + onEntered: (m) => log.push(m ? 'appeared' : 'entered'), onExit: () => log.push('exit'), onExiting: () => log.push('exiting'), onExited: () => log.push('exited'), } + const nodeRef = React.createRef() Child = function Child(props) { return ( - - + + ) } diff --git a/test/setup.js b/test/setup.js index 4b7566f9..eafd445b 100644 --- a/test/setup.js +++ b/test/setup.js @@ -1,3 +1,4 @@ +import { StrictMode, createElement } from 'react' global.requestAnimationFrame = function(callback) { setTimeout(callback, 0); @@ -6,4 +7,7 @@ global.requestAnimationFrame = function(callback) { const Enzyme = require('enzyme'); const Adapter = require('enzyme-adapter-react-16'); -Enzyme.configure({ adapter: new Adapter() }) +Enzyme.configure({ + adapter: new Adapter(), + wrappingComponent: props => createElement(StrictMode, props) +}) From 2e5619b345b7bcba914b5a92072d1fd9fe21462e Mon Sep 17 00:00:00 2001 From: Andrew Luca Date: Sat, 25 Apr 2020 19:51:13 +0300 Subject: [PATCH 2/7] refactor: adapt stories to use `nodeRef` --- .storybook/config.js | 11 +++-- stories/CSSTransitionGroupFixture.js | 38 +++++++-------- stories/NestedTransition.js | 14 +++--- stories/ReplaceTransition.js | 46 ++++++++++-------- stories/Transition.js | 12 ++--- stories/TransitionGroup.js | 12 +++-- stories/transitions/Bootstrap.js | 71 ++++++++++++++++------------ stories/transitions/Fade.js | 9 +++- stories/transitions/Scale.js | 9 +++- 9 files changed, 121 insertions(+), 101 deletions(-) diff --git a/.storybook/config.js b/.storybook/config.js index 9154670a..c608f855 100644 --- a/.storybook/config.js +++ b/.storybook/config.js @@ -1,7 +1,12 @@ -import { configure } from '@storybook/react'; +import { configure, addDecorator } from '@storybook/react' +import { createElement, StrictMode } from 'react' + +addDecorator( + storyFn => createElement(StrictMode, undefined, storyFn()), +) function loadStories() { - require('../stories'); + require('../stories') } -configure(loadStories, module); +configure(loadStories, module) diff --git a/stories/CSSTransitionGroupFixture.js b/stories/CSSTransitionGroupFixture.js index 2b3ac0c0..b303d350 100644 --- a/stories/CSSTransitionGroupFixture.js +++ b/stories/CSSTransitionGroupFixture.js @@ -1,18 +1,16 @@ -import React from 'react'; +import React, { useRef } from 'react' import TransitionGroup from '../src/TransitionGroup'; import StoryFixture from './StoryFixture'; class CSSTransitionGroupFixture extends React.Component { - constructor(props, context) { - super(props, context); - - let items = props.items || []; + static defaultProps = { + items: [] + } - this.count = items.length; - this.state = { - items, - }; + count = this.props.items.length + state = { + items: this.props.items } handleAddItem = () => { @@ -39,7 +37,8 @@ class CSSTransitionGroupFixture extends React.Component { } render() { - const { items: _, description, children, ...props } = this.props; + const { items: _, description, children, ...rest } = this.props; + const { type: Transition, props: transitionProps } = React.Children.only(children) return ( @@ -52,18 +51,13 @@ class CSSTransitionGroupFixture extends React.Component { Remove a few
- - {this.state.items.map(item => React.cloneElement(children, { - key: item, - children: ( -
- {item} - -
- ) - }))} + + {this.state.items.map(item => ( + + {item} + + + ))} ); diff --git a/stories/NestedTransition.js b/stories/NestedTransition.js index 7e6f2905..34e987ea 100644 --- a/stories/NestedTransition.js +++ b/stories/NestedTransition.js @@ -1,4 +1,4 @@ -import React, { useState } from 'react'; +import React, { useRef, useState } from 'react' import StoryFixture from './StoryFixture'; import Fade from './transitions/Fade'; @@ -7,9 +7,8 @@ import Scale from './transitions/Scale'; function FadeAndScale(props) { return ( -
-
I will fade
- {/* +
I will fade
+ {/* We also want to scale in at the same time so we pass the `in` state here as well, so it enters at the same time as the Fade. @@ -17,10 +16,9 @@ function FadeAndScale(props) { will mount at the same time as the div we want to scale, so we need to tell it to animate as it _appears_. */} - -
I should scale
- -
+ + I should scale + ); } diff --git a/stories/ReplaceTransition.js b/stories/ReplaceTransition.js index 22e397c6..ff62d5b0 100644 --- a/stories/ReplaceTransition.js +++ b/stories/ReplaceTransition.js @@ -40,7 +40,6 @@ const styles = css` const defaultProps = { in: false, - delay: false, timeout: FADE_TIMEOUT * 2, } @@ -56,8 +55,6 @@ function Fade(props) { Fade.defaultProps = defaultProps; -export default Fade; - function Example({ children }) { const [show, setShow] = useState(false); @@ -77,20 +74,29 @@ function Example({ children }) { storiesOf('Replace Transition', module) - .add('Animates on all', () => ( - - - console.log('onEnter')} - onEntering={() => console.log('onEntering')} - onEntered={() => console.log('onEntered')} - onExit={() => console.log('onExit')} - onExiting={() => console.log('onExiting')} - onExited={() => console.log('onExited')} - > -
in True
 -
in False
 - - - )) + .add('Animates on all', () => { + const firstNodeRef = React.createRef() + const secondNodeRef = React.createRef() + return ( + + console.log('onEnter')} + onEntering={() => console.log('onEntering')} + onEntered={() => console.log('onEntered')} + onExit={() => console.log('onExit')} + onExiting={() => console.log('onExiting')} + onExited={() => console.log('onExited')}> + +
in True
+ + +
in False
+ + + + ) + }) diff --git a/stories/Transition.js b/stories/Transition.js index e12d0b60..2c0aade1 100644 --- a/stories/Transition.js +++ b/stories/Transition.js @@ -26,19 +26,15 @@ function ToggleFixture({ defaultIn, description, children }) { storiesOf('Transition', module) .add('Bootstrap Fade', () => ( - -
asaghasg asgasg
- + asaghasg asgasg )) .add('Bootstrap Collapse', () => ( -
- asaghasg asgasg -
foo
-
bar
-
+ asaghasg asgasg +
foo
+
bar
 )) diff --git a/stories/TransitionGroup.js b/stories/TransitionGroup.js index 17bba6e6..31a33452 100644 --- a/stories/TransitionGroup.js +++ b/stories/TransitionGroup.js @@ -1,4 +1,4 @@ -import React, { useEffect, useState } from 'react'; +import React, { useEffect, useRef, useState } from 'react' import { storiesOf } from '@storybook/react'; import TransitionGroup from '../src/TransitionGroup'; @@ -88,6 +88,7 @@ storiesOf('Css Transition Group', module) ; class DynamicTransition extends React.Component { + nodeRef = React.createRef() state = { count: 0 } handleClick = () => { this.setState({ hide: !this.state.hide }) @@ -107,8 +108,8 @@ class DynamicTransition extends React.Component { {!hide && - -
Changing! {count}
+ +
Changing! {count}
 } @@ -119,6 +120,7 @@ class DynamicTransition extends React.Component { function ReEnterTransition() { const [hide, setHide] = useState(false); + const nodeRef = useRef() useEffect(() => { if (hide) { @@ -140,8 +142,8 @@ function ReEnterTransition() { {!hide && ( - -
I'm entering!
+ +
I'm entering!
 )} diff --git a/stories/transitions/Bootstrap.js b/stories/transitions/Bootstrap.js index ae98f597..1c8c69b4 100644 --- a/stories/transitions/Bootstrap.js +++ b/stories/transitions/Bootstrap.js @@ -1,5 +1,5 @@ import { css } from 'astroturf'; -import React from 'react'; +import React, { useRef } from 'react' import style from 'dom-helpers/css'; import Transition, { EXITED, ENTERED, ENTERING, EXITING } @@ -35,17 +35,23 @@ const fadeStyles = { [ENTERED]: styles.in, } -export const Fade = props => ( - - {status => React.cloneElement(props.children, { - className: `${styles.fade} ${fadeStyles[status] || ''}` - })} - -) +export function Fade(props) { + const nodeRef = useRef() + return ( + + {status => ( +
+ {props.children} +
+ )} + + ) +} function getHeight(elem) { let value = elem.offsetHeight; @@ -66,45 +72,48 @@ const collapseStyles = { } export class Collapse extends React.Component { + nodeRef = React.createRef() + /* -- Expanding -- */ - handleEnter = (elem) => { - elem.style.height = '0'; + handleEnter = () => { + this.nodeRef.current.style.height = '0'; } - handleEntering = (elem) => { - elem.style.height = `${elem.scrollHeight}px`; + handleEntering = () => { + this.nodeRef.current.style.height = `${this.nodeRef.current.scrollHeight}px`; } - handleEntered = (elem) => { - elem.style.height = null; + handleEntered = () => { + this.nodeRef.current.style.height = null; } /* -- Collapsing -- */ - handleExit = (elem) => { - elem.style.height = getHeight(elem) + 'px'; - elem.offsetHeight; // eslint-disable-line no-unused-expressions + handleExit = () => { + this.nodeRef.current.style.height = getHeight(this.nodeRef.current) + 'px'; + this.nodeRef.current.offsetHeight; // eslint-disable-line no-unused-expressions } - handleExiting = (elem) => { - elem.style.height = '0'; + handleExiting = () => { + this.nodeRef.current.style.height = '0'; } render() { - const { children } = this.props; + const { children, ...rest } = this.props; return ( - {(state, props) => React.cloneElement(children, { - ...props, - className: collapseStyles[state] - })} + onExiting={this.handleExiting}> + {(state, props) => ( +
+ {children} +
+ )} ); } diff --git a/stories/transitions/Fade.js b/stories/transitions/Fade.js index d8e9c360..4a3937f8 100644 --- a/stories/transitions/Fade.js +++ b/stories/transitions/Fade.js @@ -1,5 +1,5 @@ import { css } from 'astroturf'; -import React from 'react'; +import React, { useRef } from 'react' import CSSTransition from '../../src/CSSTransition'; @@ -32,7 +32,12 @@ const defaultProps = { }; function Fade(props) { - return ; + const nodeRef = useRef() + return ( + +
{props.children}
+ + ); } Fade.defaultProps = defaultProps; diff --git a/stories/transitions/Scale.js b/stories/transitions/Scale.js index cd46f361..7a8fffd7 100644 --- a/stories/transitions/Scale.js +++ b/stories/transitions/Scale.js @@ -1,5 +1,5 @@ import { css } from 'astroturf'; -import React from 'react'; +import React, { useRef } from 'react' import CSSTransition from '../../src/CSSTransition'; @@ -32,7 +32,12 @@ const defaultProps = { }; function Scale(props) { - return ; + const nodeRef = useRef() + return ( + +
{props.children}
+ + ); } Scale.defaultProps = defaultProps; From af2160c54ed978074eb65384ed48311b4534fe25 Mon Sep 17 00:00:00 2001 From: Andrew Luca Date: Sat, 25 Apr 2020 20:29:37 +0300 Subject: [PATCH 3/7] test: check when `findDOMNode` is called w or w/o `nodeRef` --- src/Transition.js | 4 ++++ test/Transition-test.js | 30 ++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/src/Transition.js b/src/Transition.js index bb8fd7fb..fae901cc 100644 --- a/src/Transition.js +++ b/src/Transition.js @@ -545,6 +545,10 @@ Transition.propTypes = { /** * A react reference to DOM element that need to transition * https://stackoverflow.com/a/51127130/4671932 + * Be aware when changing `key` prop of `Transition` in a `TransitionGroup` + * a new `nodeRef` need to be provided to `Transition` with changed `key` prop + * e.g. test/CSSTransition-test.js + * CSSTransition > reentering > should remove dynamically applied classes */ nodeRef: PropTypes.shape({ current: PropTypes.instanceOf(Element) }) } diff --git a/test/Transition-test.js b/test/Transition-test.js index a1ad8c6c..c3690fa3 100644 --- a/test/Transition-test.js +++ b/test/Transition-test.js @@ -1,4 +1,5 @@ import React from 'react' +import ReactDOM from 'react-dom' import { mount } from 'enzyme' @@ -169,6 +170,35 @@ describe('Transition', () => { }) }) + it('should use `React.findDOMNode` when `nodeRef` is not provided', () => { + const consoleSpy = jest.spyOn(console, 'error').mockImplementation() + const findDOMNodeSpy = jest.spyOn(ReactDOM, 'findDOMNode') + + mount( + +
+ + ) + + expect(findDOMNodeSpy).toHaveBeenCalled(); + findDOMNodeSpy.mockRestore() + consoleSpy.mockRestore() + }) + + it('should not use `React.findDOMNode` when `nodeRef` is provided', () => { + const findDOMNodeSpy = jest.spyOn(ReactDOM, 'findDOMNode') + + const nodeRef = React.createRef() + mount( + +
+ + ) + + expect(findDOMNodeSpy).not.toHaveBeenCalled(); + findDOMNodeSpy.mockRestore() + }) + describe('appearing timeout', () => { it('should use enter timeout if appear not set', done => { let calledBeforeEntered = false From a8d9e3c45b3aaa426606f56ef907b85971efe4f6 Mon Sep 17 00:00:00 2001 From: Andrew Luca Date: Mon, 27 Apr 2020 21:22:44 +0300 Subject: [PATCH 4/7] docs: add two more stories how to use with innerRef and forwardRef --- stories/Transition.js | 25 ++++++++++++++--- stories/transitions/Bootstrap.js | 46 +++++++++++++++++++++++++++++++- 2 files changed, 66 insertions(+), 5 deletions(-) diff --git a/stories/Transition.js b/stories/Transition.js index 2c0aade1..21df6adf 100644 --- a/stories/Transition.js +++ b/stories/Transition.js @@ -2,17 +2,17 @@ import React, { useState } from 'react' import { storiesOf } from '@storybook/react' import StoryFixture from './StoryFixture' -import { Fade, Collapse } from './transitions/Bootstrap' +import { Fade, Collapse, FadeForwardRef, FadeInnerRef } from './transitions/Bootstrap' function ToggleFixture({ defaultIn, description, children }) { - const [show, setShow] = useState(defaultIn); + const [show, setShow] = useState(defaultIn) return (
{React.cloneElement(children, { in: show })} - ); + ) } storiesOf('Transition', module) @@ -38,3 +38,20 @@ storiesOf('Transition', module) )) + .add('Fade using React.forwardRef', () => { + const nodeRef = React.createRef() + return ( + + Fade using React.forwardRef + + ) + }) + .add('Fade using innerRef', () => { + const nodeRef = React.createRef() + return ( + + Fade using innerRef + + ) + }) + diff --git a/stories/transitions/Bootstrap.js b/stories/transitions/Bootstrap.js index 1c8c69b4..78232e3a 100644 --- a/stories/transitions/Bootstrap.js +++ b/stories/transitions/Bootstrap.js @@ -1,5 +1,5 @@ import { css } from 'astroturf'; -import React, { useRef } from 'react' +import React, { useEffect, useRef } from 'react' import style from 'dom-helpers/css'; import Transition, { EXITED, ENTERED, ENTERING, EXITING } @@ -118,3 +118,47 @@ export class Collapse extends React.Component { ); } } + +export function FadeInnerRef(props) { + const nodeRef = useMergedRef(props.innerRef) + return ( + + {status => ( +
+ {props.children} +
+ )} + + ) +} + +export const FadeForwardRef = React.forwardRef((props, ref) => { + return +}) + +/** + * Compose multiple refs, there may be different implementations + * This one is derived from + * e.g. https://github.com/react-restart/hooks/blob/ed37bf3dfc8fc1d9234a6d8fe0af94d69fad3b74/src/useMergedRefs.ts + * Also here are good discussion about this + * https://github.com/facebook/react/issues/13029 + * @param ref + * @returns {React.MutableRefObject} + */ +function useMergedRef(ref) { + const nodeRef = React.useRef() + useEffect(function () { + if (ref) { + if (typeof ref === 'function') { + ref(nodeRef.current) + } else { + ref.current = nodeRef.current + } + } + }) + return nodeRef +} From a27ffa0faa80100c4f5b90633060760a6c5218b0 Mon Sep 17 00:00:00 2001 From: Andrew Luca Date: Fri, 1 May 2020 17:25:43 +0300 Subject: [PATCH 5/7] test: improve tests, stories, and move `nodeRef` propType upper --- .storybook/config.js | 10 ++++---- src/Transition.js | 24 ++++++++++--------- stories/CSSTransitionGroupFixture.js | 9 +++---- stories/TransitionGroup.js | 12 ++-------- test/CSSTransition-test.js | 36 +++++++++++++++------------- test/CSSTransitionGroup-test.js | 22 +++++++---------- test/setup.js | 4 ++-- 7 files changed, 55 insertions(+), 62 deletions(-) diff --git a/.storybook/config.js b/.storybook/config.js index c608f855..6fcf7bfa 100644 --- a/.storybook/config.js +++ b/.storybook/config.js @@ -1,12 +1,12 @@ -import { configure, addDecorator } from '@storybook/react' -import { createElement, StrictMode } from 'react' +import { configure, addDecorator } from '@storybook/react'; +import React from 'react'; addDecorator( - storyFn => createElement(StrictMode, undefined, storyFn()), + storyFn => {storyFn()}, ) function loadStories() { - require('../stories') + require('../stories'); } -configure(loadStories, module) +configure(loadStories, module); diff --git a/src/Transition.js b/src/Transition.js index fae901cc..e43d815b 100644 --- a/src/Transition.js +++ b/src/Transition.js @@ -169,7 +169,7 @@ class Transition extends React.Component { this.updateStatus(true, this.appearStatus) } - componentDidUpdate(prevProps, _prevState, _snapshot) { + componentDidUpdate(prevProps) { let nextStatus = null if (prevProps !== this.props) { const { status } = this.state @@ -383,6 +383,18 @@ class Transition extends React.Component { } Transition.propTypes = { + /** + * A react reference to DOM element that need to transition + * https://stackoverflow.com/a/51127130/4671932 + * Note: When `nodeRef` prop is used, `node` is not passed to callback functions (e.g. `onEnter`) + * because user already has direct access to the node + * Note: When changing `key` prop of `Transition` in a `TransitionGroup` + * a new `nodeRef` need to be provided to `Transition` with changed `key` prop + * (see [test/CSSTransition-test.js](https://github.com/reactjs/react-transition-group/blob/master/test/CSSTransition-test.js)) + * CSSTransition > reentering > should remove dynamically applied classes + */ + nodeRef: PropTypes.shape({ current: PropTypes.instanceOf(Element) }), + /** * A `function` child can be used instead of a React element. This function is * called with the current transition status (`'entering'`, `'entered'`, @@ -541,16 +553,6 @@ Transition.propTypes = { * @type Function(node: HtmlElement) -> void */ onExited: PropTypes.func, - - /** - * A react reference to DOM element that need to transition - * https://stackoverflow.com/a/51127130/4671932 - * Be aware when changing `key` prop of `Transition` in a `TransitionGroup` - * a new `nodeRef` need to be provided to `Transition` with changed `key` prop - * e.g. test/CSSTransition-test.js - * CSSTransition > reentering > should remove dynamically applied classes - */ - nodeRef: PropTypes.shape({ current: PropTypes.instanceOf(Element) }) } // Name the function so it is clearer in the documentation diff --git a/stories/CSSTransitionGroupFixture.js b/stories/CSSTransitionGroupFixture.js index b303d350..cc2e503f 100644 --- a/stories/CSSTransitionGroupFixture.js +++ b/stories/CSSTransitionGroupFixture.js @@ -1,4 +1,4 @@ -import React, { useRef } from 'react' +import React from 'react' import TransitionGroup from '../src/TransitionGroup'; import StoryFixture from './StoryFixture'; @@ -38,7 +38,8 @@ class CSSTransitionGroupFixture extends React.Component { render() { const { items: _, description, children, ...rest } = this.props; - const { type: Transition, props: transitionProps } = React.Children.only(children) + // e.g. `Fade`, see where `CSSTransitionGroupFixture` is used + const { type: TransitionType, props: transitionTypeProps } = React.Children.only(children) return ( @@ -53,10 +54,10 @@ class CSSTransitionGroupFixture extends React.Component {
{this.state.items.map(item => ( - + {item} - + ))} diff --git a/stories/TransitionGroup.js b/stories/TransitionGroup.js index 31a33452..d7447597 100644 --- a/stories/TransitionGroup.js +++ b/stories/TransitionGroup.js @@ -88,7 +88,6 @@ storiesOf('Css Transition Group', module) ; class DynamicTransition extends React.Component { - nodeRef = React.createRef() state = { count: 0 } handleClick = () => { this.setState({ hide: !this.state.hide }) @@ -107,11 +106,7 @@ class DynamicTransition extends React.Component {
- {!hide && - -
Changing! {count}
- - } + {!hide && Changing! {count}}
) @@ -120,7 +115,6 @@ class DynamicTransition extends React.Component { function ReEnterTransition() { const [hide, setHide] = useState(false); - const nodeRef = useRef() useEffect(() => { if (hide) { @@ -142,9 +136,7 @@ function ReEnterTransition() { {!hide && ( - -
I'm entering!
- + I'm entering! )}
diff --git a/test/CSSTransition-test.js b/test/CSSTransition-test.js index 9d8aa1af..59fa557c 100644 --- a/test/CSSTransition-test.js +++ b/test/CSSTransition-test.js @@ -177,23 +177,25 @@ describe('CSSTransition', () => { }); it('should not add undefined when appearDone is not defined', done => { + const nodeRef = React.createRef() mount( { + onEnter={(isAppearing) => { expect(isAppearing).toEqual(true); - expect(node.className).toEqual('appear-test'); + expect(nodeRef.current.className).toEqual('appear-test'); }} - onEntered={(node, isAppearing) => { + onEntered={(isAppearing) => { expect(isAppearing).toEqual(true); - expect(node.className).toEqual(''); + expect(nodeRef.current.className).toEqual(''); done(); }} > -
+
); }); @@ -406,8 +408,12 @@ describe('CSSTransition', () => { } } - let nodeRef = React.createRef() - const wrapper = mount() + const nodeRef = { + foo: React.createRef(), + bar: React.createRef(), + } + + const wrapper = mount() const rerender = getProps => new Promise(resolve => wrapper.setProps({ @@ -421,38 +427,34 @@ describe('CSSTransition', () => { }) ); - nodeRef = React.createRef() - await rerender(resolve => ({ direction: 'up', text: 'bar', - nodeRef, + nodeRef: nodeRef.bar, onEnter() { count++; - expect(nodeRef.current.className).toEqual('up-enter'); + expect(nodeRef.bar.current.className).toEqual('up-enter'); }, onEntering() { count++; - expect(nodeRef.current.className).toEqual('up-enter up-enter-active'); + expect(nodeRef.bar.current.className).toEqual('up-enter up-enter-active'); resolve() } })) - nodeRef = React.createRef() - await rerender(resolve => ({ direction: 'down', text: 'foo', - nodeRef, + nodeRef: nodeRef.foo, onEntering() { count++; - expect(nodeRef.current.className).toEqual('down-enter down-enter-active'); + expect(nodeRef.foo.current.className).toEqual('down-enter down-enter-active'); }, onEntered() { count++; - expect(nodeRef.current.className).toEqual('down-enter-done'); + expect(nodeRef.foo.current.className).toEqual('down-enter-done'); resolve(); } })) diff --git a/test/CSSTransitionGroup-test.js b/test/CSSTransitionGroup-test.js index dae08b59..2776a8e0 100644 --- a/test/CSSTransitionGroup-test.js +++ b/test/CSSTransitionGroup-test.js @@ -10,7 +10,15 @@ let TransitionGroup; describe('CSSTransitionGroup', () => { let container; let consoleErrorSpy; - let YoloTransition; + + function YoloTransition({ id, ...props }) { + const nodeRef = React.useRef() + return ( + + + + ) + } beforeEach(() => { jest.resetModuleRegistry(); @@ -21,18 +29,6 @@ describe('CSSTransitionGroup', () => { TransitionGroup = require('../src/TransitionGroup'); - YoloTransition = class extends React.Component { - nodeRef = React.createRef() - render() { - let { id, ...props } = this.props - return ( - - - - ) - } - } - container = document.createElement('div'); consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); }); diff --git a/test/setup.js b/test/setup.js index eafd445b..f053547a 100644 --- a/test/setup.js +++ b/test/setup.js @@ -1,4 +1,4 @@ -import { StrictMode, createElement } from 'react' +import React from 'react' global.requestAnimationFrame = function(callback) { setTimeout(callback, 0); @@ -9,5 +9,5 @@ const Adapter = require('enzyme-adapter-react-16'); Enzyme.configure({ adapter: new Adapter(), - wrappingComponent: props => createElement(StrictMode, props) + wrappingComponent: props => }) From ff2fdfc40f37b81b08a6e23db6f400a5e107d86f Mon Sep 17 00:00:00 2001 From: Andrew Luca Date: Tue, 5 May 2020 20:01:08 +0300 Subject: [PATCH 6/7] fix(ReplaceTransition): check child `nodeRef` instead of adding 2 props --- src/ReplaceTransition.js | 17 ++++------------- stories/ReplaceTransition.js | 2 -- 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/src/ReplaceTransition.js b/src/ReplaceTransition.js index 4a3fb7c4..746ed8d7 100644 --- a/src/ReplaceTransition.js +++ b/src/ReplaceTransition.js @@ -29,11 +29,11 @@ class ReplaceTransition extends React.Component { if (child.props[handler]) child.props[handler](...originalArgs) if (this.props[handler]) { - const nodeRef = idx === 0 - ? this.props.firstNodeRef - : this.props.secondNodeRef + const maybeNode = child.props.nodeRef + ? undefined + : ReactDOM.findDOMNode(this) - this.props[handler](nodeRef ? undefined : ReactDOM.findDOMNode(this)) + this.props[handler](maybeNode) } } @@ -51,8 +51,6 @@ class ReplaceTransition extends React.Component { delete props.onExit; delete props.onExiting; delete props.onExited; - delete props.firstNodeRef; - delete props.secondNodeRef; return ( @@ -84,13 +82,6 @@ ReplaceTransition.propTypes = { return null; }, - - /** - * A react reference to DOM element that need to transition - * https://stackoverflow.com/a/51127130/4671932 - */ - firstNodeRef: PropTypes.shape({ current: PropTypes.instanceOf(Element) }), - secondNodeRef: PropTypes.shape({ current: PropTypes.instanceOf(Element) }) }; export default ReplaceTransition; diff --git a/stories/ReplaceTransition.js b/stories/ReplaceTransition.js index ff62d5b0..9f6a0399 100644 --- a/stories/ReplaceTransition.js +++ b/stories/ReplaceTransition.js @@ -81,8 +81,6 @@ storiesOf('Replace Transition', module) console.log('onEnter')} onEntering={() => console.log('onEntering')} From 5c1daf59748e67b2566bcd5b0c4a8fac02a8b476 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20Marohni=C4=87?= Date: Tue, 5 May 2020 20:39:20 +0200 Subject: [PATCH 7/7] Update documentation formatting for `nodeRef` --- src/CSSTransition.js | 18 ++++++++++++------ src/Transition.js | 38 +++++++++++++++++++++++--------------- 2 files changed, 35 insertions(+), 21 deletions(-) diff --git a/src/CSSTransition.js b/src/CSSTransition.js index cd61d6a2..d0240b62 100644 --- a/src/CSSTransition.js +++ b/src/CSSTransition.js @@ -316,7 +316,8 @@ CSSTransition.propTypes = { /** * A `` callback fired immediately after the 'enter' or 'appear' class is * applied. - * Note: when `nodeRef` prop is passed, `node` is not passed + * + * **Note**: when `nodeRef` prop is passed, `node` is not passed. * * @type Function(node: HtmlElement, isAppearing: bool) */ @@ -325,7 +326,8 @@ CSSTransition.propTypes = { /** * A `` callback fired immediately after the 'enter-active' or * 'appear-active' class is applied. - * Note: when `nodeRef` prop is passed, `node` is not passed + * + * **Note**: when `nodeRef` prop is passed, `node` is not passed. * * @type Function(node: HtmlElement, isAppearing: bool) */ @@ -334,7 +336,8 @@ CSSTransition.propTypes = { /** * A `` callback fired immediately after the 'enter' or * 'appear' classes are **removed** and the `done` class is added to the DOM node. - * Note: when `nodeRef` prop is passed, `node` is not passed + * + * **Note**: when `nodeRef` prop is passed, `node` is not passed. * * @type Function(node: HtmlElement, isAppearing: bool) */ @@ -343,7 +346,8 @@ CSSTransition.propTypes = { /** * A `` callback fired immediately after the 'exit' class is * applied. - * Note: when `nodeRef` prop is passed, `node` is not passed + * + * **Note**: when `nodeRef` prop is passed, `node` is not passed * * @type Function(node: HtmlElement) */ @@ -351,7 +355,8 @@ CSSTransition.propTypes = { /** * A `` callback fired immediately after the 'exit-active' is applied. - * Note: when `nodeRef` prop is passed, `node` is not passed + * + * **Note**: when `nodeRef` prop is passed, `node` is not passed * * @type Function(node: HtmlElement) */ @@ -360,7 +365,8 @@ CSSTransition.propTypes = { /** * A `` callback fired immediately after the 'exit' classes * are **removed** and the `exit-done` class is added to the DOM node. - * Note: when `nodeRef` prop is passed, `node` is not passed + * + * **Note**: when `nodeRef` prop is passed, `node` is not passed * * @type Function(node: HtmlElement) */ diff --git a/src/Transition.js b/src/Transition.js index e43d815b..116852e3 100644 --- a/src/Transition.js +++ b/src/Transition.js @@ -384,14 +384,15 @@ class Transition extends React.Component { Transition.propTypes = { /** - * A react reference to DOM element that need to transition + * A React reference to DOM element that need to transition: * https://stackoverflow.com/a/51127130/4671932 - * Note: When `nodeRef` prop is used, `node` is not passed to callback functions (e.g. `onEnter`) - * because user already has direct access to the node - * Note: When changing `key` prop of `Transition` in a `TransitionGroup` - * a new `nodeRef` need to be provided to `Transition` with changed `key` prop - * (see [test/CSSTransition-test.js](https://github.com/reactjs/react-transition-group/blob/master/test/CSSTransition-test.js)) - * CSSTransition > reentering > should remove dynamically applied classes + * + * - When `nodeRef` prop is used, `node` is not passed to callback functions + * (e.g. `onEnter`) because user already has direct access to the node. + * - When changing `key` prop of `Transition` in a `TransitionGroup` a new + * `nodeRef` need to be provided to `Transition` with changed `key` prop + * (see + * [test/CSSTransition-test.js](https://github.com/reactjs/react-transition-group/blob/13435f897b3ab71f6e19d724f145596f5910581c/test/CSSTransition-test.js#L362-L437)). */ nodeRef: PropTypes.shape({ current: PropTypes.instanceOf(Element) }), @@ -491,8 +492,9 @@ Transition.propTypes = { /** * Add a custom transition end trigger. Called with the transitioning * DOM node and a `done` callback. Allows for more fine grained transition end - * logic. **Note:** Timeouts are still used as a fallback if provided. - * Note: when `nodeRef` prop is passed, `node` is not passed + * logic. Timeouts are still used as a fallback if provided. + * + * **Note**: when `nodeRef` prop is passed, `node` is not passed. * * ```jsx * addEndListener={(node, done) => { @@ -506,7 +508,8 @@ Transition.propTypes = { /** * Callback fired before the "entering" status is applied. An extra parameter * `isAppearing` is supplied to indicate if the enter stage is occurring on the initial mount - * Note: when `nodeRef` prop is passed, `node` is not passed + * + * **Note**: when `nodeRef` prop is passed, `node` is not passed. * * @type Function(node: HtmlElement, isAppearing: bool) -> void */ @@ -515,7 +518,8 @@ Transition.propTypes = { /** * Callback fired after the "entering" status is applied. An extra parameter * `isAppearing` is supplied to indicate if the enter stage is occurring on the initial mount - * Note: when `nodeRef` prop is passed, `node` is not passed + * + * **Note**: when `nodeRef` prop is passed, `node` is not passed. * * @type Function(node: HtmlElement, isAppearing: bool) */ @@ -524,7 +528,8 @@ Transition.propTypes = { /** * Callback fired after the "entered" status is applied. An extra parameter * `isAppearing` is supplied to indicate if the enter stage is occurring on the initial mount - * Note: when `nodeRef` prop is passed, `node` is not passed + * + * **Note**: when `nodeRef` prop is passed, `node` is not passed. * * @type Function(node: HtmlElement, isAppearing: bool) -> void */ @@ -532,7 +537,8 @@ Transition.propTypes = { /** * Callback fired before the "exiting" status is applied. - * Note: when `nodeRef` prop is passed, `node` is not passed + * + * **Note**: when `nodeRef` prop is passed, `node` is not passed. * * @type Function(node: HtmlElement) -> void */ @@ -540,7 +546,8 @@ Transition.propTypes = { /** * Callback fired after the "exiting" status is applied. - * Note: when `nodeRef` prop is passed, `node` is not passed + * + * **Note**: when `nodeRef` prop is passed, `node` is not passed. * * @type Function(node: HtmlElement) -> void */ @@ -548,7 +555,8 @@ Transition.propTypes = { /** * Callback fired after the "exited" status is applied. - * Note: when `nodeRef` prop is passed, `node` is not passed + * + * **Note**: when `nodeRef` prop is passed, `node` is not passed * * @type Function(node: HtmlElement) -> void */