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": { 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'], + } + }) ] } diff --git a/src/components/Provider.js b/src/components/Provider.js index f78e25e65..9268186f2 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,39 +21,83 @@ function warnAboutReceivingStore() { ) } -export function createProvider(storeKey = 'store') { - const subscriptionKey = `${storeKey}Subscription` +export function createProvider() { class Provider extends Component { - getChildContext() { - return { [storeKey]: this[storeKey], [subscriptionKey]: null } + + constructor(props) { + super(props) + + const {store} = props + + this.state = { + storeState : store.getState(), + store, + } } - constructor(props, context) { - super(props, context) - this[storeKey] = props.store; + componentDidMount() { + this._isMounted = true + this.subscribe() } - render() { - return Children.only(this.props.children) + componentWillUnmount() { + if(this.unsubscribe) this.unsubscribe() + + this._isMounted = false } - } - if (process.env.NODE_ENV !== 'production') { - Provider.prototype.componentWillReceiveProps = function (nextProps) { - if (this[storeKey] !== nextProps.store) { - warnAboutReceivingStore() + componentDidUpdate(prevProps) { + if(this.props.store !== prevProps.store) { + if(this.unsubscribe) this.unsubscribe() + + this.subscribe() + } + } + + subscribe() { + const {store} = this.props + + 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) { + return null + } + + return {storeState : newStoreState} + }) + }) + + // 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() { + const ContextProvider = this.props.contextProvider || ReactReduxContext.Provider + + return ( + + {this.props.children} + + ) } - } } + Provider.propTypes = { - store: storeShape.isRequired, - children: PropTypes.element.isRequired, - } - Provider.childContextTypes = { - [storeKey]: storeShape.isRequired, - [subscriptionKey]: subscriptionShape, + 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 46caa481d..d1305ebc0 100644 --- a/src/components/connectAdvanced.js +++ b/src/components/connectAdvanced.js @@ -1,33 +1,14 @@ import hoistStatics from 'hoist-non-react-statics' import invariant from 'invariant' -import { Component, createElement } from 'react' +import PropTypes from 'prop-types' +import React, { Component, PureComponent } from 'react' +import { isValidElementType } from 'react-is' -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) { - // wrap the selector in an object that tracks its results between runs. - const selector = { - run: function runComponentSelector(props) { - try { - const nextProps = sourceSelector(store.getState(), 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( /* @@ -58,37 +39,44 @@ 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, - // the key of props/context to get the store + + // the context consumer to use + consumer = ReactReduxContext.Consumer, + + // REMOVED: 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. + // 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 } = {} ) { - const subscriptionKey = storeKey + 'Subscription' + 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++ - const contextTypes = { - [storeKey]: storeShape, - [subscriptionKey]: subscriptionShape, - } - const childContextTypes = { - [subscriptionKey]: subscriptionShape, - } 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)}` ) @@ -103,199 +91,167 @@ export default function connectAdvanced( ...connectOptions, getDisplayName, methodName, - renderCountProp, shouldHandleStateChanges, - storeKey, - withRef, displayName, wrappedComponentName, WrappedComponent } - // TODO Actually fix our use of componentWillReceiveProps - /* eslint-disable react/no-deprecated */ - class Connect extends Component { - constructor(props, context) { - super(props, context) + const OuterBaseComponent = connectOptions.pure ? PureComponent : Component - this.version = version - this.state = {} - this.renderCount = 0 - this.store = props[storeKey] || context[storeKey] - this.propsMode = Boolean(props[storeKey]) - this.setWrappedInstance = this.setWrappedInstance.bind(this) - - 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}".` - ) + function createChildSelector(store) { + return selectorFactory(store.dispatch, selectorFactoryOptions) + } - this.initSelector() - this.initSubscription() - } + class ConnectInner extends Component { + constructor(props) { + super(props) - 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] } - } + this.state = { + wrapperProps : props.wrapperProps, + store : props.store, + error : null, + childPropsSelector : createChildSelector(props.store), + childProps : {}, + } - 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.subscription.trySubscribe() - this.selector.run(this.props) - if (this.selector.shouldComponentUpdate) this.forceUpdate() + this.state = { + ...this.state, + ...ConnectInner.getChildPropsState(props, this.state) + } } - componentWillReceiveProps(nextProps) { - this.selector.run(nextProps) - } - 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 - } + static getChildPropsState(props, state) { + try { + let {childPropsSelector} = state - 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 + 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 } + } } - setWrappedInstance(ref) { - this.wrappedInstance = ref + static getDerivedStateFromProps(props, state) { + const nextChildProps = ConnectInner.getChildPropsState(props, state) + + if(nextChildProps === null) { + return null + } + + return { + ...nextChildProps, + wrapperProps : props.wrapperProps, + } } - initSelector() { - const sourceSelector = selectorFactory(this.store.dispatch, selectorFactoryOptions) - this.selector = makeSelectorStateful(sourceSelector, this.store) - this.selector.run(this.props) + shouldComponentUpdate(nextProps, nextState) { + const childPropsChanged = nextState.childProps !== this.state.childProps + const hasError = !!nextState.error + + return childPropsChanged || hasError } - 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) + render() { + if(this.state.error) { + throw this.state.error + } + + return } + } + + ConnectInner.propTypes = { + wrapperProps : PropTypes.object, + store : storeShape, + } - onStateChange() { - this.selector.run(this.props) - if (!this.selector.shouldComponentUpdate) { - this.notifyNestedSubs() - } else { - this.componentDidUpdate = this.notifyNestedSubsOnComponentDidUpdate - this.setState(dummyState) + function createWrapperPropsMemoizer() { + let result, prevProps + + return function wrapperPropsMemoizer(props) { + if(props === prevProps) { + return result } - } - 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() + 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) - isSubscribed() { - return Boolean(this.subscription) && this.subscription.isSubscribed() + this.wrapperPropsMemoizer = createWrapperPropsMemoizer() } - addExtraProps(props) { - if (!withRef && !renderCountProp && !(this.propsMode && this.subscription)) 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++ - if (this.propsMode && this.subscription) withExtras[subscriptionKey] = this.subscription - return withExtras + renderInner(providerValue) { + const {storeState, store} = providerValue + + const {forwardRef, wrapperProps} = this.wrapperPropsMemoizer(this.props) + + return ( + + ) } render() { - const selector = this.selector - selector.shouldComponentUpdate = false + const ContextConsumer = this.props.contextConsumer || consumer - if (selector.error) { - throw selector.error - } else { - return createElement(WrappedComponent, this.addExtraProps(selector.props)) - } + return ( + + {this.renderInner} + + ) } } - /* eslint-enable react/no-deprecated */ - Connect.WrappedComponent = WrappedComponent Connect.displayName = displayName - Connect.childContextTypes = childContextTypes - Connect.contextTypes = contextTypes - Connect.propTypes = contextTypes - - 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)) - } - } - } + 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 + + if(forwardRef) { + const forwarded = React.forwardRef(function (props, ref) { + return + }) + + wrapperComponent = forwarded } - return hoistStatics(Connect, WrappedComponent) + return hoistStatics(wrapperComponent, 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 - } - } -} diff --git a/test/components/Provider.spec.js b/test/components/Provider.spec.js index 9ada2ac0d..06bc5eeb0 100644 --- a/test/components/Provider.spec.js +++ b/test/components/Provider.spec.js @@ -1,96 +1,68 @@ /*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' +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(); - 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 to the child context', () => { + it('should add the store state to context', () => { const store = createStore(createExampleTextReducer()) const spy = jest.spyOn(console, 'error').mockImplementation(() => {}) @@ -105,27 +77,11 @@ 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', () => { + 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 { @@ -134,6 +90,7 @@ describe('React', () => { this.state = { store: store1 } externalSetState = this.setState.bind(this) } + render() { return ( @@ -145,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', () => { @@ -190,87 +144,129 @@ 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 + } - store.dispatch({ type: 'APPEND', body: 'a' }) - let childMapStateInvokes = 0 + const store = createStore(stringBuilder) - @connect(state => ({ state }), null, null, { withRef: true }) - class Container extends Component { - emitChange() { - store.dispatch({ type: 'APPEND', body: 'b' }) - } + store.dispatch({ type: 'APPEND', body: 'a' }) + let childMapStateInvokes = 0 - render() { - return ( -
- - -
- ) + @connect(state => ({ state })) + class Container extends Component { + emitChange() { + store.dispatch({ type: 'APPEND', body: 'b' }) + } + + 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 childCalls = [] + @connect((state, parentProps) => { + childMapStateInvokes++ + childCalls.push([state, parentProps.parentState]) + // The state from parent props should always be consistent with the current state + 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) + expect(childCalls).toEqual([ + ['a', 'a'], + ['ac', 'ac'] + ]) + + // 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(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) + }) - // 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) + it('works in without warnings (React 16.3+)', () => { + if (!React.StrictMode) { + return + } + const spy = jest.spyOn(console, 'error').mockImplementation(() => {}) + const store = createStore(() => ({})) - // Provider uses unstable_batchedUpdates() under the hood - store.dispatch({ type: 'APPEND', body: 'd' }) - expect(childMapStateInvokes).toBe(4) - }) + rtl.render( + + +
+ + + ) + expect(spy).not.toHaveBeenCalled() + }) - 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( - + 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..13c4bad9c 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 } from '../../src/index.js' +import { 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) @@ -293,13 +284,12 @@ describe('React', () => { componentDidMount() { this.bar = 'foo' this.forceUpdate() - this.c.forceUpdate() } render() { return ( - this.c = c} /> + ) } @@ -846,42 +836,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 +991,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 +1292,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( @@ -1395,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) { @@ -1516,13 +1470,15 @@ describe('React', () => { expect(decorated.foo).toBe('bar') }) - it('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: {} } @@ -1537,12 +1493,12 @@ describe('React', () => { getState: () => expectedState } - rtl.render() + rtl.render() 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 +1519,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 { @@ -1616,17 +1572,15 @@ describe('React', () => { } } - const decorator = connect(state => state, null, null, { withRef: true }) + const decorator = connect(state => state, null, null, {forwardRef : true}) const Decorated = decorator(Container) - let ref + const ref = React.createRef() + class Wrapper extends Component { render() { return ( - { - if (!comp) return - ref = comp.getWrappedInstance() - }}/> + ) } } @@ -1636,10 +1590,9 @@ describe('React', () => { ) - await rtl.waitForElement(() => tester.getByTestId('loaded')) - expect(ref.someInstanceMethod()).toBe(someData) + expect(ref.current.someInstanceMethod()).toBe(someData) done() }) @@ -1762,7 +1715,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() { @@ -1872,24 +1825,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 +2246,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)) @@ -2343,7 +2290,7 @@ describe('React', () => { }) - it.skip('works in without warnings (React 16.3+)', () => { + it('works in without warnings (React 16.3+)', () => { if (!React.StrictMode) { return } @@ -2368,25 +2315,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]') - }) }) })