From 0f55079fd35c293dff3aef9d624d5125856ff44a Mon Sep 17 00:00:00 2001 From: Gergen Date: Mon, 14 Oct 2019 12:14:58 -0400 Subject: [PATCH 01/11] Convert decorator class component to function component --- src/__tests__/e2e.test.js | 2 +- src/useTracking.js | 13 +-- src/withTrackingComponentDecorator.js | 149 +++++++++++++------------- 3 files changed, 80 insertions(+), 84 deletions(-) diff --git a/src/__tests__/e2e.test.js b/src/__tests__/e2e.test.js index f8226ac..6d60e74 100644 --- a/src/__tests__/e2e.test.js +++ b/src/__tests__/e2e.test.js @@ -644,7 +644,7 @@ describe('e2e', () => { const Child = () => { const trackingContext = useContext(ReactTrackingContext); - expect(Object.keys(trackingContext.tracking)).toEqual([ + expect(Object.keys(trackingContext)).toEqual([ 'dispatch', 'getTrackingData', 'process', diff --git a/src/useTracking.js b/src/useTracking.js index d472db8..6256b2d 100644 --- a/src/useTracking.js +++ b/src/useTracking.js @@ -2,9 +2,9 @@ import { useContext, useMemo } from 'react'; import { ReactTrackingContext } from './withTrackingComponentDecorator'; export default function useTracking() { - const trackingContext = useContext(ReactTrackingContext); + const { dispatch, getTrackingData } = useContext(ReactTrackingContext); - if (!(trackingContext && trackingContext.tracking)) { + if (!dispatch) { throw new Error( 'Attempting to call `useTracking` ' + 'without a ReactTrackingContext present. Did you forget to wrap the top of ' + @@ -14,12 +14,9 @@ export default function useTracking() { return useMemo( () => ({ - getTrackingData: trackingContext.tracking.getTrackingData, - trackEvent: trackingContext.tracking.dispatch, + getTrackingData, + trackEvent: dispatch, }), - [ - trackingContext.tracking.getTrackingData, - trackingContext.tracking.dispatch, - ] + [getTrackingData, dispatch] ); } diff --git a/src/withTrackingComponentDecorator.js b/src/withTrackingComponentDecorator.js index b7b447e..76ad5ee 100644 --- a/src/withTrackingComponentDecorator.js +++ b/src/withTrackingComponentDecorator.js @@ -1,5 +1,5 @@ -/* eslint-disable react/static-property-placement, react/sort-comp, react/jsx-props-no-spreading */ -import React, { Component } from 'react'; +/* eslint-disable react/jsx-props-no-spreading */ +import React, { useCallback, useContext, useEffect, useMemo } from 'react'; import PropTypes from 'prop-types'; import merge from 'deepmerge'; import hoistNonReactStatic from 'hoist-non-react-statics'; @@ -22,104 +22,103 @@ export default function withTrackingComponentDecorator( const decoratedComponentName = DecoratedComponent.displayName || DecoratedComponent.name || 'Component'; - class WithTracking extends Component { - static displayName = `WithTracking(${decoratedComponentName})`; + function WithTracking(props) { + const tracking = useContext(ReactTrackingContext); - static contextType = ReactTrackingContext; + const getProcessFn = useCallback(() => tracking && tracking.process, []); - constructor(props, context) { - super(props, context); + const getOwnTrackingData = useCallback(() => { + const ownTrackingData = + typeof trackingData === 'function' + ? trackingData(props) + : trackingData; + return ownTrackingData || {}; + }, [trackingData, props]); + + const getTrackingDataFn = useCallback(() => { + const contextGetTrackingData = + (tracking && tracking.getTrackingData) || getOwnTrackingData; + + return () => + contextGetTrackingData === getOwnTrackingData + ? getOwnTrackingData() + : merge(contextGetTrackingData(), getOwnTrackingData()); + }, [getOwnTrackingData]); + + const getTrackingDispatcher = useCallback(() => { + const contextDispatch = (tracking && tracking.dispatch) || dispatch; + return data => contextDispatch(merge(getOwnTrackingData(), data || {})); + }, [dispatch, getOwnTrackingData]); + + const trackEvent = useCallback( + (data = {}) => { + getTrackingDispatcher()(data); + }, + [getTrackingDispatcher] + ); - if (this.getProcessFn() && process) { + useEffect(() => { + const contextProcess = getProcessFn(); + const getTrackingData = getTrackingDataFn(); + + if (getProcessFn() && process) { // eslint-disable-next-line console.error( '[react-tracking] options.process should be defined once on a top-level component' ); } - this.tracking = { - trackEvent: this.trackEvent, - getTrackingData: this.getTrackingDataFn(), - }; - - this.contextValueForProvider = { - tracking: { - dispatch: this.getTrackingDispatcher(), - getTrackingData: this.getTrackingDataFn(), - process: this.getProcessFn() || process, - }, - }; - } - - componentDidMount() { - const contextProcess = this.getProcessFn(); - if ( typeof contextProcess === 'function' && typeof dispatchOnMount === 'function' ) { - this.trackEvent( + trackEvent( merge( - contextProcess(this.getOwnTrackingData()) || {}, - dispatchOnMount(this.tracking.getTrackingData()) || {} + contextProcess(getOwnTrackingData()) || {}, + dispatchOnMount(getTrackingData()) || {} ) ); } else if (typeof contextProcess === 'function') { - const processed = contextProcess(this.getOwnTrackingData()); + const processed = contextProcess(getOwnTrackingData()); if (processed || dispatchOnMount === true) { - this.trackEvent(processed); + trackEvent(processed); } } else if (typeof dispatchOnMount === 'function') { - this.trackEvent(dispatchOnMount(this.tracking.getTrackingData())); + trackEvent(dispatchOnMount(getTrackingData())); } else if (dispatchOnMount === true) { - this.trackEvent(); + trackEvent(); } - } - - getProcessFn = () => { - const { tracking } = this.context; - return tracking && tracking.process; - }; - - getOwnTrackingData = () => { - const ownTrackingData = - typeof trackingData === 'function' - ? trackingData(this.props) - : trackingData; - return ownTrackingData || {}; - }; - - getTrackingDataFn = () => { - const { tracking } = this.context; - const contextGetTrackingData = - (tracking && tracking.getTrackingData) || this.getOwnTrackingData; - - return () => - contextGetTrackingData === this.getOwnTrackingData - ? this.getOwnTrackingData() - : merge(contextGetTrackingData(), this.getOwnTrackingData()); - }; - - getTrackingDispatcher = () => { - const { tracking } = this.context; - const contextDispatch = (tracking && tracking.dispatch) || dispatch; - return data => - contextDispatch(merge(this.getOwnTrackingData(), data || {})); - }; - - trackEvent = (data = {}) => { - this.contextValueForProvider.tracking.dispatch(data); - }; - - render() { - return ( - - + }, []); + + const trackingProp = useMemo( + () => ({ + trackEvent, + getTrackingData: getTrackingDataFn(), + }), + [trackEvent, getTrackingDataFn] + ); + + const contextValue = useMemo( + () => ({ + dispatch: getTrackingDispatcher(), + getTrackingData: getTrackingDataFn(), + process: getProcessFn() || process, + }), + [getTrackingDispatcher, getTrackingDataFn, getProcessFn, process] + ); + + return useMemo( + () => ( + + - ); - } + ), + [contextValue, trackingProp] + ); } + WithTracking.displayName = `WithTracking(${decoratedComponentName})`; + hoistNonReactStatic(WithTracking, DecoratedComponent); return WithTracking; From 3e3779e498ca9643ab76eb0ad4c789e735f8390b Mon Sep 17 00:00:00 2001 From: Gergen Date: Tue, 15 Oct 2019 01:21:17 -0400 Subject: [PATCH 02/11] Revert unnecessary changes --- src/__tests__/e2e.test.js | 2 +- src/useTracking.js | 13 ++++++++----- src/withTrackingComponentDecorator.js | 10 ++++++---- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/__tests__/e2e.test.js b/src/__tests__/e2e.test.js index 6d60e74..f8226ac 100644 --- a/src/__tests__/e2e.test.js +++ b/src/__tests__/e2e.test.js @@ -644,7 +644,7 @@ describe('e2e', () => { const Child = () => { const trackingContext = useContext(ReactTrackingContext); - expect(Object.keys(trackingContext)).toEqual([ + expect(Object.keys(trackingContext.tracking)).toEqual([ 'dispatch', 'getTrackingData', 'process', diff --git a/src/useTracking.js b/src/useTracking.js index 6256b2d..d472db8 100644 --- a/src/useTracking.js +++ b/src/useTracking.js @@ -2,9 +2,9 @@ import { useContext, useMemo } from 'react'; import { ReactTrackingContext } from './withTrackingComponentDecorator'; export default function useTracking() { - const { dispatch, getTrackingData } = useContext(ReactTrackingContext); + const trackingContext = useContext(ReactTrackingContext); - if (!dispatch) { + if (!(trackingContext && trackingContext.tracking)) { throw new Error( 'Attempting to call `useTracking` ' + 'without a ReactTrackingContext present. Did you forget to wrap the top of ' + @@ -14,9 +14,12 @@ export default function useTracking() { return useMemo( () => ({ - getTrackingData, - trackEvent: dispatch, + getTrackingData: trackingContext.tracking.getTrackingData, + trackEvent: trackingContext.tracking.dispatch, }), - [getTrackingData, dispatch] + [ + trackingContext.tracking.getTrackingData, + trackingContext.tracking.dispatch, + ] ); } diff --git a/src/withTrackingComponentDecorator.js b/src/withTrackingComponentDecorator.js index 76ad5ee..7ddc7b0 100644 --- a/src/withTrackingComponentDecorator.js +++ b/src/withTrackingComponentDecorator.js @@ -23,7 +23,7 @@ export default function withTrackingComponentDecorator( DecoratedComponent.displayName || DecoratedComponent.name || 'Component'; function WithTracking(props) { - const tracking = useContext(ReactTrackingContext); + const { tracking } = useContext(ReactTrackingContext); const getProcessFn = useCallback(() => tracking && tracking.process, []); @@ -100,9 +100,11 @@ export default function withTrackingComponentDecorator( const contextValue = useMemo( () => ({ - dispatch: getTrackingDispatcher(), - getTrackingData: getTrackingDataFn(), - process: getProcessFn() || process, + tracking: { + dispatch: getTrackingDispatcher(), + getTrackingData: getTrackingDataFn(), + process: getProcessFn() || process, + }, }), [getTrackingDispatcher, getTrackingDataFn, getProcessFn, process] ); From c1695b31f8b360511c9fbb52bebef00fb0fabad9 Mon Sep 17 00:00:00 2001 From: Gergen Date: Wed, 16 Oct 2019 00:26:48 -0400 Subject: [PATCH 03/11] Update unit tests --- .../withTrackingComponentDecorator.test.js | 178 ++++++++++-------- 1 file changed, 95 insertions(+), 83 deletions(-) diff --git a/src/__tests__/withTrackingComponentDecorator.test.js b/src/__tests__/withTrackingComponentDecorator.test.js index 917fd0c..309ba01 100644 --- a/src/__tests__/withTrackingComponentDecorator.test.js +++ b/src/__tests__/withTrackingComponentDecorator.test.js @@ -1,5 +1,6 @@ +/* eslint-disable react/destructuring-assignment react/prop-types */ import React from 'react'; -import { shallow } from 'enzyme'; +import { shallow, mount } from 'enzyme'; const mockDispatchTrackingEvent = jest.fn(); jest.setMock('../dispatchTrackingEvent', mockDispatchTrackingEvent); @@ -16,133 +17,129 @@ describe('withTrackingComponentDecorator', () => { expect(typeof decorated).toBe('function'); }); - describe('with a function trackingContext', () => { + describe('with process option', () => { const props = { props: 1 }; - const context = { context: 1 }; - const trackingContext = jest.fn(() => {}); + const trackingContext = { page: 1 }; + const process = jest.fn(() => null); + + @withTrackingComponentDecorator({}, { process }) + class ParentTestComponent extends React.Component { + static displayName = 'ParentTestComponent'; + + render() { + return this.props.children; + } + } @withTrackingComponentDecorator(trackingContext) - class TestComponent { + class TestComponent extends React.Component { static displayName = 'TestComponent'; - } - const myTC = new TestComponent(props, context); + render() { + return null; + } + } beforeEach(() => { mockDispatchTrackingEvent.mockClear(); }); - it('defines the expected static properties', () => { - expect(TestComponent.displayName).toBe('WithTracking(TestComponent)'); - expect(TestComponent.contextType).toBeDefined(); - }); - - it('calls trackingContext() in getContextForProvider', () => { - expect(myTC.contextValueForProvider.tracking.getTrackingData()).toEqual( - {} + it('process function gets called', () => { + mount( + + + ); - expect(trackingContext).toHaveBeenCalledTimes(1); - }); - it('dispatches event in trackEvent', () => { - const data = { data: 1 }; - myTC.trackEvent({ data }); - expect(mockDispatchTrackingEvent).toHaveBeenCalledWith({ data }); - }); - - it('does not dispatch event in componentDidMount', () => { - myTC.componentDidMount(); + expect(process).toHaveBeenCalled(); expect(mockDispatchTrackingEvent).not.toHaveBeenCalled(); }); - - it('renders', () => { - expect(myTC.render()).toBeDefined(); - }); }); - describe('with an object trackingContext', () => { + describe('with process option from parent and dispatchOnMount option on component', () => { const props = { props: 1 }; - const context = { context: 1 }; const trackingContext = { page: 1 }; + const process = jest.fn(() => ({ event: 'pageView' })); + const dispatchOnMount = jest.fn(() => ({ specificEvent: true })); - @withTrackingComponentDecorator(trackingContext, { - dispatchOnMount: true, - }) - class TestComponent { - static displayName = 'TestComponent'; - } - - const myTC = new TestComponent(props, context); - - beforeEach(() => { - mockDispatchTrackingEvent.mockClear(); - }); - - // We'll only test what differs from the functional trackingContext variation - - it('returns the proper object in getContextForProvider', () => { - expect(Object.keys(myTC.contextValueForProvider.tracking)).toEqual([ - 'dispatch', - 'getTrackingData', - 'process', - ]); - }); - - it('dispatches event in componentDidMount', () => { - myTC.componentDidMount(); - expect(mockDispatchTrackingEvent).toHaveBeenCalledWith(trackingContext); - }); - }); + @withTrackingComponentDecorator({}, { process }) + class ParentTestComponent extends React.Component { + static displayName = 'ParentTestComponent'; - describe('with process option', () => { - const props = { props: 1 }; - const trackingContext = { page: 1 }; - const process = jest.fn(() => ({ event: 'pageView' })); - const context = { context: 1, tracking: { process } }; + render() { + return this.props.children; + } + } - @withTrackingComponentDecorator(trackingContext) - class TestComponent { + @withTrackingComponentDecorator(trackingContext, { dispatchOnMount }) + class TestComponent extends React.Component { static displayName = 'TestComponent'; - } - const myTC = new TestComponent(props, context); + render() { + return null; + } + } beforeEach(() => { mockDispatchTrackingEvent.mockClear(); }); - it('process function gets called', () => { - myTC.componentDidMount(); + it('dispatches only once when process and dispatchOnMount functions are passed', () => { + mount( + + + + ); + expect(process).toHaveBeenCalled(); + expect(dispatchOnMount).toHaveBeenCalled(); expect(mockDispatchTrackingEvent).toHaveBeenCalledWith({ page: 1, event: 'pageView', + specificEvent: true, }); + expect(mockDispatchTrackingEvent).toHaveBeenCalledTimes(1); }); }); - describe('with process option from parent and dispatchOnMount option on component', () => { - const props = { props: 1 }; - const trackingContext = { page: 1 }; + describe('with function trackingContext', () => { + const props = { page: 1 }; + const trackingContext = jest.fn(p => ({ page: p.page })); const process = jest.fn(() => ({ event: 'pageView' })); - const context = { context: 1, tracking: { process } }; const dispatchOnMount = jest.fn(() => ({ specificEvent: true })); + @withTrackingComponentDecorator({}, { process }) + class ParentTestComponent extends React.Component { + static displayName = 'ParentTestComponent'; + + render() { + return this.props.children; + } + } + @withTrackingComponentDecorator(trackingContext, { dispatchOnMount }) - class TestComponent { + class TestComponent extends React.Component { static displayName = 'TestComponent'; - } - const myTC = new TestComponent(props, context); + render() { + return null; + } + } beforeEach(() => { mockDispatchTrackingEvent.mockClear(); }); it('dispatches only once when process and dispatchOnMount functions are passed', () => { - myTC.componentDidMount(); + mount( + + + + ); + expect(process).toHaveBeenCalled(); expect(dispatchOnMount).toHaveBeenCalled(); + expect(trackingContext).toHaveBeenCalled(); expect(mockDispatchTrackingEvent).toHaveBeenCalledWith({ page: 1, event: 'pageView', @@ -156,22 +153,37 @@ describe('withTrackingComponentDecorator', () => { const props = { props: 1 }; const trackingContext = { page: 1 }; const process = jest.fn(() => null); - const context = { context: 1, tracking: { process } }; const dispatchOnMount = true; + @withTrackingComponentDecorator({}, { process }) + class ParentTestComponent extends React.Component { + static displayName = 'ParentTestComponent'; + + render() { + return this.props.children; + } + } + @withTrackingComponentDecorator(trackingContext, { dispatchOnMount }) - class TestComponent { + class TestComponent extends React.Component { static displayName = 'TestComponent'; - } - const myTC = new TestComponent(props, context); + render() { + return null; + } + } beforeEach(() => { mockDispatchTrackingEvent.mockClear(); }); it('dispatches only once when process and dispatchOnMount functions are passed', () => { - myTC.componentDidMount(); + mount( + + + + ); + expect(process).toHaveBeenCalled(); expect(mockDispatchTrackingEvent).toHaveBeenCalledWith({ page: 1, From bf14f239493bcaa6006bb0f9886464a54682e068 Mon Sep 17 00:00:00 2001 From: Gergen Date: Wed, 16 Oct 2019 00:29:28 -0400 Subject: [PATCH 04/11] Disable lint errors --- src/__tests__/withTrackingComponentDecorator.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/__tests__/withTrackingComponentDecorator.test.js b/src/__tests__/withTrackingComponentDecorator.test.js index 309ba01..9a26ff6 100644 --- a/src/__tests__/withTrackingComponentDecorator.test.js +++ b/src/__tests__/withTrackingComponentDecorator.test.js @@ -1,4 +1,4 @@ -/* eslint-disable react/destructuring-assignment react/prop-types */ +/* eslint-disable react/destructuring-assignment,react/prop-types,react/jsx-props-no-spreading */ import React from 'react'; import { shallow, mount } from 'enzyme'; From 7b16e277d954caa31d20140d83afc566949de3cb Mon Sep 17 00:00:00 2001 From: Gergen Date: Wed, 16 Oct 2019 17:14:48 -0400 Subject: [PATCH 05/11] Add test for legacy context API --- src/__tests__/e2e.test.js | 51 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/src/__tests__/e2e.test.js b/src/__tests__/e2e.test.js index f8226ac..089e13c 100644 --- a/src/__tests__/e2e.test.js +++ b/src/__tests__/e2e.test.js @@ -1,6 +1,7 @@ /* eslint-disable react/destructuring-assignment,react/no-multi-comp,react/prop-types,react/prefer-stateless-function */ /* eslint-disable jsx-a11y/control-has-associated-label */ import React, { useContext } from 'react'; +import PropTypes from 'prop-types'; import { mount } from 'enzyme'; const dispatchTrackingEvent = jest.fn(); @@ -633,6 +634,56 @@ describe('e2e', () => { expect(innerRenderCount).toEqual(1); }); + it('does not prevent components using the legacy context API from receiving updates', () => { + class Top extends React.Component { + render() { + return this.props.children; + } + } + + @track({ page: 'Page' }, { dispatchOnMount: true }) + class Page extends React.Component { + static contextTypes = { theme: PropTypes.string }; + + render() { + return {this.context.theme}; + } + } + + class App extends React.Component { + static childContextTypes = { theme: PropTypes.string }; + + constructor(props) { + super(props); + this.state = { theme: 'light' }; + } + + getChildContext() { + return { theme: this.state.theme }; + } + + render() { + return ( +
+
+ ); + } + } + + const wrapper = mount(); + expect(wrapper.find('span').text()).toBe('light'); + + wrapper.find('button').simulate('click'); + expect(wrapper.find('span').text()).toBe('dark'); + }); + it('root context items are accessible to children', () => { const { ReactTrackingContext, From e3865edd9b60401ef47108bf74bd1a48114bf7e2 Mon Sep 17 00:00:00 2001 From: Gergen Date: Wed, 16 Oct 2019 18:07:34 -0400 Subject: [PATCH 06/11] Fix legacy context test --- src/__tests__/e2e.test.js | 36 +++++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/src/__tests__/e2e.test.js b/src/__tests__/e2e.test.js index 089e13c..7f34faf 100644 --- a/src/__tests__/e2e.test.js +++ b/src/__tests__/e2e.test.js @@ -2,6 +2,7 @@ /* eslint-disable jsx-a11y/control-has-associated-label */ import React, { useContext } from 'react'; import PropTypes from 'prop-types'; +import hoistNonReactStatics from 'hoist-non-react-statics'; import { mount } from 'enzyme'; const dispatchTrackingEvent = jest.fn(); @@ -635,21 +636,41 @@ describe('e2e', () => { }); it('does not prevent components using the legacy context API from receiving updates', () => { + const withLegacyContext = DecoratedComponent => { + class WithLegacyContext extends React.Component { + static contextTypes = { theme: PropTypes.string }; + + render() { + return ( + + ); + } + } + + hoistNonReactStatics(WithLegacyContext, DecoratedComponent); + + return WithLegacyContext; + }; + + @track() class Top extends React.Component { render() { return this.props.children; } } + @withLegacyContext @track({ page: 'Page' }, { dispatchOnMount: true }) class Page extends React.Component { - static contextTypes = { theme: PropTypes.string }; - render() { - return {this.context.theme}; + return {this.props.theme}; } } + @track() class App extends React.Component { static childContextTypes = { theme: PropTypes.string }; @@ -662,13 +683,14 @@ describe('e2e', () => { return { theme: this.state.theme }; } + handleUpdateTheme = () => { + this.setState({ theme: 'dark' }); + }; + render() { return (
-
+ ); } } diff --git a/src/__tests__/hoist-non-react-statics/index.js b/src/__tests__/hoist-non-react-statics/index.js new file mode 100644 index 0000000..9f10832 --- /dev/null +++ b/src/__tests__/hoist-non-react-statics/index.js @@ -0,0 +1,90 @@ +/* eslint-disable */ +/** + * Copyright 2015, Yahoo! Inc. + * Copyrights licensed under the New BSD License. See the accompanying LICENSE file for terms. + */ +const ReactIs = require('react-is'); +const React = require('react'); +const REACT_STATICS = { + childContextTypes: true, + contextTypes: true, + defaultProps: true, + displayName: true, + getDefaultProps: true, + getDerivedStateFromProps: true, + mixins: true, + propTypes: true, + type: true, +}; + +const KNOWN_STATICS = { + name: true, + length: true, + prototype: true, + caller: true, + callee: true, + arguments: true, + arity: true, +}; + +const TYPE_STATICS = { + [ReactIs.ForwardRef]: { + ['$$typeof']: true, + render: true, + }, +}; + +const defineProperty = Object.defineProperty; +const getOwnPropertyNames = Object.getOwnPropertyNames; +const getOwnPropertySymbols = Object.getOwnPropertySymbols; +const getOwnPropertyDescriptor = Object.getOwnPropertyDescriptor; +const getPrototypeOf = Object.getPrototypeOf; +const objectPrototype = Object.prototype; + +export default function hoistNonReactStatics( + targetComponent, + sourceComponent, + blacklist +) { + if (typeof sourceComponent !== 'string') { + // don't hoist over string (html) components + + if (objectPrototype) { + const inheritedComponent = getPrototypeOf(sourceComponent); + if (inheritedComponent && inheritedComponent !== objectPrototype) { + hoistNonReactStatics(targetComponent, inheritedComponent, blacklist); + } + } + + let keys = getOwnPropertyNames(sourceComponent); + + if (getOwnPropertySymbols) { + keys = keys.concat(getOwnPropertySymbols(sourceComponent)); + } + + const targetStatics = + TYPE_STATICS[targetComponent['$$typeof']] || REACT_STATICS; + const sourceStatics = + TYPE_STATICS[sourceComponent['$$typeof']] || REACT_STATICS; + + for (let i = 0; i < keys.length; ++i) { + const key = keys[i]; + if ( + !KNOWN_STATICS[key] && + !(blacklist && blacklist[key]) && + !(sourceStatics && sourceStatics[key]) && + !(targetStatics && targetStatics[key]) + ) { + const descriptor = getOwnPropertyDescriptor(sourceComponent, key); + try { + // Avoid failures from read-only properties + defineProperty(targetComponent, key, descriptor); + } catch (e) {} + } + } + + return targetComponent; + } + + return targetComponent; +} From b8c1ca896719935525ec62aa195cfc10a5282aee Mon Sep 17 00:00:00 2001 From: Gergen Date: Thu, 17 Oct 2019 00:16:01 -0400 Subject: [PATCH 08/11] Remove unneeded test decorator --- src/__tests__/e2e.test.js | 28 +++------------------------- 1 file changed, 3 insertions(+), 25 deletions(-) diff --git a/src/__tests__/e2e.test.js b/src/__tests__/e2e.test.js index ed6cf13..1f7745c 100644 --- a/src/__tests__/e2e.test.js +++ b/src/__tests__/e2e.test.js @@ -635,7 +635,7 @@ describe('e2e', () => { expect(innerRenderCount).toEqual(1); }); - it('does not prevent components using the legacy context API from receiving updates', () => { + it('does not prevent components using the legacy context API and hoist-non-react-statics < v3.1.0 from receiving updates', () => { const withLegacyContext = DecoratedComponent => { class WithLegacyContext extends React.Component { static contextTypes = { theme: PropTypes.string }; @@ -655,27 +655,6 @@ describe('e2e', () => { return WithLegacyContext; }; - const CurrentContext = React.createContext({}); - - const withCurrentContext = DecoratedComponent => { - class WithCurrentContext extends React.Component { - static contextType = CurrentContext; - - render() { - return ( - - {/* eslint-disable-next-line react/jsx-props-no-spreading */} - {value => } - - ); - } - } - - hoistNonReactStatics(WithCurrentContext, DecoratedComponent); - - return WithCurrentContext; - }; - @track() class Top extends React.Component { render() { @@ -683,7 +662,6 @@ describe('e2e', () => { } } - @withCurrentContext @withLegacyContext @track({ page: 'Page' }, { dispatchOnMount: true }) class Page extends React.Component { @@ -711,12 +689,12 @@ describe('e2e', () => { render() { return ( - +
); } } From befd4d0381cfa9369f16dd647072ddd9e897db2a Mon Sep 17 00:00:00 2001 From: Gergen Date: Thu, 17 Oct 2019 00:18:29 -0400 Subject: [PATCH 09/11] Indicate version in folder name --- src/__tests__/e2e.test.js | 3 ++- .../index.js | 0 2 files changed, 2 insertions(+), 1 deletion(-) rename src/__tests__/{hoist-non-react-statics => hoist-non-react-statics@3.0.1}/index.js (100%) diff --git a/src/__tests__/e2e.test.js b/src/__tests__/e2e.test.js index 1f7745c..890cdaa 100644 --- a/src/__tests__/e2e.test.js +++ b/src/__tests__/e2e.test.js @@ -3,7 +3,7 @@ import React, { useContext } from 'react'; import PropTypes from 'prop-types'; import { mount } from 'enzyme'; -import hoistNonReactStatics from './hoist-non-react-statics'; +import hoistNonReactStatics from './hoist-non-react-statics@3.0.1'; const dispatchTrackingEvent = jest.fn(); jest.setMock('../dispatchTrackingEvent', dispatchTrackingEvent); @@ -650,6 +650,7 @@ describe('e2e', () => { } } + // Uses the src file from hoist-non-react-statics v3.0.1 hoistNonReactStatics(WithLegacyContext, DecoratedComponent); return WithLegacyContext; diff --git a/src/__tests__/hoist-non-react-statics/index.js b/src/__tests__/hoist-non-react-statics@3.0.1/index.js similarity index 100% rename from src/__tests__/hoist-non-react-statics/index.js rename to src/__tests__/hoist-non-react-statics@3.0.1/index.js From 7bf31e3ea79b42c157a16d150a77620b09fd5384 Mon Sep 17 00:00:00 2001 From: Gergen Date: Thu, 17 Oct 2019 17:45:43 -0400 Subject: [PATCH 10/11] Don`t try to run hoist as test file --- jest.config.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/jest.config.js b/jest.config.js index 19a966a..e34a1cd 100644 --- a/jest.config.js +++ b/jest.config.js @@ -1,3 +1,6 @@ module.exports = { setupFiles: ['raf/polyfill', './setup/enzymeAdapter.js'], + testPathIgnorePatterns: [ + '/src/__tests__/hoist-non-react-statics@3.0.1/', + ], }; From dc500d3c363aedb0195e2c264192636fbf50cc95 Mon Sep 17 00:00:00 2001 From: Gergen Date: Fri, 18 Oct 2019 12:30:28 -0400 Subject: [PATCH 11/11] Simulate older hoist-non-react-statics behavior --- jest.config.js | 3 - src/__tests__/e2e.test.js | 9 +- .../hoist-non-react-statics@3.0.1/index.js | 90 ------------------- 3 files changed, 6 insertions(+), 96 deletions(-) delete mode 100644 src/__tests__/hoist-non-react-statics@3.0.1/index.js diff --git a/jest.config.js b/jest.config.js index e34a1cd..19a966a 100644 --- a/jest.config.js +++ b/jest.config.js @@ -1,6 +1,3 @@ module.exports = { setupFiles: ['raf/polyfill', './setup/enzymeAdapter.js'], - testPathIgnorePatterns: [ - '/src/__tests__/hoist-non-react-statics@3.0.1/', - ], }; diff --git a/src/__tests__/e2e.test.js b/src/__tests__/e2e.test.js index 890cdaa..b9b9d67 100644 --- a/src/__tests__/e2e.test.js +++ b/src/__tests__/e2e.test.js @@ -1,9 +1,9 @@ /* eslint-disable react/destructuring-assignment,react/no-multi-comp,react/prop-types,react/prefer-stateless-function */ /* eslint-disable jsx-a11y/control-has-associated-label */ import React, { useContext } from 'react'; -import PropTypes from 'prop-types'; import { mount } from 'enzyme'; -import hoistNonReactStatics from './hoist-non-react-statics@3.0.1'; +import PropTypes from 'prop-types'; +import hoistNonReactStatics from 'hoist-non-react-statics'; const dispatchTrackingEvent = jest.fn(); jest.setMock('../dispatchTrackingEvent', dispatchTrackingEvent); @@ -650,9 +650,12 @@ describe('e2e', () => { } } - // Uses the src file from hoist-non-react-statics v3.0.1 hoistNonReactStatics(WithLegacyContext, DecoratedComponent); + // Explicitly hoist statc contextType to simulate behavior of + // hoist-non-react-statics versions older than v3.1.0 + WithLegacyContext.contextType = DecoratedComponent.contextType; + return WithLegacyContext; }; diff --git a/src/__tests__/hoist-non-react-statics@3.0.1/index.js b/src/__tests__/hoist-non-react-statics@3.0.1/index.js deleted file mode 100644 index 9f10832..0000000 --- a/src/__tests__/hoist-non-react-statics@3.0.1/index.js +++ /dev/null @@ -1,90 +0,0 @@ -/* eslint-disable */ -/** - * Copyright 2015, Yahoo! Inc. - * Copyrights licensed under the New BSD License. See the accompanying LICENSE file for terms. - */ -const ReactIs = require('react-is'); -const React = require('react'); -const REACT_STATICS = { - childContextTypes: true, - contextTypes: true, - defaultProps: true, - displayName: true, - getDefaultProps: true, - getDerivedStateFromProps: true, - mixins: true, - propTypes: true, - type: true, -}; - -const KNOWN_STATICS = { - name: true, - length: true, - prototype: true, - caller: true, - callee: true, - arguments: true, - arity: true, -}; - -const TYPE_STATICS = { - [ReactIs.ForwardRef]: { - ['$$typeof']: true, - render: true, - }, -}; - -const defineProperty = Object.defineProperty; -const getOwnPropertyNames = Object.getOwnPropertyNames; -const getOwnPropertySymbols = Object.getOwnPropertySymbols; -const getOwnPropertyDescriptor = Object.getOwnPropertyDescriptor; -const getPrototypeOf = Object.getPrototypeOf; -const objectPrototype = Object.prototype; - -export default function hoistNonReactStatics( - targetComponent, - sourceComponent, - blacklist -) { - if (typeof sourceComponent !== 'string') { - // don't hoist over string (html) components - - if (objectPrototype) { - const inheritedComponent = getPrototypeOf(sourceComponent); - if (inheritedComponent && inheritedComponent !== objectPrototype) { - hoistNonReactStatics(targetComponent, inheritedComponent, blacklist); - } - } - - let keys = getOwnPropertyNames(sourceComponent); - - if (getOwnPropertySymbols) { - keys = keys.concat(getOwnPropertySymbols(sourceComponent)); - } - - const targetStatics = - TYPE_STATICS[targetComponent['$$typeof']] || REACT_STATICS; - const sourceStatics = - TYPE_STATICS[sourceComponent['$$typeof']] || REACT_STATICS; - - for (let i = 0; i < keys.length; ++i) { - const key = keys[i]; - if ( - !KNOWN_STATICS[key] && - !(blacklist && blacklist[key]) && - !(sourceStatics && sourceStatics[key]) && - !(targetStatics && targetStatics[key]) - ) { - const descriptor = getOwnPropertyDescriptor(sourceComponent, key); - try { - // Avoid failures from read-only properties - defineProperty(targetComponent, key, descriptor); - } catch (e) {} - } - } - - return targetComponent; - } - - return targetComponent; -}