Skip to content

merge in master #5

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 14, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
615 changes: 223 additions & 392 deletions package-lock.json

Large diffs are not rendered by default.

11 changes: 5 additions & 6 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
"build:umd:min": "cross-env BABEL_ENV=rollup NODE_ENV=production rollup -c -o dist/react-redux.min.js",
"build": "npm run build:commonjs && npm run build:es && npm run build:umd && npm run build:umd:min",
"clean": "rimraf lib dist es coverage",
"lint": "eslint src test/utils test/components test/getTestDeps.js",
"lint": "eslint src test/utils test/components",
"prepare": "npm run clean && npm run build",
"test": "node ./test/run-tests.js",
"coverage": "codecov"
Expand Down Expand Up @@ -81,18 +81,17 @@
"create-react-class": "^15.6.3",
"cross-env": "^5.2.0",
"cross-spawn": "^6.0.5",
"enzyme": "^3.3.0",
"enzyme-adapter-react-16": "^1.1.1",
"es3ify": "^0.2.0",
"eslint": "^4.19.1",
"eslint-plugin-import": "^2.12.0",
"eslint-plugin-react": "^7.9.1",
"glob": "^7.1.1",
"jest": "^23.4.1",
"jest-dom": "^1.12.0",
"npm-run": "^5.0.1",
"react": "^16.3.2",
"react-dom": "^16.3.2",
"react-test-renderer": "^16.3.2",
"react": "^16.4.2",
"react-dom": "^16.4.2",
"react-testing-library": "^5.0.0",
"redux": "^4.0.0",
"rimraf": "^2.6.2",
"rollup": "^0.61.1",
Expand Down
4 changes: 2 additions & 2 deletions src/components/Provider.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ export function createProvider(storeKey = 'store') {
}

if (process.env.NODE_ENV !== 'production') {
Provider.prototype.componentDidUpdate = function () {
if (this[storeKey] !== this.props.store) {
Provider.prototype.componentWillReceiveProps = function (nextProps) {
if (this[storeKey] !== nextProps.store) {
warnAboutReceivingStore()
}
}
Expand Down
113 changes: 62 additions & 51 deletions src/components/connectAdvanced.js
Original file line number Diff line number Diff line change
@@ -1,34 +1,32 @@
import hoistStatics from 'hoist-non-react-statics'
import invariant from 'invariant'
import { Component, createElement } from 'react'
import { polyfill } from 'react-lifecycles-compat'

import Subscription from '../utils/Subscription'
import { storeShape, subscriptionShape } from '../utils/PropTypes'

let hotReloadingVersion = 0
const dummyState = {}
function noop() {}
function makeUpdater(sourceSelector, store) {
return function updater(props, prevState) {
try {
const nextProps = sourceSelector(store.getState(), props)
if (nextProps !== prevState.props || prevState.error) {
return {
shouldComponentUpdate: true,
props: nextProps,
error: null,
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
}
}
return {
shouldComponentUpdate: false,
}
} catch (error) {
return {
shouldComponentUpdate: true,
error,
} catch (error) {
selector.shouldComponentUpdate = true
selector.error = error
}
}
}

return selector
}

export default function connectAdvanced(
Expand Down Expand Up @@ -88,10 +86,6 @@ export default function connectAdvanced(
[subscriptionKey]: subscriptionShape,
}

function getDerivedStateFromProps(nextProps, prevState) {
return prevState.updater(nextProps, prevState)
}

return function wrapWithConnect(WrappedComponent) {
invariant(
typeof WrappedComponent == 'function',
Expand All @@ -118,11 +112,15 @@ 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)

this.version = version
this.state = {}
this.renderCount = 0
this.store = props[storeKey] || context[storeKey]
this.propsMode = Boolean(props[storeKey])
Expand All @@ -134,9 +132,7 @@ export default function connectAdvanced(
`or explicitly pass "${storeKey}" as a prop to "${displayName}".`
)

this.state = {
updater: this.createUpdater()
}
this.initSelector()
this.initSubscription()
}

Expand All @@ -159,19 +155,25 @@ export default function connectAdvanced(
// dispatching an action in its componentWillMount, we have to re-run the select and maybe
// re-render.
this.subscription.trySubscribe()
this.runUpdater()
this.selector.run(this.props)
if (this.selector.shouldComponentUpdate) this.forceUpdate()
}

componentWillReceiveProps(nextProps) {
this.selector.run(nextProps)
}

shouldComponentUpdate(_, nextState) {
return nextState.shouldComponentUpdate
shouldComponentUpdate() {
return this.selector.shouldComponentUpdate
}

componentWillUnmount() {
if (this.subscription) this.subscription.tryUnsubscribe()
this.subscription = null
this.notifyNestedSubs = noop
this.store = null
this.isUnmounted = true
this.selector.run = noop
this.selector.shouldComponentUpdate = false
}

getWrappedInstance() {
Expand All @@ -186,17 +188,10 @@ export default function connectAdvanced(
this.wrappedInstance = ref
}

createUpdater() {
initSelector() {
const sourceSelector = selectorFactory(this.store.dispatch, selectorFactoryOptions)
return makeUpdater(sourceSelector, this.store)
}

runUpdater(callback = noop) {
if (this.isUnmounted) {
return
}

this.setState(prevState => prevState.updater(this.props, prevState), callback)
this.selector = makeSelectorStateful(sourceSelector, this.store)
this.selector.run(this.props)
}

initSubscription() {
Expand All @@ -217,7 +212,24 @@ export default function connectAdvanced(
}

onStateChange() {
this.runUpdater(this.notifyNestedSubs)
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() {
Expand All @@ -238,26 +250,31 @@ export default function connectAdvanced(
}

render() {
if (this.state.error) {
throw this.state.error
const selector = this.selector
selector.shouldComponentUpdate = false

if (selector.error) {
throw selector.error
} else {
return createElement(WrappedComponent, this.addExtraProps(this.state.props))
return createElement(WrappedComponent, this.addExtraProps(selector.props))
}
}
}

/* eslint-enable react/no-deprecated */

Connect.WrappedComponent = WrappedComponent
Connect.displayName = displayName
Connect.childContextTypes = childContextTypes
Connect.contextTypes = contextTypes
Connect.propTypes = contextTypes
Connect.getDerivedStateFromProps = getDerivedStateFromProps

if (process.env.NODE_ENV !== 'production') {
Connect.prototype.componentDidUpdate = function componentDidUpdate() {
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
Expand All @@ -275,16 +292,10 @@ export default function connectAdvanced(
this.subscription.trySubscribe()
oldListeners.forEach(listener => this.subscription.listeners.subscribe(listener))
}

const updater = this.createUpdater()
this.setState({updater})
this.runUpdater()
}
}
}

polyfill(Connect)

return hoistStatics(Connect, WrappedComponent)
}
}
Loading