From 4e654e2fa136b0ec9f551a854e038da3393be41d Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Sat, 11 Aug 2018 15:31:13 -0400 Subject: [PATCH 01/18] Copy changes from original v6 experiment branch --- src/components/Provider.js | 42 +++++--- src/components/connectAdvanced.js | 172 ++++++++++++------------------ src/components/context.js | 3 + src/utils/PropTypes.js | 7 -- src/utils/Subscription.js | 87 --------------- 5 files changed, 99 insertions(+), 212 deletions(-) create mode 100644 src/components/context.js delete mode 100644 src/utils/Subscription.js diff --git a/src/components/Provider.js b/src/components/Provider.js index f78e25e65..e2d597199 100644 --- a/src/components/Provider.js +++ b/src/components/Provider.js @@ -1,8 +1,10 @@ -import { Component, Children } from 'react' +import React, { Component, Children } from 'react' import PropTypes from 'prop-types' -import { storeShape, subscriptionShape } from '../utils/PropTypes' +import { storeShape } from '../utils/PropTypes' import warning from '../utils/warning' +import {ReactReduxContext} from "./context"; + let didWarnAboutReceivingStore = false function warnAboutReceivingStore() { if (didWarnAboutReceivingStore) { @@ -19,21 +21,36 @@ function warnAboutReceivingStore() { ) } -export function createProvider(storeKey = 'store') { - const subscriptionKey = `${storeKey}Subscription` +export function createProvider(storeKey = 'store', subKey) { class Provider extends Component { - getChildContext() { - return { [storeKey]: this[storeKey], [subscriptionKey]: null } + + constructor(props) { + super(props) + + const {store} = props; + + this.state = { + storeState : store.getState(), + dispatch : store.dispatch, + }; } - constructor(props, context) { - super(props, context) - this[storeKey] = props.store; + componentDidMount() { + const {store} = this.props; + + // TODO What about any actions that might have been dispatched between ctor and cDM? + this.unsubscribe = store.subscribe( () => { + this.setState({storeState : store.getState()}); + }); } render() { - return Children.only(this.props.children) + return ( + + {Children.only(this.props.children)} + + ); } } @@ -45,14 +62,11 @@ export function createProvider(storeKey = 'store') { } } + Provider.propTypes = { store: storeShape.isRequired, children: PropTypes.element.isRequired, } - Provider.childContextTypes = { - [storeKey]: storeShape.isRequired, - [subscriptionKey]: subscriptionShape, - } return Provider } diff --git a/src/components/connectAdvanced.js b/src/components/connectAdvanced.js index 46caa481d..131faa817 100644 --- a/src/components/connectAdvanced.js +++ b/src/components/connectAdvanced.js @@ -1,19 +1,19 @@ import hoistStatics from 'hoist-non-react-statics' import invariant from 'invariant' -import { Component, createElement } from 'react' +import React, { Component, createElement } from 'react' -import Subscription from '../utils/Subscription' -import { storeShape, subscriptionShape } from '../utils/PropTypes' +import {ReactReduxContext} from "./context"; +import { storeShape } from '../utils/PropTypes' let hotReloadingVersion = 0 const dummyState = {} function noop() {} -function makeSelectorStateful(sourceSelector, store) { +function makeSelectorStateful(sourceSelector) { // wrap the selector in an object that tracks its results between runs. const selector = { - run: function runComponentSelector(props) { + run: function runComponentSelector(props, storeState) { try { - const nextProps = sourceSelector(store.getState(), props) + const nextProps = sourceSelector(storeState, props) if (nextProps !== selector.props || selector.error) { selector.shouldComponentUpdate = true selector.props = nextProps @@ -75,20 +75,12 @@ export default function connectAdvanced( ...connectOptions } = {} ) { - const subscriptionKey = storeKey + 'Subscription' const version = hotReloadingVersion++ - const contextTypes = { - [storeKey]: storeShape, - [subscriptionKey]: subscriptionShape, - } - const childContextTypes = { - [subscriptionKey]: subscriptionShape, - } return function wrapWithConnect(WrappedComponent) { invariant( - typeof WrappedComponent == 'function', + typeof WrappedComponent === 'function', `You must pass a component to the function returned by ` + `${methodName}. Instead received ${JSON.stringify(WrappedComponent)}` ) @@ -112,37 +104,26 @@ export default function connectAdvanced( WrappedComponent } - // TODO Actually fix our use of componentWillReceiveProps - /* eslint-disable react/no-deprecated */ - class Connect extends Component { - constructor(props, context) { - super(props, context) + constructor(props) { + super(props) this.version = version - this.state = {} this.renderCount = 0 - this.store = props[storeKey] || context[storeKey] - this.propsMode = Boolean(props[storeKey]) + this.storeState = null; + + this.setWrappedInstance = this.setWrappedInstance.bind(this) + this.renderChild = this.renderChild.bind(this); + // TODO How do we express the invariant of needing a Provider when it's used in render()? + /* invariant(this.store, `Could not find "${storeKey}" in either the context or props of ` + `"${displayName}". Either wrap the root component in a , ` + `or explicitly pass "${storeKey}" as a prop to "${displayName}".` ) - - this.initSelector() - this.initSubscription() - } - - getChildContext() { - // If this component received store from props, its subscription should be transparent - // to any descendants receiving store+subscription from context; it passes along - // subscription passed to it. Otherwise, it shadows the parent subscription, which allows - // Connect to control ordering of notifications to flow top-down. - const subscription = this.propsMode ? null : this.subscription - return { [subscriptionKey]: subscription || this.context[subscriptionKey] } + */ } componentDidMount() { @@ -154,24 +135,22 @@ export default function connectAdvanced( // To handle the case where a child component may have triggered a state change by // dispatching an action in its componentWillMount, we have to re-run the select and maybe // re-render. - this.subscription.trySubscribe() - this.selector.run(this.props) + this.selector.run(this.props, this.storeState); if (this.selector.shouldComponentUpdate) this.forceUpdate() } - componentWillReceiveProps(nextProps) { - this.selector.run(nextProps) + + UNSAFE_componentWillReceiveProps(nextProps) { + // TODO Do we still want/need to implement sCU / cWRP now? + this.selector.run(nextProps, this.storeState); } shouldComponentUpdate() { return this.selector.shouldComponentUpdate } + componentWillUnmount() { - if (this.subscription) this.subscription.tryUnsubscribe() - this.subscription = null - this.notifyNestedSubs = noop - this.store = null this.selector.run = noop this.selector.shouldComponentUpdate = false } @@ -188,56 +167,15 @@ export default function connectAdvanced( this.wrappedInstance = ref } - initSelector() { - const sourceSelector = selectorFactory(this.store.dispatch, selectorFactoryOptions) - this.selector = makeSelectorStateful(sourceSelector, this.store) - this.selector.run(this.props) - } - - initSubscription() { - if (!shouldHandleStateChanges) return - - // parentSub's source should match where store came from: props vs. context. A component - // connected to the store via props shouldn't use subscription from context, or vice versa. - const parentSub = (this.propsMode ? this.props : this.context)[subscriptionKey] - this.subscription = new Subscription(this.store, parentSub, this.onStateChange.bind(this)) - - // `notifyNestedSubs` is duplicated to handle the case where the component is unmounted in - // the middle of the notification loop, where `this.subscription` will then be null. An - // extra null check every change can be avoided by copying the method onto `this` and then - // replacing it with a no-op on unmount. This can probably be avoided if Subscription's - // listeners logic is changed to not call listeners that have been unsubscribed in the - // middle of the notification loop. - this.notifyNestedSubs = this.subscription.notifyNestedSubs.bind(this.subscription) - } - - onStateChange() { - this.selector.run(this.props) - - if (!this.selector.shouldComponentUpdate) { - this.notifyNestedSubs() - } else { - this.componentDidUpdate = this.notifyNestedSubsOnComponentDidUpdate - this.setState(dummyState) - } - } - - notifyNestedSubsOnComponentDidUpdate() { - // `componentDidUpdate` is conditionally implemented when `onStateChange` determines it - // needs to notify nested subs. Once called, it unimplements itself until further state - // changes occur. Doing it this way vs having a permanent `componentDidUpdate` that does - // a boolean check every time avoids an extra method call most of the time, resulting - // in some perf boost. - this.componentDidUpdate = undefined - this.notifyNestedSubs() - } - - isSubscribed() { - return Boolean(this.subscription) && this.subscription.isSubscribed() + initSelector(dispatch, storeState) { + const sourceSelector = selectorFactory(dispatch, selectorFactoryOptions) + this.selector = makeSelectorStateful(sourceSelector) + this.selector.run(this.props, storeState); } addExtraProps(props) { - if (!withRef && !renderCountProp && !(this.propsMode && this.subscription)) return props + if (!withRef && !renderCountProp) return props; + // make a shallow copy so that fields added don't leak to the original selector. // this is especially important for 'ref' since that's a reference back to the component // instance. a singleton memoized selector would then be holding a reference to the @@ -245,30 +183,55 @@ export default function connectAdvanced( const withExtras = { ...props } if (withRef) withExtras.ref = this.setWrappedInstance if (renderCountProp) withExtras[renderCountProp] = this.renderCount++ - if (this.propsMode && this.subscription) withExtras[subscriptionKey] = this.subscription + return withExtras } - render() { - const selector = this.selector - selector.shouldComponentUpdate = false + renderChild(providerValue) { + const {storeState, dispatch} = providerValue; - if (selector.error) { - throw selector.error - } else { - return createElement(WrappedComponent, this.addExtraProps(selector.props)) - } + this.storeState = storeState; + + if(this.selector) { + this.selector.run(this.props, storeState); + } + else { + this.initSelector(dispatch, storeState); + } + + if (this.selector.error) { + // TODO This will unmount the whole tree now that we're throwing in render. Good idea? + // TODO Related: https://github.com/reactjs/react-redux/issues/802 + throw this.selector.error + } + else if(this.selector.shouldComponentUpdate) { + //console.log(`Re-rendering component (${displayName})`, this.selector.props); + this.selector.shouldComponentUpdate = false; + this.renderedElement = createElement(WrappedComponent, this.addExtraProps(this.selector.props)); + } + else { + //console.log(`Returning existing render result (${displayName})`, this.props) + } + + return this.renderedElement; } - } - /* eslint-enable react/no-deprecated */ + render() { + return ( + + {this.renderChild} + + ) + } + } Connect.WrappedComponent = WrappedComponent Connect.displayName = displayName - Connect.childContextTypes = childContextTypes - Connect.contextTypes = contextTypes - Connect.propTypes = contextTypes + // TODO We're losing the ability to add a store as a prop. Not sure there's anything we can do about that. + //Connect.propTypes = contextTypes + // TODO With connect no longer managing subscriptions, I _think_ is is all unneeded + /* if (process.env.NODE_ENV !== 'production') { Connect.prototype.componentWillUpdate = function componentWillUpdate() { // We are hot reloading! @@ -295,6 +258,7 @@ export default function connectAdvanced( } } } + */ return hoistStatics(Connect, WrappedComponent) } diff --git a/src/components/context.js b/src/components/context.js new file mode 100644 index 000000000..ba029da20 --- /dev/null +++ b/src/components/context.js @@ -0,0 +1,3 @@ +import React from "react"; + +export const ReactReduxContext = React.createContext(null); \ No newline at end of file diff --git a/src/utils/PropTypes.js b/src/utils/PropTypes.js index 725b02012..3177fa7a3 100644 --- a/src/utils/PropTypes.js +++ b/src/utils/PropTypes.js @@ -1,12 +1,5 @@ import PropTypes from 'prop-types' -export const subscriptionShape = PropTypes.shape({ - trySubscribe: PropTypes.func.isRequired, - tryUnsubscribe: PropTypes.func.isRequired, - notifyNestedSubs: PropTypes.func.isRequired, - isSubscribed: PropTypes.func.isRequired, -}) - export const storeShape = PropTypes.shape({ subscribe: PropTypes.func.isRequired, dispatch: PropTypes.func.isRequired, diff --git a/src/utils/Subscription.js b/src/utils/Subscription.js deleted file mode 100644 index db8146799..000000000 --- a/src/utils/Subscription.js +++ /dev/null @@ -1,87 +0,0 @@ -// encapsulates the subscription logic for connecting a component to the redux store, as -// well as nesting subscriptions of descendant components, so that we can ensure the -// ancestor components re-render before descendants - -const CLEARED = null -const nullListeners = { notify() {} } - -function createListenerCollection() { - // the current/next pattern is copied from redux's createStore code. - // TODO: refactor+expose that code to be reusable here? - let current = [] - let next = [] - - return { - clear() { - next = CLEARED - current = CLEARED - }, - - notify() { - const listeners = current = next - for (let i = 0; i < listeners.length; i++) { - listeners[i]() - } - }, - - get() { - return next - }, - - subscribe(listener) { - let isSubscribed = true - if (next === current) next = current.slice() - next.push(listener) - - return function unsubscribe() { - if (!isSubscribed || current === CLEARED) return - isSubscribed = false - - if (next === current) next = current.slice() - next.splice(next.indexOf(listener), 1) - } - } - } -} - -export default class Subscription { - constructor(store, parentSub, onStateChange) { - this.store = store - this.parentSub = parentSub - this.onStateChange = onStateChange - this.unsubscribe = null - this.listeners = nullListeners - } - - addNestedSub(listener) { - this.trySubscribe() - return this.listeners.subscribe(listener) - } - - notifyNestedSubs() { - this.listeners.notify() - } - - isSubscribed() { - return Boolean(this.unsubscribe) - } - - trySubscribe() { - if (!this.unsubscribe) { - this.unsubscribe = this.parentSub - ? this.parentSub.addNestedSub(this.onStateChange) - : this.store.subscribe(this.onStateChange) - - this.listeners = createListenerCollection() - } - } - - tryUnsubscribe() { - if (this.unsubscribe) { - this.unsubscribe() - this.unsubscribe = null - this.listeners.clear() - this.listeners = nullListeners - } - } -} From 31fc95103041e5f8f4897aa716d8f79844e623e2 Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Sat, 11 Aug 2018 17:01:14 -0400 Subject: [PATCH 02/18] Split Connect into two wrapper components --- src/components/Provider.js | 36 ++++++++++---- src/components/connectAdvanced.js | 78 ++++++++++++++++++++++++++++--- 2 files changed, 98 insertions(+), 16 deletions(-) diff --git a/src/components/Provider.js b/src/components/Provider.js index e2d597199..065886eac 100644 --- a/src/components/Provider.js +++ b/src/components/Provider.js @@ -21,7 +21,7 @@ function warnAboutReceivingStore() { ) } -export function createProvider(storeKey = 'store', subKey) { +export function createProvider(storeKey = 'store') { class Provider extends Component { @@ -32,17 +32,35 @@ export function createProvider(storeKey = 'store', subKey) { this.state = { storeState : store.getState(), - dispatch : store.dispatch, + store, }; } componentDidMount() { - const {store} = this.props; + this.subscribe(); + } + + subscribe() { + const {store} = this.props; + + this.unsubscribe = store.subscribe( () => { + const newStoreState = store.getState(); + + this.setState(providerState => { + // If the value is the same, skip the unnecessary state update. + if(providerState.storeState === newStoreState) { + return null; + } + + return {storeState : newStoreState}; + }) + }); - // TODO What about any actions that might have been dispatched between ctor and cDM? - this.unsubscribe = store.subscribe( () => { - this.setState({storeState : store.getState()}); - }); + // Actions might have been dispatched between render and mount - handle those + const postMountStoreState = store.getState(); + if(postMountStoreState !== this.state.storeState) { + this.setState({storeState : postMountStoreState}); + } } render() { @@ -55,8 +73,8 @@ export function createProvider(storeKey = 'store', subKey) { } if (process.env.NODE_ENV !== 'production') { - Provider.prototype.componentWillReceiveProps = function (nextProps) { - if (this[storeKey] !== nextProps.store) { + Provider.getDerivedStateFromProps = function (props, state) { + if (state.store !== props.store) { warnAboutReceivingStore() } } diff --git a/src/components/connectAdvanced.js b/src/components/connectAdvanced.js index 131faa817..cc0e6b27c 100644 --- a/src/components/connectAdvanced.js +++ b/src/components/connectAdvanced.js @@ -3,7 +3,7 @@ import invariant from 'invariant' import React, { Component, createElement } from 'react' import {ReactReduxContext} from "./context"; -import { storeShape } from '../utils/PropTypes' +import shallowEqual from '../utils/shallowEqual' let hotReloadingVersion = 0 const dummyState = {} @@ -104,6 +104,67 @@ export default function connectAdvanced( WrappedComponent } + + class ConnectInner extends Component { + constructor(props) { + super(props); + + this.state = { + storeState : props.storeState, + wrapperProps : props.wrapperProps, + renderCount : 0, + store : props.store, + error : null, + childPropsSelector : this.createChildSelector(props.store), + childProps : {}, + } + + this.state = { + ...this.state, + ...ConnectInner.getChildPropsState(props, this.state) + } + } + + createChildSelector(store = this.state.store) { + return selectorFactory(store.dispatch, selectorFactoryOptions) + } + + static getChildPropsState(props, state) { + try { + const nextProps = state.childPropsSelector(state.storeState, props.wrapperProps) + if (nextProps === state.childProps) return null + return { childProps: nextProps } + } catch (error) { + return { error } + } + } + + static getDerivedStateFromProps(props, state) { + if ((connectOptions.pure && shallowEqual(props.wrapperProps, state.wrapperProps)) || state.error){ + return null; + } + + const nextChildProps = ConnectInner.getChildPropsState(props, state) + + return { + ...nextChildProps, + wrapperProps : props.wrapperProps, + } + } + + shouldComponentUpdate(nextProps, nextState) { + return nextState.childProps !== this.state.childProps; + } + + render() { + if(this.state.error) { + throw this.state.error; + } + + return + } + } + class Connect extends Component { constructor(props) { super(props) @@ -113,8 +174,8 @@ export default function connectAdvanced( this.storeState = null; - this.setWrappedInstance = this.setWrappedInstance.bind(this) - this.renderChild = this.renderChild.bind(this); + //this.setWrappedInstance = this.setWrappedInstance.bind(this) + this.renderInner = this.renderInner.bind(this); // TODO How do we express the invariant of needing a Provider when it's used in render()? /* @@ -181,15 +242,16 @@ export default function connectAdvanced( // instance. a singleton memoized selector would then be holding a reference to the // instance, preventing the instance from being garbage collected, and that would be bad const withExtras = { ...props } - if (withRef) withExtras.ref = this.setWrappedInstance + //if (withRef) withExtras.ref = this.setWrappedInstance if (renderCountProp) withExtras[renderCountProp] = this.renderCount++ return withExtras } - renderChild(providerValue) { - const {storeState, dispatch} = providerValue; + renderInner(providerValue) { + const {storeState, store} = providerValue; + /* this.storeState = storeState; if(this.selector) { @@ -214,12 +276,14 @@ export default function connectAdvanced( } return this.renderedElement; + */ + return } render() { return ( - {this.renderChild} + {this.renderInner} ) } From 03749361e63bbc700e2068359056f941c55a3190 Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Sat, 11 Aug 2018 22:17:28 -0400 Subject: [PATCH 03/18] Clean up and finish reworking Connect and Provider --- src/components/Provider.js | 1 + src/components/connectAdvanced.js | 149 ++++++------------------------ 2 files changed, 29 insertions(+), 121 deletions(-) diff --git a/src/components/Provider.js b/src/components/Provider.js index 065886eac..d90b3358d 100644 --- a/src/components/Provider.js +++ b/src/components/Provider.js @@ -77,6 +77,7 @@ export function createProvider(storeKey = 'store') { if (state.store !== props.store) { warnAboutReceivingStore() } + return null; } } diff --git a/src/components/connectAdvanced.js b/src/components/connectAdvanced.js index cc0e6b27c..e5baa644d 100644 --- a/src/components/connectAdvanced.js +++ b/src/components/connectAdvanced.js @@ -1,33 +1,11 @@ import hoistStatics from 'hoist-non-react-statics' import invariant from 'invariant' -import React, { Component, createElement } from 'react' +import React, { Component, PureComponent, createElement } from 'react' import {ReactReduxContext} from "./context"; -import shallowEqual from '../utils/shallowEqual' let hotReloadingVersion = 0 -const dummyState = {} -function noop() {} -function makeSelectorStateful(sourceSelector) { - // wrap the selector in an object that tracks its results between runs. - const selector = { - run: function runComponentSelector(props, storeState) { - try { - const nextProps = sourceSelector(storeState, props) - if (nextProps !== selector.props || selector.error) { - selector.shouldComponentUpdate = true - selector.props = nextProps - selector.error = null - } - } catch (error) { - selector.shouldComponentUpdate = true - selector.error = error - } - } - } - return selector -} export default function connectAdvanced( /* @@ -105,12 +83,13 @@ export default function connectAdvanced( } + const OuterBaseComponent = connectOptions.pure ? PureComponent : Component; + class ConnectInner extends Component { constructor(props) { super(props); this.state = { - storeState : props.storeState, wrapperProps : props.wrapperProps, renderCount : 0, store : props.store, @@ -131,7 +110,7 @@ export default function connectAdvanced( static getChildPropsState(props, state) { try { - const nextProps = state.childPropsSelector(state.storeState, props.wrapperProps) + const nextProps = state.childPropsSelector(props.storeState, props.wrapperProps) if (nextProps === state.childProps) return null return { childProps: nextProps } } catch (error) { @@ -140,12 +119,12 @@ export default function connectAdvanced( } static getDerivedStateFromProps(props, state) { - if ((connectOptions.pure && shallowEqual(props.wrapperProps, state.wrapperProps)) || state.error){ + const nextChildProps = ConnectInner.getChildPropsState(props, state) + + if(nextChildProps === null) { return null; } - const nextChildProps = ConnectInner.getChildPropsState(props, state) - return { ...nextChildProps, wrapperProps : props.wrapperProps, @@ -153,7 +132,15 @@ export default function connectAdvanced( } shouldComponentUpdate(nextProps, nextState) { - return nextState.childProps !== this.state.childProps; + const childPropsChanged = nextState.childProps !== this.state.childProps; + const storeStateChanged = nextProps.storeState !== this.props.storeState; + const hasError = !!nextState.error; + + let wrapperPropsChanged = false; + + const shouldUpdate = childPropsChanged || hasError; + return shouldUpdate; + } render() { @@ -165,75 +152,13 @@ export default function connectAdvanced( } } - class Connect extends Component { + class Connect extends OuterBaseComponent { constructor(props) { super(props) - this.version = version - this.renderCount = 0 - this.storeState = null; - - - //this.setWrappedInstance = this.setWrappedInstance.bind(this) this.renderInner = this.renderInner.bind(this); - - // TODO How do we express the invariant of needing a Provider when it's used in render()? - /* - invariant(this.store, - `Could not find "${storeKey}" in either the context or props of ` + - `"${displayName}". Either wrap the root component in a , ` + - `or explicitly pass "${storeKey}" as a prop to "${displayName}".` - ) - */ } - - componentDidMount() { - if (!shouldHandleStateChanges) return - - // componentWillMount fires during server side rendering, but componentDidMount and - // componentWillUnmount do not. Because of this, trySubscribe happens during ...didMount. - // Otherwise, unsubscription would never take place during SSR, causing a memory leak. - // To handle the case where a child component may have triggered a state change by - // dispatching an action in its componentWillMount, we have to re-run the select and maybe - // re-render. - this.selector.run(this.props, this.storeState); - if (this.selector.shouldComponentUpdate) this.forceUpdate() - } - - - UNSAFE_componentWillReceiveProps(nextProps) { - // TODO Do we still want/need to implement sCU / cWRP now? - this.selector.run(nextProps, this.storeState); - } - - shouldComponentUpdate() { - return this.selector.shouldComponentUpdate - } - - - componentWillUnmount() { - this.selector.run = noop - this.selector.shouldComponentUpdate = false - } - - getWrappedInstance() { - invariant(withRef, - `To access the wrapped instance, you need to specify ` + - `{ withRef: true } in the options argument of the ${methodName}() call.` - ) - return this.wrappedInstance - } - - setWrappedInstance(ref) { - this.wrappedInstance = ref - } - - initSelector(dispatch, storeState) { - const sourceSelector = selectorFactory(dispatch, selectorFactoryOptions) - this.selector = makeSelectorStateful(sourceSelector) - this.selector.run(this.props, storeState); - } - +/* addExtraProps(props) { if (!withRef && !renderCountProp) return props; @@ -247,37 +172,19 @@ export default function connectAdvanced( return withExtras } +*/ renderInner(providerValue) { const {storeState, store} = providerValue; - /* - this.storeState = storeState; - - if(this.selector) { - this.selector.run(this.props, storeState); - } - else { - this.initSelector(dispatch, storeState); - } - - if (this.selector.error) { - // TODO This will unmount the whole tree now that we're throwing in render. Good idea? - // TODO Related: https://github.com/reactjs/react-redux/issues/802 - throw this.selector.error - } - else if(this.selector.shouldComponentUpdate) { - //console.log(`Re-rendering component (${displayName})`, this.selector.props); - this.selector.shouldComponentUpdate = false; - this.renderedElement = createElement(WrappedComponent, this.addExtraProps(this.selector.props)); - } - else { - //console.log(`Returning existing render result (${displayName})`, this.props) - } - - return this.renderedElement; - */ - return + return ( + + ); } render() { @@ -291,8 +198,8 @@ export default function connectAdvanced( Connect.WrappedComponent = WrappedComponent Connect.displayName = displayName + // TODO We're losing the ability to add a store as a prop. Not sure there's anything we can do about that. - //Connect.propTypes = contextTypes // TODO With connect no longer managing subscriptions, I _think_ is is all unneeded /* From a64103cf38787536fb710acd367da94b6570d6b6 Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Sat, 11 Aug 2018 23:12:40 -0400 Subject: [PATCH 04/18] Don't call setState in Provider if unmounting --- src/components/Provider.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/components/Provider.js b/src/components/Provider.js index d90b3358d..b169f580e 100644 --- a/src/components/Provider.js +++ b/src/components/Provider.js @@ -40,12 +40,25 @@ export function createProvider(storeKey = 'store') { this.subscribe(); } + componentWillUnmount() { + if(this.unsubscribe) { + this.unsubscribe() + this._isMounted = false; + } + } + subscribe() { const {store} = this.props; + this._isMounted = true; + this.unsubscribe = store.subscribe( () => { const newStoreState = store.getState(); + if(!this._isMounted) { + return; + } + this.setState(providerState => { // If the value is the same, skip the unnecessary state update. if(providerState.storeState === newStoreState) { From ed7a9104060a5f4b4076ad23c928f838338fcce1 Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Mon, 13 Aug 2018 23:54:06 -0400 Subject: [PATCH 05/18] Rework tests based on changes to connect and Provider --- test/components/Provider.spec.js | 202 ++++++++++++++++--------------- test/components/connect.spec.js | 95 ++------------- 2 files changed, 119 insertions(+), 178 deletions(-) diff --git a/test/components/Provider.spec.js b/test/components/Provider.spec.js index 9ada2ac0d..9aeec5b81 100644 --- a/test/components/Provider.spec.js +++ b/test/components/Provider.spec.js @@ -5,39 +5,34 @@ import PropTypes from 'prop-types' import semver from 'semver' import { createStore } from 'redux' import { Provider, createProvider, connect } from '../../src/index.js' +import {ReactReduxContext} from "../../src/components/context" import * as rtl from 'react-testing-library' import 'jest-dom/extend-expect' +import ReactDOM from "react-dom" const createExampleTextReducer = () => (state = "example text") => state; describe('React', () => { describe('Provider', () => { afterEach(() => rtl.cleanup()) + const createChild = (storeKey = 'store') => { class Child extends Component { render() { - const store = this.context[storeKey]; - - let text = ''; - - if(store) { - text = store.getState().toString() - } - return (
- {storeKey} - {text} + + {({storeState}) => { + return `${storeKey} - ${storeState}` + }} + +
) } } - - Child.contextTypes = { - [storeKey]: PropTypes.object.isRequired - } - - return Child + return Child } const Child = createChild(); @@ -90,10 +85,12 @@ describe('React', () => { } }) - it('should add the store to the child context', () => { + it('should add the store state to context', () => { const store = createStore(createExampleTextReducer()) - const spy = jest.spyOn(console, 'error').mockImplementation(() => {}) + const spy = jest.spyOn(console, 'error').mockImplementation((e) => { + const q = 42; + }) const tester = rtl.render( @@ -105,22 +102,6 @@ describe('React', () => { expect(tester.getByTestId('store')).toHaveTextContent('store - example text') }) - it('should add the store to the child context using a custom store key', () => { - const store = createStore(createExampleTextReducer()) - const CustomProvider = createProvider('customStoreKey'); - const CustomChild = createChild('customStoreKey'); - - const spy = jest.spyOn(console, 'error').mockImplementation(() => {}); - const tester = rtl.render( - - - - ) - expect(spy).toHaveBeenCalledTimes(0) - spy.mockRestore() - - expect(tester.getByTestId('store')).toHaveTextContent('customStoreKey - example text') - }) it('should warn once when receiving a new store in props', () => { const store1 = createStore((state = 10) => state + 1) @@ -190,87 +171,118 @@ describe('React', () => { innerStore.dispatch({ type: 'INC'}) expect(innerMapStateToProps).toHaveBeenCalledTimes(2) }) - }) - it('should pass state consistently to mapState', () => { - function stringBuilder(prev = '', action) { - return action.type === 'APPEND' - ? prev + action.body - : prev - } - const store = createStore(stringBuilder) + it('should pass state consistently to mapState', () => { + function stringBuilder(prev = '', action) { + return action.type === 'APPEND' + ? prev + action.body + : prev + } + + const store = createStore(stringBuilder) - store.dispatch({ type: 'APPEND', body: 'a' }) - let childMapStateInvokes = 0 + store.dispatch({ type: 'APPEND', body: 'a' }) + let childMapStateInvokes = 0 - @connect(state => ({ state }), null, null, { withRef: true }) - class Container extends Component { - emitChange() { - store.dispatch({ type: 'APPEND', body: 'b' }) - } + @connect(state => ({ state }), null, null, { withRef: true }) + class Container extends Component { + emitChange() { + store.dispatch({ type: 'APPEND', body: 'b' }) + } - render() { - return ( -
- - -
- ) + render() { + return ( +
+ + +
+ ) + } } - } - @connect((state, parentProps) => { - childMapStateInvokes++ - // The state from parent props should always be consistent with the current state - expect(state).toEqual(parentProps.parentState) - return {} - }) - class ChildContainer extends Component { - render() { - return
+ @connect((state, parentProps) => { + childMapStateInvokes++ + // The state from parent props should always be consistent with the current state + expect(state).toEqual(parentProps.parentState) + return {} + }) + class ChildContainer extends Component { + render() { + return
+ } } - } - const tester = rtl.render( - - - - ) + const tester = rtl.render( + + + + ) + + expect(childMapStateInvokes).toBe(1) - expect(childMapStateInvokes).toBe(1) + // The store state stays consistent when setState calls are batched + store.dispatch({ type: 'APPEND', body: 'c' }) + expect(childMapStateInvokes).toBe(2) - // The store state stays consistent when setState calls are batched - store.dispatch({ type: 'APPEND', body: 'c' }) - expect(childMapStateInvokes).toBe(2) + // setState calls DOM handlers are batched + const button = tester.getByText('change') + rtl.fireEvent.click(button) + expect(childMapStateInvokes).toBe(3) - // setState calls DOM handlers are batched - const button = tester.getByText('change') - rtl.fireEvent.click(button) - expect(childMapStateInvokes).toBe(3) + // Provider uses unstable_batchedUpdates() under the hood + store.dispatch({ type: 'APPEND', body: 'd' }) + expect(childMapStateInvokes).toBe(4) + }) - // Provider uses unstable_batchedUpdates() under the hood - store.dispatch({ type: 'APPEND', body: 'd' }) - expect(childMapStateInvokes).toBe(4) - }) + it.skip('works in without warnings (React 16.3+)', () => { + if (!React.StrictMode) { + return + } + const spy = jest.spyOn(console, 'error').mockImplementation(() => {}) + const store = createStore(() => ({})) - it.skip('works in without warnings (React 16.3+)', () => { - if (!React.StrictMode) { - return - } - const spy = jest.spyOn(console, 'error').mockImplementation(() => {}) - const store = createStore(() => ({})) + rtl.render( + + +
+ + + ) - rtl.render( - + expect(spy).not.toHaveBeenCalled() + }) + + + it('should unsubscribe before unmounting', () => { + const store = createStore(createExampleTextReducer()) + const subscribe = store.subscribe + + // Keep track of unsubscribe by wrapping subscribe() + const spy = jest.fn(() => ({})) + store.subscribe = (listener) => { + const unsubscribe = subscribe(listener) + return () => { + spy() + return unsubscribe() + } + } + + const div = document.createElement('div') + ReactDOM.render(
- - - ) + , + div + ) - expect(spy).not.toHaveBeenCalled() + expect(spy).toHaveBeenCalledTimes(0) + ReactDOM.unmountComponentAtNode(div) + expect(spy).toHaveBeenCalledTimes(1) + }) }) + + }) diff --git a/test/components/connect.spec.js b/test/components/connect.spec.js index da0f08071..4c326e948 100644 --- a/test/components/connect.spec.js +++ b/test/components/connect.spec.js @@ -5,7 +5,7 @@ import createClass from 'create-react-class' import PropTypes from 'prop-types' import ReactDOM from 'react-dom' import { createStore } from 'redux' -import { createProvider, connect } from '../../src/index.js' +import { createProvider, connect, Provider } from '../../src/index.js' import * as rtl from 'react-testing-library' import 'jest-dom/extend-expect' @@ -34,18 +34,7 @@ describe('React', () => { } } - class ProviderMock extends Component { - getChildContext() { - return { store: this.props.store } - } - - render() { - return Children.only(this.props.children) - } - } - ProviderMock.childContextTypes = { - store: PropTypes.object.isRequired - } + const ProviderMock = Provider; class ContextBoundStore { constructor(reducer) { @@ -91,7 +80,9 @@ describe('React', () => { } afterEach(() => rtl.cleanup()) - it('should receive the store in the context', () => { + + + it('should receive the store state in the context', () => { const store = createStore(() => ({ hi: 'there' })) @connect(state => state) @@ -846,42 +837,6 @@ describe('React', () => { runCheck(false, false, false) }) - it('should unsubscribe before unmounting', () => { - const store = createStore(stringBuilder) - const subscribe = store.subscribe - - // Keep track of unsubscribe by wrapping subscribe() - const spy = jest.fn(() => ({})) - store.subscribe = (listener) => { - const unsubscribe = subscribe(listener) - return () => { - spy() - return unsubscribe() - } - } - - @connect( - state => ({ string: state }), - dispatch => ({ dispatch }) - ) - class Container extends Component { - render() { - return - } - } - - const div = document.createElement('div') - ReactDOM.render( - - - , - div - ) - - expect(spy).toHaveBeenCalledTimes(0) - ReactDOM.unmountComponentAtNode(div) - expect(spy).toHaveBeenCalledTimes(1) - }) it('should not attempt to set state after unmounting', () => { const store = createStore(stringBuilder) @@ -1037,7 +992,7 @@ describe('React', () => { linkB.click() document.body.removeChild(div) - expect(mapStateToPropsCalls).toBe(3) + expect(mapStateToPropsCalls).toBe(2) expect(spy).toHaveBeenCalledTimes(0) spy.mockRestore() }) @@ -1338,7 +1293,7 @@ describe('React', () => { spy.mockRestore() }) - it('should recalculate the state and rebind the actions on hot update', () => { + it.skip('should recalculate the state and rebind the actions on hot update', () => { const store = createStore(() => {}) @connect( @@ -1516,7 +1471,7 @@ describe('React', () => { expect(decorated.foo).toBe('bar') }) - it('should use the store from the props instead of from the context if present', () => { + it.skip('should use the store from the props instead of from the context if present', () => { class Container extends Component { render() { return @@ -1542,7 +1497,7 @@ describe('React', () => { expect(actualState).toEqual(expectedState) }) - it('should throw an error if the store is not in the props or context', () => { + it.skip('should throw an error if the store is not in the props or context', () => { const spy = jest.spyOn(console, 'error').mockImplementation(() => {}) class Container extends Component { @@ -1563,7 +1518,7 @@ describe('React', () => { spy.mockRestore() }) - it('should throw when trying to access the wrapped instance if withRef is not specified', () => { + it.skip('should throw when trying to access the wrapped instance if withRef is not specified', () => { const store = createStore(() => ({})) class Container extends Component { @@ -1599,7 +1554,7 @@ describe('React', () => { }) - it('should return the instance of the wrapped component for use in calling child methods', async (done) => { + it.skip('should return the instance of the wrapped component for use in calling child methods', async (done) => { const store = createStore(() => ({})) const someData = { @@ -1872,24 +1827,18 @@ describe('React', () => { expect(renderCalls).toBe(1) expect(mapStateCalls).toBe(1) - const spy = jest.spyOn(Container.prototype, 'setState') store.dispatch({ type: 'APPEND', body: 'a' }) expect(mapStateCalls).toBe(2) expect(renderCalls).toBe(1) - expect(spy).toHaveBeenCalledTimes(0) store.dispatch({ type: 'APPEND', body: 'a' }) expect(mapStateCalls).toBe(3) expect(renderCalls).toBe(1) - expect(spy).toHaveBeenCalledTimes(0) store.dispatch({ type: 'APPEND', body: 'a' }) expect(mapStateCalls).toBe(4) expect(renderCalls).toBe(2) - expect(spy).toHaveBeenCalledTimes(1) - - spy.mockRestore() }) it('should not swallow errors when bailing out early', () => { @@ -2299,7 +2248,7 @@ describe('React', () => { store.dispatch({ type: 'INC' }) }) - it('should subscribe properly when a new store is provided via props', () => { + it.skip('should subscribe properly when a new store is provided via props', () => { const store1 = createStore((state = 0, action) => (action.type === 'INC' ? state + 1 : state)) const store2 = createStore((state = 0, action) => (action.type === 'INC' ? state + 1 : state)) @@ -2368,25 +2317,5 @@ describe('React', () => { expect(spy).not.toHaveBeenCalled() }) - it('should receive the store in the context using a custom store key', () => { - const store = createStore(() => ({})) - const CustomProvider = createProvider('customStoreKey') - const connectOptions = { storeKey: 'customStoreKey' } - - @connect(undefined, undefined, undefined, connectOptions) - class Container extends Component { - render() { - return - } - } - - const tester = rtl.render( - - - - ) - - expect(tester.getByTestId('dispatch')).toHaveTextContent('[function dispatch]') - }) }) }) From 3cf4a0fd95e01ba5c2acc6982a61fe379ab17587 Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Sat, 18 Aug 2018 14:48:41 -0600 Subject: [PATCH 06/18] Fix lint errors --- src/components/Provider.js | 2 +- src/components/connectAdvanced.js | 18 ++++++++++-------- test/components/Provider.spec.js | 7 ++----- test/components/connect.spec.js | 4 ++-- 4 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/components/Provider.js b/src/components/Provider.js index b169f580e..694a28b12 100644 --- a/src/components/Provider.js +++ b/src/components/Provider.js @@ -21,7 +21,7 @@ function warnAboutReceivingStore() { ) } -export function createProvider(storeKey = 'store') { +export function createProvider() { class Provider extends Component { diff --git a/src/components/connectAdvanced.js b/src/components/connectAdvanced.js index e5baa644d..099ebd9cb 100644 --- a/src/components/connectAdvanced.js +++ b/src/components/connectAdvanced.js @@ -1,8 +1,10 @@ import hoistStatics from 'hoist-non-react-statics' import invariant from 'invariant' -import React, { Component, PureComponent, createElement } from 'react' +import PropTypes from 'prop-types' +import React, { Component, PureComponent } from 'react' import {ReactReduxContext} from "./context"; +import {storeShape} from "../utils/PropTypes" let hotReloadingVersion = 0 @@ -43,9 +45,6 @@ export default function connectAdvanced( // determines whether this HOC subscribes to store changes shouldHandleStateChanges = true, - // the key of props/context to get the store - storeKey = 'store', - // if true, the wrapped element is exposed by this HOC via the getWrappedInstance() function. withRef = false, @@ -75,7 +74,6 @@ export default function connectAdvanced( methodName, renderCountProp, shouldHandleStateChanges, - storeKey, withRef, displayName, wrappedComponentName, @@ -86,6 +84,11 @@ export default function connectAdvanced( const OuterBaseComponent = connectOptions.pure ? PureComponent : Component; class ConnectInner extends Component { + static propTypes = { + wrapperProps : PropTypes.object, + store : storeShape, + }; + constructor(props) { super(props); @@ -133,11 +136,8 @@ export default function connectAdvanced( shouldComponentUpdate(nextProps, nextState) { const childPropsChanged = nextState.childProps !== this.state.childProps; - const storeStateChanged = nextProps.storeState !== this.props.storeState; const hasError = !!nextState.error; - let wrapperPropsChanged = false; - const shouldUpdate = childPropsChanged || hasError; return shouldUpdate; @@ -156,6 +156,8 @@ export default function connectAdvanced( constructor(props) { super(props) + this.version = version + this.renderInner = this.renderInner.bind(this); } /* diff --git a/test/components/Provider.spec.js b/test/components/Provider.spec.js index 9aeec5b81..c43a61914 100644 --- a/test/components/Provider.spec.js +++ b/test/components/Provider.spec.js @@ -1,10 +1,9 @@ /*eslint-disable react/prop-types*/ import React, { Component } from 'react' -import PropTypes from 'prop-types' import semver from 'semver' import { createStore } from 'redux' -import { Provider, createProvider, connect } from '../../src/index.js' +import { Provider, connect } from '../../src/index.js' import {ReactReduxContext} from "../../src/components/context" import * as rtl from 'react-testing-library' import 'jest-dom/extend-expect' @@ -88,9 +87,7 @@ describe('React', () => { it('should add the store state to context', () => { const store = createStore(createExampleTextReducer()) - const spy = jest.spyOn(console, 'error').mockImplementation((e) => { - const q = 42; - }) + const spy = jest.spyOn(console, 'error').mockImplementation(() => {}) const tester = rtl.render( diff --git a/test/components/connect.spec.js b/test/components/connect.spec.js index 4c326e948..94263e992 100644 --- a/test/components/connect.spec.js +++ b/test/components/connect.spec.js @@ -1,11 +1,11 @@ /*eslint-disable react/prop-types*/ -import React, { Children, Component } from 'react' +import React, { Component } from 'react' import createClass from 'create-react-class' import PropTypes from 'prop-types' import ReactDOM from 'react-dom' import { createStore } from 'redux' -import { createProvider, connect, Provider } from '../../src/index.js' +import { connect, Provider } from '../../src/index.js' import * as rtl from 'react-testing-library' import 'jest-dom/extend-expect' From 6bfedb1fbcacfd3a56e4cd83c30fe3c2cca1c993 Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Sat, 18 Aug 2018 15:08:36 -0600 Subject: [PATCH 07/18] Add react-is --- package-lock.json | 71 ++++++++++++++--------------------------------- package.json | 1 + 2 files changed, 22 insertions(+), 50 deletions(-) diff --git a/package-lock.json b/package-lock.json index d0b8f744b..befdf9122 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3069,8 +3069,7 @@ "ansi-regex": { "version": "2.1.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "aproba": { "version": "1.1.1", @@ -3137,7 +3136,6 @@ "version": "0.0.9", "bundled": true, "dev": true, - "optional": true, "requires": { "inherits": "~2.0.0" } @@ -3146,7 +3144,6 @@ "version": "2.10.1", "bundled": true, "dev": true, - "optional": true, "requires": { "hoek": "2.x.x" } @@ -3164,8 +3161,7 @@ "buffer-shims": { "version": "1.0.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "caseless": { "version": "0.12.0", @@ -3182,8 +3178,7 @@ "code-point-at": { "version": "1.1.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "combined-stream": { "version": "1.0.5", @@ -3203,20 +3198,17 @@ "console-control-strings": { "version": "1.1.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "core-util-is": { "version": "1.0.2", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "cryptiles": { "version": "2.0.5", "bundled": true, "dev": true, - "optional": true, "requires": { "boom": "2.x.x" } @@ -3289,8 +3281,7 @@ "extsprintf": { "version": "1.0.2", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "forever-agent": { "version": "0.6.1", @@ -3312,14 +3303,12 @@ "fs.realpath": { "version": "1.0.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "fstream": { "version": "1.0.11", "bundled": true, "dev": true, - "optional": true, "requires": { "graceful-fs": "^4.1.2", "inherits": "~2.0.0", @@ -3375,7 +3364,6 @@ "version": "7.1.2", "bundled": true, "dev": true, - "optional": true, "requires": { "fs.realpath": "^1.0.0", "inflight": "^1.0.4", @@ -3388,8 +3376,7 @@ "graceful-fs": { "version": "4.1.11", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "har-schema": { "version": "1.0.5", @@ -3417,7 +3404,6 @@ "version": "3.1.3", "bundled": true, "dev": true, - "optional": true, "requires": { "boom": "2.x.x", "cryptiles": "2.x.x", @@ -3428,8 +3414,7 @@ "hoek": { "version": "2.16.3", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "http-signature": { "version": "1.1.1", @@ -3446,7 +3431,6 @@ "version": "1.0.6", "bundled": true, "dev": true, - "optional": true, "requires": { "once": "^1.3.0", "wrappy": "1" @@ -3455,8 +3439,7 @@ "inherits": { "version": "2.0.3", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "ini": { "version": "1.3.4", @@ -3468,7 +3451,6 @@ "version": "1.0.0", "bundled": true, "dev": true, - "optional": true, "requires": { "number-is-nan": "^1.0.0" } @@ -3482,8 +3464,7 @@ "isarray": { "version": "1.0.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "isstream": { "version": "0.1.2", @@ -3572,7 +3553,6 @@ "version": "3.0.4", "bundled": true, "dev": true, - "optional": true, "requires": { "brace-expansion": "^1.1.7" } @@ -3587,7 +3567,6 @@ "version": "0.5.1", "bundled": true, "dev": true, - "optional": true, "requires": { "minimist": "0.0.8" } @@ -3661,7 +3640,6 @@ "version": "1.4.0", "bundled": true, "dev": true, - "optional": true, "requires": { "wrappy": "1" } @@ -3691,8 +3669,7 @@ "path-is-absolute": { "version": "1.0.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "performance-now": { "version": "0.2.0", @@ -3703,8 +3680,7 @@ "process-nextick-args": { "version": "1.0.7", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "punycode": { "version": "1.4.1", @@ -3742,7 +3718,6 @@ "version": "2.2.9", "bundled": true, "dev": true, - "optional": true, "requires": { "buffer-shims": "~1.0.0", "core-util-is": "~1.0.0", @@ -3787,7 +3762,6 @@ "version": "2.6.1", "bundled": true, "dev": true, - "optional": true, "requires": { "glob": "^7.0.5" } @@ -3795,8 +3769,7 @@ "safe-buffer": { "version": "5.0.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "semver": { "version": "5.3.0", @@ -3820,7 +3793,6 @@ "version": "1.0.9", "bundled": true, "dev": true, - "optional": true, "requires": { "hoek": "2.x.x" } @@ -3854,7 +3826,6 @@ "version": "1.0.2", "bundled": true, "dev": true, - "optional": true, "requires": { "code-point-at": "^1.0.0", "is-fullwidth-code-point": "^1.0.0", @@ -3865,7 +3836,6 @@ "version": "1.0.1", "bundled": true, "dev": true, - "optional": true, "requires": { "safe-buffer": "^5.0.1" } @@ -3880,7 +3850,6 @@ "version": "3.0.1", "bundled": true, "dev": true, - "optional": true, "requires": { "ansi-regex": "^2.0.0" } @@ -3895,7 +3864,6 @@ "version": "2.2.1", "bundled": true, "dev": true, - "optional": true, "requires": { "block-stream": "*", "fstream": "^1.0.2", @@ -3951,8 +3919,7 @@ "util-deprecate": { "version": "1.0.2", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "uuid": { "version": "3.0.1", @@ -3981,8 +3948,7 @@ "wrappy": { "version": "1.0.2", "bundled": true, - "dev": true, - "optional": true + "dev": true } } }, @@ -6887,6 +6853,11 @@ "prop-types": "^15.6.0" } }, + "react-is": { + "version": "16.4.2", + "resolved": "https://registry.npmjs.org/react-is/-/react-is-16.4.2.tgz", + "integrity": "sha512-rI3cGFj/obHbBz156PvErrS5xc6f1eWyTwyV4mo0vF2lGgXgS+mm7EKD5buLJq6jNgIagQescGSVG2YzgXt8Yg==" + }, "react-lifecycles-compat": { "version": "3.0.4", "resolved": "https://registry.npmjs.org/react-lifecycles-compat/-/react-lifecycles-compat-3.0.4.tgz", diff --git a/package.json b/package.json index 209b34da8..af4ae7a79 100644 --- a/package.json +++ b/package.json @@ -47,6 +47,7 @@ "invariant": "^2.2.4", "loose-envify": "^1.1.0", "prop-types": "^15.6.1", + "react-is": "^16.4.2", "react-lifecycles-compat": "^3.0.0" }, "devDependencies": { From 5df5a0f4cdd12bba7e2bdb612c94a1274f7fc982 Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Sat, 18 Aug 2018 17:43:26 -0600 Subject: [PATCH 08/18] Add missing functionality for connect and Provider Added ability to swap stores Removed single-child limitation Added invariant warnings for storeKey and withRef Added valid element check using react-is Refactored child selector creation for reusability Added prop types for components Added forwardRef and consumer as prop capability Added a tiny memoized function for wrapper props handling Removed semicolons --- src/components/Provider.js | 64 +++++------ src/components/connectAdvanced.js | 184 ++++++++++++++++-------------- 2 files changed, 131 insertions(+), 117 deletions(-) diff --git a/src/components/Provider.js b/src/components/Provider.js index 694a28b12..9268186f2 100644 --- a/src/components/Provider.js +++ b/src/components/Provider.js @@ -3,7 +3,7 @@ import PropTypes from 'prop-types' import { storeShape } from '../utils/PropTypes' import warning from '../utils/warning' -import {ReactReduxContext} from "./context"; +import {ReactReduxContext} from "./context" let didWarnAboutReceivingStore = false function warnAboutReceivingStore() { @@ -28,76 +28,76 @@ export function createProvider() { constructor(props) { super(props) - const {store} = props; + const {store} = props this.state = { storeState : store.getState(), store, - }; + } } componentDidMount() { - this.subscribe(); + this._isMounted = true + this.subscribe() } componentWillUnmount() { - if(this.unsubscribe) { - this.unsubscribe() - this._isMounted = false; + if(this.unsubscribe) this.unsubscribe() + + this._isMounted = false + } + + componentDidUpdate(prevProps) { + if(this.props.store !== prevProps.store) { + if(this.unsubscribe) this.unsubscribe() + + this.subscribe() } } subscribe() { - const {store} = this.props; - - this._isMounted = true; + const {store} = this.props this.unsubscribe = store.subscribe( () => { - const newStoreState = store.getState(); + const newStoreState = store.getState() if(!this._isMounted) { - return; + return } this.setState(providerState => { // If the value is the same, skip the unnecessary state update. if(providerState.storeState === newStoreState) { - return null; + return null } - return {storeState : newStoreState}; + return {storeState : newStoreState} }) - }); + }) // Actions might have been dispatched between render and mount - handle those - const postMountStoreState = store.getState(); + const postMountStoreState = store.getState() if(postMountStoreState !== this.state.storeState) { - this.setState({storeState : postMountStoreState}); + this.setState({storeState : postMountStoreState}) } } render() { - return ( - - {Children.only(this.props.children)} - - ); - } - } + const ContextProvider = this.props.contextProvider || ReactReduxContext.Provider - if (process.env.NODE_ENV !== 'production') { - Provider.getDerivedStateFromProps = function (props, state) { - if (state.store !== props.store) { - warnAboutReceivingStore() + return ( + + {this.props.children} + + ) } - return null; - } } Provider.propTypes = { - store: storeShape.isRequired, - children: PropTypes.element.isRequired, + store: storeShape.isRequired, + children: PropTypes.element.isRequired, + contextProvider : PropTypes.object, } return Provider diff --git a/src/components/connectAdvanced.js b/src/components/connectAdvanced.js index 099ebd9cb..183f95e42 100644 --- a/src/components/connectAdvanced.js +++ b/src/components/connectAdvanced.js @@ -2,8 +2,9 @@ import hoistStatics from 'hoist-non-react-statics' import invariant from 'invariant' import PropTypes from 'prop-types' import React, { Component, PureComponent } from 'react' +import { isValidElementType } from 'react-is' -import {ReactReduxContext} from "./context"; +import {ReactReduxContext} from "./context" import {storeShape} from "../utils/PropTypes" let hotReloadingVersion = 0 @@ -38,26 +39,41 @@ export default function connectAdvanced( // probably overridden by wrapper functions such as connect() methodName = 'connectAdvanced', - // if defined, the name of the property passed to the wrapped element indicating the number of - // calls to render. useful for watching in react devtools for unnecessary re-renders. - renderCountProp = undefined, - // determines whether this HOC subscribes to store changes shouldHandleStateChanges = true, - // if true, the wrapped element is exposed by this HOC via the getWrappedInstance() function. + + // the context consumer to use + consumer = ReactReduxContext.Consumer, + + // REMOVED: the key of props/context to get the store + storeKey = 'store', + + // REMOVED: expose the wrapped component via refs withRef = false, // additional options are passed through to the selectorFactory ...connectOptions } = {} ) { + invariant(!withRef, + "withRef is removed. To access the wrapped instance, use a ref on the connected component" + ) + + invariant(storeKey === 'store', + 'storeKey has been removed. To use a custom redux store for a single component, ' + + 'create a custom React context with React.createContext() and pass the Provider to react-redux\'s provider ' + + 'and the Consumer to this component as in <' + + 'ConnectedComponent consumer={context.Consumer} />' + ) + + const version = hotReloadingVersion++ return function wrapWithConnect(WrappedComponent) { invariant( - typeof WrappedComponent === 'function', + isValidElementType(WrappedComponent), `You must pass a component to the function returned by ` + `${methodName}. Instead received ${JSON.stringify(WrappedComponent)}` ) @@ -72,32 +88,28 @@ export default function connectAdvanced( ...connectOptions, getDisplayName, methodName, - renderCountProp, shouldHandleStateChanges, - withRef, displayName, wrappedComponentName, WrappedComponent } - const OuterBaseComponent = connectOptions.pure ? PureComponent : Component; + const OuterBaseComponent = connectOptions.pure ? PureComponent : Component - class ConnectInner extends Component { - static propTypes = { - wrapperProps : PropTypes.object, - store : storeShape, - }; + function createChildSelector(store) { + return selectorFactory(store.dispatch, selectorFactoryOptions) + } + class ConnectInner extends Component { constructor(props) { - super(props); + super(props) this.state = { wrapperProps : props.wrapperProps, - renderCount : 0, store : props.store, error : null, - childPropsSelector : this.createChildSelector(props.store), + childPropsSelector : createChildSelector(props.store), childProps : {}, } @@ -107,15 +119,20 @@ export default function connectAdvanced( } } - createChildSelector(store = this.state.store) { - return selectorFactory(store.dispatch, selectorFactoryOptions) - } + static getChildPropsState(props, state) { try { - const nextProps = state.childPropsSelector(props.storeState, props.wrapperProps) + let {childPropsSelector} = state + + if(props.store !== state.store) { + childPropsSelector = createChildSelector(props.store) + } + + const nextProps = childPropsSelector(props.storeState, props.wrapperProps) if (nextProps === state.childProps) return null - return { childProps: nextProps } + + return { childProps: nextProps, store : props.store, childPropsSelector } } catch (error) { return { error } } @@ -125,7 +142,7 @@ export default function connectAdvanced( const nextChildProps = ConnectInner.getChildPropsState(props, state) if(nextChildProps === null) { - return null; + return null } return { @@ -135,20 +152,39 @@ export default function connectAdvanced( } shouldComponentUpdate(nextProps, nextState) { - const childPropsChanged = nextState.childProps !== this.state.childProps; - const hasError = !!nextState.error; - - const shouldUpdate = childPropsChanged || hasError; - return shouldUpdate; + const childPropsChanged = nextState.childProps !== this.state.childProps + const hasError = !!nextState.error + return childPropsChanged || hasError } render() { if(this.state.error) { - throw this.state.error; + throw this.state.error + } + + return + } + } + + ConnectInner.propTypes = { + wrapperProps : PropTypes.object, + store : storeShape, + } + + + function createWrapperPropsMemoizer() { + let result, prevProps + + return function wrapperPropsMemoizer(props) { + if(props === prevProps) { + return result } - return + const {consumer, forwardRef, ...wrapperProps} = props + result = {consumer, forwardRef, wrapperProps} + + return result } } @@ -158,81 +194,59 @@ export default function connectAdvanced( this.version = version - this.renderInner = this.renderInner.bind(this); - } -/* - addExtraProps(props) { - if (!withRef && !renderCountProp) return props; - - // make a shallow copy so that fields added don't leak to the original selector. - // this is especially important for 'ref' since that's a reference back to the component - // instance. a singleton memoized selector would then be holding a reference to the - // instance, preventing the instance from being garbage collected, and that would be bad - const withExtras = { ...props } - //if (withRef) withExtras.ref = this.setWrappedInstance - if (renderCountProp) withExtras[renderCountProp] = this.renderCount++ - - return withExtras + this.renderInner = this.renderInner.bind(this) + + this.wrapperPropsMemoizer = createWrapperPropsMemoizer() } -*/ renderInner(providerValue) { - const {storeState, store} = providerValue; + const {storeState, store} = providerValue + + const {forwardRef, wrapperProps} = this.wrapperPropsMemoizer(this.props) return ( - ); + ) } render() { - return ( - - {this.renderInner} - - ) + const ContextConsumer = this.props.consumer || consumer + + return ( + + {this.renderInner} + + ) } } Connect.WrappedComponent = WrappedComponent Connect.displayName = displayName + Connect.propTypes = { + consumer: PropTypes.object, + forwardRef: PropTypes.oneOfType([ + PropTypes.func, + PropTypes.object + ]) + } // TODO We're losing the ability to add a store as a prop. Not sure there's anything we can do about that. - // TODO With connect no longer managing subscriptions, I _think_ is is all unneeded - /* - if (process.env.NODE_ENV !== 'production') { - Connect.prototype.componentWillUpdate = function componentWillUpdate() { - // We are hot reloading! - if (this.version !== version) { - this.version = version - this.initSelector() - - // If any connected descendants don't hot reload (and resubscribe in the process), their - // listeners will be lost when we unsubscribe. Unfortunately, by copying over all - // listeners, this does mean that the old versions of connected descendants will still be - // notified of state changes; however, their onStateChange function is a no-op so this - // isn't a huge deal. - let oldListeners = []; - - if (this.subscription) { - oldListeners = this.subscription.listeners.get() - this.subscription.tryUnsubscribe() - } - this.initSubscription() - if (shouldHandleStateChanges) { - this.subscription.trySubscribe() - oldListeners.forEach(listener => this.subscription.listeners.subscribe(listener)) - } - } - } - } - */ - return hoistStatics(Connect, WrappedComponent) + + + const forwarded = React.forwardRef(function (props, ref) { + return + }) + + forwarded.displayName = Connect.displayName + forwarded.WrappedComponent = WrappedComponent + return hoistStatics(forwarded, WrappedComponent) } } From 1fe01d0d7e538ff7251facb7e78df2786db83096 Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Sat, 18 Aug 2018 17:55:02 -0600 Subject: [PATCH 09/18] Borrow a bunch of tests from Greg's 1000 branch --- test/components/Provider.spec.js | 117 ++++++++++++++----------------- test/components/connect.spec.js | 29 ++++---- 2 files changed, 66 insertions(+), 80 deletions(-) diff --git a/test/components/Provider.spec.js b/test/components/Provider.spec.js index c43a61914..06bc5eeb0 100644 --- a/test/components/Provider.spec.js +++ b/test/components/Provider.spec.js @@ -19,14 +19,11 @@ describe('React', () => { class Child extends Component { render() { return ( -
{({storeState}) => { - return `${storeKey} - ${storeState}` + return
{`${storeKey} - ${storeState}`}
}}
- -
) } } @@ -35,53 +32,34 @@ describe('React', () => { } const Child = createChild(); - it('should enforce a single child', () => { + it('should not enforce a single child', () => { const store = createStore(() => ({})) - // Ignore propTypes warnings + // Ignore propTypes warning data- const propTypes = Provider.propTypes Provider.propTypes = {} const spy = jest.spyOn(console, 'error').mockImplementation(() => {}) - try { - expect(() => rtl.render( - -
- - )).not.toThrow() + expect(() => rtl.render( + +
+ + )).not.toThrow() - if (semver.lt(React.version, '15.0.0')) { - expect(() => rtl.render( - - - )).toThrow(/children with exactly one child/) - } else { - expect(() => rtl.render( - - - )).toThrow(/a single React element child/) - } + expect(() => rtl.render( + + + )).not.toThrow(/children with exactly one child/) - if (semver.lt(React.version, '15.0.0')) { - expect(() => rtl.render( - -
-
- - )).toThrow(/children with exactly one child/) - } else { - expect(() => rtl.render( - -
-
- - )).toThrow(/a single React element child/) - } - } finally { - Provider.propTypes = propTypes - spy.mockRestore() - } + expect(() => rtl.render( + +
+
+ + )).not.toThrow(/a single React element child/) + spy.mockRestore() + Provider.propTypes = propTypes }) it('should add the store state to context', () => { @@ -100,10 +78,10 @@ describe('React', () => { }) - it('should warn once when receiving a new store in props', () => { + it('accepts new store in props', () => { const store1 = createStore((state = 10) => state + 1) const store2 = createStore((state = 10) => state * 2) - const store3 = createStore((state = 10) => state * state) + const store3 = createStore((state = 10) => state * state+1) let externalSetState class ProviderContainer extends Component { @@ -112,6 +90,7 @@ describe('React', () => { this.state = { store: store1 } externalSetState = this.setState.bind(this) } + render() { return ( @@ -123,27 +102,24 @@ describe('React', () => { const tester = rtl.render() expect(tester.getByTestId('store')).toHaveTextContent('store - 11') + store1.dispatch({ type: 'hi' }) + expect(tester.getByTestId('store')).toHaveTextContent('store - 12') - let spy = jest.spyOn(console, 'error').mockImplementation(() => {}) externalSetState({ store: store2 }) - - expect(tester.getByTestId('store')).toHaveTextContent('store - 11') - expect(spy).toHaveBeenCalledTimes(1) - expect(spy.mock.calls[0][0]).toBe( - ' does not support changing `store` on the fly. ' + - 'It is most likely that you see this error because you updated to ' + - 'Redux 2.x and React Redux 2.x which no longer hot reload reducers ' + - 'automatically. See https://github.com/reduxjs/react-redux/releases/' + - 'tag/v2.0.0 for the migration instructions.' - ) - spy.mockRestore() - - spy = jest.spyOn(console, 'error').mockImplementation(() => {}) + expect(tester.getByTestId('store')).toHaveTextContent('store - 20') + store1.dispatch({ type: 'hi' }) + expect(tester.getByTestId('store')).toHaveTextContent('store - 20') + store2.dispatch({ type: 'hi' }) + expect(tester.getByTestId('store')).toHaveTextContent('store - 40') + externalSetState({ store: store3 }) - - expect(tester.getByTestId('store')).toHaveTextContent('store - 11') - expect(spy).toHaveBeenCalledTimes(0) - spy.mockRestore() + expect(tester.getByTestId('store')).toHaveTextContent('store - 101') + store1.dispatch({ type: 'hi' }) + expect(tester.getByTestId('store')).toHaveTextContent('store - 101') + store2.dispatch({ type: 'hi' }) + expect(tester.getByTestId('store')).toHaveTextContent('store - 101') + store3.dispatch({ type: 'hi' }) + expect(tester.getByTestId('store')).toHaveTextContent('store - 10202') }) it('should handle subscriptions correctly when there is nested Providers', () => { @@ -182,7 +158,7 @@ describe('React', () => { store.dispatch({ type: 'APPEND', body: 'a' }) let childMapStateInvokes = 0 - @connect(state => ({ state }), null, null, { withRef: true }) + @connect(state => ({ state })) class Container extends Component { emitChange() { store.dispatch({ type: 'APPEND', body: 'b' }) @@ -198,10 +174,11 @@ describe('React', () => { } } + const childCalls = [] @connect((state, parentProps) => { childMapStateInvokes++ + childCalls.push([state, parentProps.parentState]) // The state from parent props should always be consistent with the current state - expect(state).toEqual(parentProps.parentState) return {} }) class ChildContainer extends Component { @@ -221,6 +198,10 @@ describe('React', () => { // The store state stays consistent when setState calls are batched store.dispatch({ type: 'APPEND', body: 'c' }) expect(childMapStateInvokes).toBe(2) + expect(childCalls).toEqual([ + ['a', 'a'], + ['ac', 'ac'] + ]) // setState calls DOM handlers are batched const button = tester.getByText('change') @@ -229,11 +210,17 @@ describe('React', () => { // Provider uses unstable_batchedUpdates() under the hood store.dispatch({ type: 'APPEND', body: 'd' }) + expect(childCalls).toEqual([ + ['a', 'a'], + ['ac', 'ac'], // then store update is processed + ['acb', 'acb'], // then store update is processed + ['acbd', 'acbd'], // then store update is processed + ]) expect(childMapStateInvokes).toBe(4) }) - it.skip('works in without warnings (React 16.3+)', () => { + it('works in without warnings (React 16.3+)', () => { if (!React.StrictMode) { return } diff --git a/test/components/connect.spec.js b/test/components/connect.spec.js index 94263e992..176cb0438 100644 --- a/test/components/connect.spec.js +++ b/test/components/connect.spec.js @@ -284,13 +284,12 @@ describe('React', () => { componentDidMount() { this.bar = 'foo' this.forceUpdate() - this.c.forceUpdate() } render() { return ( - this.c = c} /> + ) } @@ -1350,7 +1349,7 @@ describe('React', () => { expect(tester.getByTestId('scooby')).toHaveTextContent('boo') }) - it('should persist listeners through hot update', () => { + it.skip('should persist listeners through hot update', () => { const ACTION_TYPE = "ACTION" const store = createStore((state = {actions: 0}, action) => { switch (action.type) { @@ -1471,13 +1470,15 @@ describe('React', () => { expect(decorated.foo).toBe('bar') }) - it.skip('should use the store from the props instead of from the context if present', () => { + it('should use a custom context provider and consumer if present', () => { class Container extends Component { render() { return } } + const context = React.createContext(null) + let actualState const expectedState = { foos: {} } @@ -1492,7 +1493,7 @@ describe('React', () => { getState: () => expectedState } - rtl.render() + rtl.render() expect(actualState).toEqual(expectedState) }) @@ -1554,7 +1555,7 @@ describe('React', () => { }) - it.skip('should return the instance of the wrapped component for use in calling child methods', async (done) => { + it('should return the instance of the wrapped component for use in calling child methods', async (done) => { const store = createStore(() => ({})) const someData = { @@ -1571,17 +1572,15 @@ describe('React', () => { } } - const decorator = connect(state => state, null, null, { withRef: true }) + const decorator = connect(state => state) const Decorated = decorator(Container) - let ref + const ref = React.createRef() + class Wrapper extends Component { render() { return ( - { - if (!comp) return - ref = comp.getWrappedInstance() - }}/> + ) } } @@ -1594,7 +1593,7 @@ describe('React', () => { await rtl.waitForElement(() => tester.getByTestId('loaded')) - expect(ref.someInstanceMethod()).toBe(someData) + expect(ref.current.someInstanceMethod()).toBe(someData) done() }) @@ -1717,7 +1716,7 @@ describe('React', () => { store.dispatch({ type: 'APPEND', body: 'a' }) let childMapStateInvokes = 0 - @connect(state => ({ state }), null, null, { withRef: true }) + @connect(state => ({ state })) class Container extends Component { emitChange() { @@ -2292,7 +2291,7 @@ describe('React', () => { }) - it.skip('works in without warnings (React 16.3+)', () => { + it('works in without warnings (React 16.3+)', () => { if (!React.StrictMode) { return } From 0d0360f9e1588490e33816b12f824b5b63470846 Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Thu, 23 Aug 2018 21:10:02 -0400 Subject: [PATCH 10/18] Include `react-is` in build output --- rollup.config.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/rollup.config.js b/rollup.config.js index 0a68ad49c..5e3aa6f43 100644 --- a/rollup.config.js +++ b/rollup.config.js @@ -25,7 +25,11 @@ const config = { replace({ 'process.env.NODE_ENV': JSON.stringify(env) }), - commonjs() + commonjs({ + namedExports: { + 'node_modules/react-is/index.js': ['isValidElementType'], + } + }) ] } From 577efb0b1e054731c76c86420b408c4c10cb7e83 Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Thu, 23 Aug 2018 21:10:15 -0400 Subject: [PATCH 11/18] Try using a functional component instead of forwardRef --- src/components/connectAdvanced.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/components/connectAdvanced.js b/src/components/connectAdvanced.js index 183f95e42..c695b9e68 100644 --- a/src/components/connectAdvanced.js +++ b/src/components/connectAdvanced.js @@ -241,12 +241,19 @@ export default function connectAdvanced( + /* const forwarded = React.forwardRef(function (props, ref) { return }) + */ - forwarded.displayName = Connect.displayName - forwarded.WrappedComponent = WrappedComponent - return hoistStatics(forwarded, WrappedComponent) + + const Forwarded = (props) => + + Forwarded.displayName = Connect.displayName + Forwarded.WrappedComponent = WrappedComponent + return hoistStatics(Forwarded, WrappedComponent); + + //return hoistStatics(Connect, WrappedComponent) } } From 7f72494c526f6e98e6dcd8274ef37f9977ef4b3b Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Thu, 23 Aug 2018 22:55:46 -0400 Subject: [PATCH 12/18] Temporarily undo all connect wrappings --- src/components/connectAdvanced.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/components/connectAdvanced.js b/src/components/connectAdvanced.js index c695b9e68..e391911ba 100644 --- a/src/components/connectAdvanced.js +++ b/src/components/connectAdvanced.js @@ -247,13 +247,14 @@ export default function connectAdvanced( }) */ - +/* const Forwarded = (props) => Forwarded.displayName = Connect.displayName Forwarded.WrappedComponent = WrappedComponent return hoistStatics(Forwarded, WrappedComponent); + */ - //return hoistStatics(Connect, WrappedComponent) + return hoistStatics(Connect, WrappedComponent) } } From 9210282d38ab705ad37ea16880637910ae263e86 Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Wed, 12 Sep 2018 21:37:37 -0400 Subject: [PATCH 13/18] Make forwardRef optional --- src/components/connectAdvanced.js | 33 ++++++++++++++----------------- test/components/connect.spec.js | 7 +++---- 2 files changed, 18 insertions(+), 22 deletions(-) diff --git a/src/components/connectAdvanced.js b/src/components/connectAdvanced.js index e391911ba..d1305ebc0 100644 --- a/src/components/connectAdvanced.js +++ b/src/components/connectAdvanced.js @@ -52,6 +52,9 @@ export default function connectAdvanced( // REMOVED: expose the wrapped component via refs withRef = false, + // use React's forwardRef to expose a ref of the wrapped component + forwardRef = false, + // additional options are passed through to the selectorFactory ...connectOptions } = {} @@ -181,8 +184,8 @@ export default function connectAdvanced( return result } - const {consumer, forwardRef, ...wrapperProps} = props - result = {consumer, forwardRef, wrapperProps} + const {contextConsumer, forwardRef, ...wrapperProps} = props + result = {contextConsumer, forwardRef, wrapperProps} return result } @@ -216,7 +219,7 @@ export default function connectAdvanced( } render() { - const ContextConsumer = this.props.consumer || consumer + const ContextConsumer = this.props.contextConsumer || consumer return ( @@ -229,7 +232,7 @@ export default function connectAdvanced( Connect.WrappedComponent = WrappedComponent Connect.displayName = displayName Connect.propTypes = { - consumer: PropTypes.object, + contextConsumer: PropTypes.object, forwardRef: PropTypes.oneOfType([ PropTypes.func, PropTypes.object @@ -239,22 +242,16 @@ export default function connectAdvanced( // TODO We're losing the ability to add a store as a prop. Not sure there's anything we can do about that. + let wrapperComponent = Connect + if(forwardRef) { + const forwarded = React.forwardRef(function (props, ref) { + return + }) - /* - const forwarded = React.forwardRef(function (props, ref) { - return - }) - */ - -/* - const Forwarded = (props) => - - Forwarded.displayName = Connect.displayName - Forwarded.WrappedComponent = WrappedComponent - return hoistStatics(Forwarded, WrappedComponent); - */ + wrapperComponent = forwarded + } - return hoistStatics(Connect, WrappedComponent) + return hoistStatics(wrapperComponent, WrappedComponent) } } diff --git a/test/components/connect.spec.js b/test/components/connect.spec.js index 176cb0438..13c4bad9c 100644 --- a/test/components/connect.spec.js +++ b/test/components/connect.spec.js @@ -1493,7 +1493,7 @@ describe('React', () => { getState: () => expectedState } - rtl.render() + rtl.render() expect(actualState).toEqual(expectedState) }) @@ -1572,7 +1572,7 @@ describe('React', () => { } } - const decorator = connect(state => state) + const decorator = connect(state => state, null, null, {forwardRef : true}) const Decorated = decorator(Container) const ref = React.createRef() @@ -1580,7 +1580,7 @@ describe('React', () => { class Wrapper extends Component { render() { return ( - + ) } } @@ -1590,7 +1590,6 @@ describe('React', () => { ) - await rtl.waitForElement(() => tester.getByTestId('loaded')) expect(ref.current.someInstanceMethod()).toBe(someData) From b6a267a3f726f6a223b304e806def389cf0bf119 Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Thu, 25 Oct 2018 16:59:19 -0700 Subject: [PATCH 14/18] Update React packages to 16.7.0-alpha --- package-lock.json | 106 +++++++++++++++++++++++++++++++++++----------- package.json | 6 +-- 2 files changed, 85 insertions(+), 27 deletions(-) diff --git a/package-lock.json b/package-lock.json index befdf9122..56792f72e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3069,7 +3069,8 @@ "ansi-regex": { "version": "2.1.1", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "aproba": { "version": "1.1.1", @@ -3136,6 +3137,7 @@ "version": "0.0.9", "bundled": true, "dev": true, + "optional": true, "requires": { "inherits": "~2.0.0" } @@ -3144,6 +3146,7 @@ "version": "2.10.1", "bundled": true, "dev": true, + "optional": true, "requires": { "hoek": "2.x.x" } @@ -3161,7 +3164,8 @@ "buffer-shims": { "version": "1.0.0", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "caseless": { "version": "0.12.0", @@ -3178,7 +3182,8 @@ "code-point-at": { "version": "1.1.0", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "combined-stream": { "version": "1.0.5", @@ -3198,17 +3203,20 @@ "console-control-strings": { "version": "1.1.0", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "core-util-is": { "version": "1.0.2", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "cryptiles": { "version": "2.0.5", "bundled": true, "dev": true, + "optional": true, "requires": { "boom": "2.x.x" } @@ -3281,7 +3289,8 @@ "extsprintf": { "version": "1.0.2", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "forever-agent": { "version": "0.6.1", @@ -3309,6 +3318,7 @@ "version": "1.0.11", "bundled": true, "dev": true, + "optional": true, "requires": { "graceful-fs": "^4.1.2", "inherits": "~2.0.0", @@ -3376,7 +3386,8 @@ "graceful-fs": { "version": "4.1.11", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "har-schema": { "version": "1.0.5", @@ -3404,6 +3415,7 @@ "version": "3.1.3", "bundled": true, "dev": true, + "optional": true, "requires": { "boom": "2.x.x", "cryptiles": "2.x.x", @@ -3451,6 +3463,7 @@ "version": "1.0.0", "bundled": true, "dev": true, + "optional": true, "requires": { "number-is-nan": "^1.0.0" } @@ -3464,7 +3477,8 @@ "isarray": { "version": "1.0.0", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "isstream": { "version": "0.1.2", @@ -3567,6 +3581,7 @@ "version": "0.5.1", "bundled": true, "dev": true, + "optional": true, "requires": { "minimist": "0.0.8" } @@ -3680,7 +3695,8 @@ "process-nextick-args": { "version": "1.0.7", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "punycode": { "version": "1.4.1", @@ -3718,6 +3734,7 @@ "version": "2.2.9", "bundled": true, "dev": true, + "optional": true, "requires": { "buffer-shims": "~1.0.0", "core-util-is": "~1.0.0", @@ -3769,7 +3786,8 @@ "safe-buffer": { "version": "5.0.1", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "semver": { "version": "5.3.0", @@ -3793,6 +3811,7 @@ "version": "1.0.9", "bundled": true, "dev": true, + "optional": true, "requires": { "hoek": "2.x.x" } @@ -3826,6 +3845,7 @@ "version": "1.0.2", "bundled": true, "dev": true, + "optional": true, "requires": { "code-point-at": "^1.0.0", "is-fullwidth-code-point": "^1.0.0", @@ -3836,6 +3856,7 @@ "version": "1.0.1", "bundled": true, "dev": true, + "optional": true, "requires": { "safe-buffer": "^5.0.1" } @@ -3850,6 +3871,7 @@ "version": "3.0.1", "bundled": true, "dev": true, + "optional": true, "requires": { "ansi-regex": "^2.0.0" } @@ -3864,6 +3886,7 @@ "version": "2.2.1", "bundled": true, "dev": true, + "optional": true, "requires": { "block-stream": "*", "fstream": "^1.0.2", @@ -3919,7 +3942,8 @@ "util-deprecate": { "version": "1.0.2", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "uuid": { "version": "3.0.1", @@ -6830,33 +6854,57 @@ } }, "react": { - "version": "16.4.2", - "resolved": "https://registry.npmjs.org/react/-/react-16.4.2.tgz", - "integrity": "sha512-dMv7YrbxO4y2aqnvA7f/ik9ibeLSHQJTI6TrYAenPSaQ6OXfb+Oti+oJiy8WBxgRzlKatYqtCjphTgDSCEiWFg==", + "version": "16.7.0-alpha.0", + "resolved": "https://registry.npmjs.org/react/-/react-16.7.0-alpha.0.tgz", + "integrity": "sha512-V0za4H01aoAF0SdzahHepvfvzTQ1xxkgMX4z8uKzn+wzZAlVk0IVpleqyxZWluqmdftNedj6fIIZRO/rVYVFvQ==", "dev": true, "requires": { - "fbjs": "^0.8.16", "loose-envify": "^1.1.0", "object-assign": "^4.1.1", - "prop-types": "^15.6.0" + "prop-types": "^15.6.2", + "scheduler": "^0.11.0-alpha.0" + }, + "dependencies": { + "prop-types": { + "version": "15.6.2", + "resolved": "https://registry.npmjs.org/prop-types/-/prop-types-15.6.2.tgz", + "integrity": "sha512-3pboPvLiWD7dkI3qf3KbUe6hKFKa52w+AE0VCqECtf+QHAKgOL37tTaNCnuX1nAAQ4ZhyP+kYVKf8rLmJ/feDQ==", + "dev": true, + "requires": { + "loose-envify": "^1.3.1", + "object-assign": "^4.1.1" + } + } } }, "react-dom": { - "version": "16.4.2", - "resolved": "https://registry.npmjs.org/react-dom/-/react-dom-16.4.2.tgz", - "integrity": "sha512-Usl73nQqzvmJN+89r97zmeUpQDKDlh58eX6Hbs/ERdDHzeBzWy+ENk7fsGQ+5KxArV1iOFPT46/VneklK9zoWw==", + "version": "16.7.0-alpha.0", + "resolved": "https://registry.npmjs.org/react-dom/-/react-dom-16.7.0-alpha.0.tgz", + "integrity": "sha512-/XUn1ldxmoV2B7ov0rWT5LMZaaHMlF9GGLkUsmPRxmWTJwRDOuAPXidSaSlmR/VOhDSI1s+v3+KzFqhhDFJxYA==", "dev": true, "requires": { - "fbjs": "^0.8.16", "loose-envify": "^1.1.0", "object-assign": "^4.1.1", - "prop-types": "^15.6.0" + "prop-types": "^15.6.2", + "scheduler": "^0.11.0-alpha.0" + }, + "dependencies": { + "prop-types": { + "version": "15.6.2", + "resolved": "https://registry.npmjs.org/prop-types/-/prop-types-15.6.2.tgz", + "integrity": "sha512-3pboPvLiWD7dkI3qf3KbUe6hKFKa52w+AE0VCqECtf+QHAKgOL37tTaNCnuX1nAAQ4ZhyP+kYVKf8rLmJ/feDQ==", + "dev": true, + "requires": { + "loose-envify": "^1.3.1", + "object-assign": "^4.1.1" + } + } } }, "react-is": { - "version": "16.4.2", - "resolved": "https://registry.npmjs.org/react-is/-/react-is-16.4.2.tgz", - "integrity": "sha512-rI3cGFj/obHbBz156PvErrS5xc6f1eWyTwyV4mo0vF2lGgXgS+mm7EKD5buLJq6jNgIagQescGSVG2YzgXt8Yg==" + "version": "16.7.0-alpha.0", + "resolved": "https://registry.npmjs.org/react-is/-/react-is-16.7.0-alpha.0.tgz", + "integrity": "sha512-8jBaW1rcnUoZFJ4wB5W6xgnNfr70pIW6DadourJ6zkgXZVVcACbUkZyfjI/zI84uyopCJQ6VFHY6qGh6wfJ59Q==" }, "react-lifecycles-compat": { "version": "3.0.4", @@ -8261,6 +8309,16 @@ "integrity": "sha512-NqVDv9TpANUjFm0N8uM5GxL36UgKi9/atZw+x7YFnQ8ckwFGKrl4xX4yWtrey3UJm5nP1kUbnYgLopqWNSRhWw==", "dev": true }, + "scheduler": { + "version": "0.11.0-alpha.0", + "resolved": "https://registry.npmjs.org/scheduler/-/scheduler-0.11.0-alpha.0.tgz", + "integrity": "sha512-0tUDHYSyni/EHkMMBysVXVwfanCWWbLsulnDB1tGrQiWWrVuRVoclWCPHCYC/1iR5Rj34EQhxh3/F36V+F+ZpA==", + "dev": true, + "requires": { + "loose-envify": "^1.1.0", + "object-assign": "^4.1.1" + } + }, "semver": { "version": "5.5.0", "resolved": "https://registry.npmjs.org/semver/-/semver-5.5.0.tgz", diff --git a/package.json b/package.json index af4ae7a79..ca60954c7 100644 --- a/package.json +++ b/package.json @@ -47,7 +47,7 @@ "invariant": "^2.2.4", "loose-envify": "^1.1.0", "prop-types": "^15.6.1", - "react-is": "^16.4.2", + "react-is": "^16.7.0-alpha.0", "react-lifecycles-compat": "^3.0.0" }, "devDependencies": { @@ -90,8 +90,8 @@ "jest": "^23.4.1", "jest-dom": "^1.12.0", "npm-run": "^5.0.1", - "react": "^16.4.2", - "react-dom": "^16.4.2", + "react": "^16.7.0-alpha.0", + "react-dom": "^16.7.0-alpha.0", "react-testing-library": "^5.0.0", "redux": "^4.0.0", "rimraf": "^2.6.2", From ef8aa4e4712983e58a50b0fd637966427712be17 Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Thu, 25 Oct 2018 17:02:25 -0700 Subject: [PATCH 15/18] Remove unneeded react-lifecycles-compat dep --- package-lock.json | 18 ++++++++++-------- package.json | 3 +-- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/package-lock.json b/package-lock.json index 56792f72e..80cd61f28 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3312,7 +3312,8 @@ "fs.realpath": { "version": "1.0.0", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "fstream": { "version": "1.0.11", @@ -3374,6 +3375,7 @@ "version": "7.1.2", "bundled": true, "dev": true, + "optional": true, "requires": { "fs.realpath": "^1.0.0", "inflight": "^1.0.4", @@ -3426,7 +3428,8 @@ "hoek": { "version": "2.16.3", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "http-signature": { "version": "1.1.1", @@ -3443,6 +3446,7 @@ "version": "1.0.6", "bundled": true, "dev": true, + "optional": true, "requires": { "once": "^1.3.0", "wrappy": "1" @@ -3567,6 +3571,7 @@ "version": "3.0.4", "bundled": true, "dev": true, + "optional": true, "requires": { "brace-expansion": "^1.1.7" } @@ -3684,7 +3689,8 @@ "path-is-absolute": { "version": "1.0.1", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "performance-now": { "version": "0.2.0", @@ -3779,6 +3785,7 @@ "version": "2.6.1", "bundled": true, "dev": true, + "optional": true, "requires": { "glob": "^7.0.5" } @@ -6906,11 +6913,6 @@ "resolved": "https://registry.npmjs.org/react-is/-/react-is-16.7.0-alpha.0.tgz", "integrity": "sha512-8jBaW1rcnUoZFJ4wB5W6xgnNfr70pIW6DadourJ6zkgXZVVcACbUkZyfjI/zI84uyopCJQ6VFHY6qGh6wfJ59Q==" }, - "react-lifecycles-compat": { - "version": "3.0.4", - "resolved": "https://registry.npmjs.org/react-lifecycles-compat/-/react-lifecycles-compat-3.0.4.tgz", - "integrity": "sha512-fBASbA6LnOU9dOU2eW7aQ8xmYBSXUIWr+UmF9b1efZBazGNO+rcXT/icdKnYm2pTwcRylVUYwW7H1PHfLekVzA==" - }, "react-testing-library": { "version": "5.0.0", "resolved": "https://registry.npmjs.org/react-testing-library/-/react-testing-library-5.0.0.tgz", diff --git a/package.json b/package.json index ca60954c7..487336e56 100644 --- a/package.json +++ b/package.json @@ -47,8 +47,7 @@ "invariant": "^2.2.4", "loose-envify": "^1.1.0", "prop-types": "^15.6.1", - "react-is": "^16.7.0-alpha.0", - "react-lifecycles-compat": "^3.0.0" + "react-is": "^16.7.0-alpha.0" }, "devDependencies": { "babel-cli": "^6.26.0", From 3c0428b1bfe06169b7e3e0cdf9d5dfda5e78e05b Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Thu, 25 Oct 2018 17:04:06 -0700 Subject: [PATCH 16/18] Update hoist-non-react-statics --- package-lock.json | 20 ++++++++++++++++---- package.json | 2 +- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index 80cd61f28..82cf16992 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3455,7 +3455,8 @@ "inherits": { "version": "2.0.3", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "ini": { "version": "1.3.4", @@ -3660,6 +3661,7 @@ "version": "1.4.0", "bundled": true, "dev": true, + "optional": true, "requires": { "wrappy": "1" } @@ -4225,9 +4227,19 @@ } }, "hoist-non-react-statics": { - "version": "2.5.5", - "resolved": "https://registry.npmjs.org/hoist-non-react-statics/-/hoist-non-react-statics-2.5.5.tgz", - "integrity": "sha512-rqcy4pJo55FTTLWt+bU8ukscqHeE/e9KWvsOW2b/a3afxQZhwkQdT1rPPCJ0rYXdj4vNcasY8zHTH+jF/qStxw==" + "version": "3.0.1", + "resolved": "https://registry.npmjs.org/hoist-non-react-statics/-/hoist-non-react-statics-3.0.1.tgz", + "integrity": "sha512-1kXwPsOi0OGQIZNVMPvgWJ9tSnGMiMfJdihqEzrPEXlHOBh9AAHXX/QYmAJTXztnz/K+PQ8ryCb4eGaN6HlGbQ==", + "requires": { + "react-is": "^16.3.2" + }, + "dependencies": { + "react-is": { + "version": "16.6.0", + "resolved": "https://registry.npmjs.org/react-is/-/react-is-16.6.0.tgz", + "integrity": "sha512-q8U7k0Fi7oxF1HvQgyBjPwDXeMplEsArnKt2iYhuIF86+GBbgLHdAmokL3XUFjTd7Q363OSNG55FOGUdONVn1g==" + } + } }, "home-or-tmp": { "version": "2.0.0", diff --git a/package.json b/package.json index 487336e56..fd87cc132 100644 --- a/package.json +++ b/package.json @@ -43,7 +43,7 @@ "redux": "^2.0.0 || ^3.0.0 || ^4.0.0-0" }, "dependencies": { - "hoist-non-react-statics": "^2.5.5", + "hoist-non-react-statics": "^3.0.1", "invariant": "^2.2.4", "loose-envify": "^1.1.0", "prop-types": "^15.6.1", From 6b8df35a25919c1f31c61c0586dbfe0bfa48fe66 Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Thu, 25 Oct 2018 17:05:17 -0700 Subject: [PATCH 17/18] Update React and Redux peer deps --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index fd87cc132..8075213cb 100644 --- a/package.json +++ b/package.json @@ -39,8 +39,8 @@ "coverage": "codecov" }, "peerDependencies": { - "react": "^0.14.0 || ^15.0.0-0 || ^16.0.0-0", - "redux": "^2.0.0 || ^3.0.0 || ^4.0.0-0" + "react": "^16.7.0-alpha.0", + "redux": "^3.0.0 || ^4.0.0-0" }, "dependencies": { "hoist-non-react-statics": "^3.0.1", From d4ab1d8326c4c596d2c9ea8cd699416ce989aad6 Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Thu, 25 Oct 2018 17:20:14 -0700 Subject: [PATCH 18/18] Rewrite connectAdvanced to use hooks internally Replaced multiple levels of class wrappers with a simple hook that reads the store state from context, initializes and uses the provided selector, and memoizes the wrapped component as needed. Updated Provider and connect to accept an entire context object as a prop, because `useContext` only works with a context object and not just the consumer. Also allows passing a new store instance to Provider, because we can easily reset the subscription aspect now. Updated tests. --- src/components/Provider.js | 27 +---- src/components/connectAdvanced.js | 168 +++++------------------------- test/components/Provider.spec.js | 1 - test/components/connect.spec.js | 10 +- 4 files changed, 35 insertions(+), 171 deletions(-) diff --git a/src/components/Provider.js b/src/components/Provider.js index 9268186f2..8906e1599 100644 --- a/src/components/Provider.js +++ b/src/components/Provider.js @@ -1,26 +1,9 @@ -import React, { Component, Children } from 'react' +import React, { Component } from 'react' import PropTypes from 'prop-types' import { storeShape } from '../utils/PropTypes' -import warning from '../utils/warning' import {ReactReduxContext} from "./context" -let didWarnAboutReceivingStore = false -function warnAboutReceivingStore() { - if (didWarnAboutReceivingStore) { - return - } - didWarnAboutReceivingStore = true - - warning( - ' does not support changing `store` on the fly. ' + - 'It is most likely that you see this error because you updated to ' + - 'Redux 2.x and React Redux 2.x which no longer hot reload reducers ' + - 'automatically. See https://github.com/reduxjs/react-redux/releases/' + - 'tag/v2.0.0 for the migration instructions.' - ) -} - export function createProvider() { class Provider extends Component { @@ -83,12 +66,12 @@ export function createProvider() { } render() { - const ContextProvider = this.props.contextProvider || ReactReduxContext.Provider + const Context = this.props.context || ReactReduxContext return ( - + {this.props.children} - + ) } } @@ -97,7 +80,7 @@ export function createProvider() { Provider.propTypes = { store: storeShape.isRequired, children: PropTypes.element.isRequired, - contextProvider : PropTypes.object, + context : PropTypes.object, } return Provider diff --git a/src/components/connectAdvanced.js b/src/components/connectAdvanced.js index d1305ebc0..caebc2e84 100644 --- a/src/components/connectAdvanced.js +++ b/src/components/connectAdvanced.js @@ -1,13 +1,10 @@ import hoistStatics from 'hoist-non-react-statics' import invariant from 'invariant' import PropTypes from 'prop-types' -import React, { Component, PureComponent } from 'react' +import React, { useContext, useMemo} from 'react' import { isValidElementType } from 'react-is' import {ReactReduxContext} from "./context" -import {storeShape} from "../utils/PropTypes" - -let hotReloadingVersion = 0 export default function connectAdvanced( @@ -43,8 +40,8 @@ export default function connectAdvanced( shouldHandleStateChanges = true, - // the context consumer to use - consumer = ReactReduxContext.Consumer, + // the context to use + context : defaultContext = ReactReduxContext, // REMOVED: the key of props/context to get the store storeKey = 'store', @@ -65,15 +62,12 @@ export default function connectAdvanced( invariant(storeKey === 'store', 'storeKey has been removed. To use a custom redux store for a single component, ' + - 'create a custom React context with React.createContext() and pass the Provider to react-redux\'s provider ' + - 'and the Consumer to this component as in <' + - 'ConnectedComponent consumer={context.Consumer} />' + 'create a custom React context with React.createContext() and pass it to react-redux\'s Provider ' + + 'and this component as in <' + + 'ConnectedComponent context={MyCustomContext} />' ) - const version = hotReloadingVersion++ - - return function wrapWithConnect(WrappedComponent) { invariant( isValidElementType(WrappedComponent), @@ -98,160 +92,48 @@ export default function connectAdvanced( } - const OuterBaseComponent = connectOptions.pure ? PureComponent : Component - function createChildSelector(store) { return selectorFactory(store.dispatch, selectorFactoryOptions) } - class ConnectInner extends Component { - constructor(props) { - super(props) - - this.state = { - wrapperProps : props.wrapperProps, - store : props.store, - error : null, - childPropsSelector : createChildSelector(props.store), - childProps : {}, - } - - this.state = { - ...this.state, - ...ConnectInner.getChildPropsState(props, this.state) - } - } - - - - static getChildPropsState(props, state) { - try { - let {childPropsSelector} = state - - if(props.store !== state.store) { - childPropsSelector = createChildSelector(props.store) - } - - const nextProps = childPropsSelector(props.storeState, props.wrapperProps) - if (nextProps === state.childProps) return null - - return { childProps: nextProps, store : props.store, childPropsSelector } - } catch (error) { - return { error } - } - } - - static getDerivedStateFromProps(props, state) { - const nextChildProps = ConnectInner.getChildPropsState(props, state) - - if(nextChildProps === null) { - return null - } - - return { - ...nextChildProps, - wrapperProps : props.wrapperProps, - } - } - - shouldComponentUpdate(nextProps, nextState) { - const childPropsChanged = nextState.childProps !== this.state.childProps - const hasError = !!nextState.error - - return childPropsChanged || hasError - } - - render() { - if(this.state.error) { - throw this.state.error - } - - return - } - } - - ConnectInner.propTypes = { - wrapperProps : PropTypes.object, - store : storeShape, - } - - - function createWrapperPropsMemoizer() { - let result, prevProps - - return function wrapperPropsMemoizer(props) { - if(props === prevProps) { - return result - } - - const {contextConsumer, forwardRef, ...wrapperProps} = props - result = {contextConsumer, forwardRef, wrapperProps} - - return result - } - } - - class Connect extends OuterBaseComponent { - constructor(props) { - super(props) - - this.version = version - - this.renderInner = this.renderInner.bind(this) - - this.wrapperPropsMemoizer = createWrapperPropsMemoizer() - } - renderInner(providerValue) { - const {storeState, store} = providerValue + function ConnectFunction(props) { + const {context, forwardRef, ...wrapperProps} = props + const contextToRead = context || defaultContext - const {forwardRef, wrapperProps} = this.wrapperPropsMemoizer(this.props) + const {store, storeState} = useContext(contextToRead) + const childPropsSelector = useMemo(() => createChildSelector(store), [store]) - return ( - - ) - } + const childProps = childPropsSelector(storeState, wrapperProps) - render() { - const ContextConsumer = this.props.contextConsumer || consumer + const renderedChild = useMemo(() => { + return + }, [childProps, forwardRef]) - return ( - - {this.renderInner} - - ) - } + return renderedChild } - Connect.WrappedComponent = WrappedComponent - Connect.displayName = displayName - Connect.propTypes = { - contextConsumer: PropTypes.object, - forwardRef: PropTypes.oneOfType([ - PropTypes.func, - PropTypes.object - ]) - } // TODO We're losing the ability to add a store as a prop. Not sure there's anything we can do about that. - let wrapperComponent = Connect + let wrapperComponent = ConnectFunction if(forwardRef) { const forwarded = React.forwardRef(function (props, ref) { - return + return }) wrapperComponent = forwarded } + wrapperComponent.WrappedComponent = WrappedComponent + wrapperComponent.displayName = displayName + wrapperComponent.propTypes = { + context: PropTypes.object, + } + + return hoistStatics(wrapperComponent, WrappedComponent) } } diff --git a/test/components/Provider.spec.js b/test/components/Provider.spec.js index 06bc5eeb0..ebde26741 100644 --- a/test/components/Provider.spec.js +++ b/test/components/Provider.spec.js @@ -1,7 +1,6 @@ /*eslint-disable react/prop-types*/ import React, { Component } from 'react' -import semver from 'semver' import { createStore } from 'redux' import { Provider, connect } from '../../src/index.js' import {ReactReduxContext} from "../../src/components/context" diff --git a/test/components/connect.spec.js b/test/components/connect.spec.js index 13c4bad9c..a7de5aa21 100644 --- a/test/components/connect.spec.js +++ b/test/components/connect.spec.js @@ -1493,7 +1493,7 @@ describe('React', () => { getState: () => expectedState } - rtl.render() + rtl.render() expect(actualState).toEqual(expectedState) }) @@ -1696,16 +1696,16 @@ describe('React', () => { ) - expect(mapStateSpy).toHaveBeenCalledTimes(2) - expect(mapDispatchSpy).toHaveBeenCalledTimes(2) + expect(mapStateSpy).toHaveBeenCalledTimes(1) + expect(mapDispatchSpy).toHaveBeenCalledTimes(1) expect(tester.getByTestId('statefulValue')).toHaveTextContent('foo') // Impure update storeGetter.storeKey = 'bar' externalSetState({ storeGetter }) - expect(mapStateSpy).toHaveBeenCalledTimes(3) - expect(mapDispatchSpy).toHaveBeenCalledTimes(3) + expect(mapStateSpy).toHaveBeenCalledTimes(2) + expect(mapDispatchSpy).toHaveBeenCalledTimes(2) expect(tester.getByTestId('statefulValue')).toHaveTextContent('bar') })