Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix getDerivedStateFromProps usage #980

Closed
wants to merge 20 commits into from
Closed

Fix getDerivedStateFromProps usage #980

Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
b631441
up min react versions
cellog Jul 21, 2018
25e3fc6
working, except the issue with multiple mapState calls
cellog Jul 21, 2018
de4e4f3
typo
cellog Jul 21, 2018
f751ac9
success! gDSFP-based react-redux
cellog Jul 22, 2018
9284894
up the peer dependency to React 16.4
cellog Jul 22, 2018
948c20f
remove debugging code
cellog Jul 22, 2018
490b0e3
use polyfill, make peer dependency relaxed
cellog Jul 24, 2018
86e986b
fix style details
cellog Jul 25, 2018
1d96dac
move instance props into state
cellog Aug 1, 2018
26e4fb8
make subscription async-safe
cellog Aug 2, 2018
6b83e13
fully fix subscription
cellog Aug 2, 2018
8a8e151
update subscription change check to be accurate
cellog Aug 2, 2018
52d4f45
rename resolves to be clearer
cellog Aug 3, 2018
3333363
resolve conflicts
cellog Aug 3, 2018
d50fb7d
Merge react-redux master into another-fix-5.1
cellog Aug 3, 2018
1d8ab34
remove console statements
cellog Aug 3, 2018
fe60a4b
fix for React 16-16.3
cellog Aug 3, 2018
72fca1c
fix breaking tests
cellog Aug 3, 2018
1285e4e
remove unnecessary Promise (React lifecycle does this for us)
cellog Aug 3, 2018
35e9d12
fix nitpicks and use a constant instead of function call every connect()
cellog Aug 4, 2018
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
14 changes: 12 additions & 2 deletions src/components/connectAdvanced.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import hoistStatics from 'hoist-non-react-statics'
import invariant from 'invariant'
import { Component, createElement } from 'react'
import React, { Component, createElement } from 'react'
import { polyfill } from 'react-lifecycles-compat'
import shallowEqual from '../utils/shallowEqual'

Expand All @@ -10,6 +10,12 @@ import { storeShape, subscriptionShape } from '../utils/PropTypes'
let hotReloadingVersion = 0
function noop() {}

function isOldReact() {
const version = React.version.split('.')
if (+version[0] < 16) return true
if (+version[0] > 16) return false
return +version[1] < 4
}
export default function connectAdvanced(
/*
selectorFactory is a func that is responsible for returning the selector function used to
Expand Down Expand Up @@ -140,7 +146,11 @@ export default function connectAdvanced(

static getDerivedStateFromProps(props, state) {
let ret = null
if (state.lastNotify !== state.notifyNestedSubs) {
// this next check only should trigger if gDSFP reacts to state changes
// in React 16.3, it doesn't. react-lifecycles-compat matches React 16.3 behavior
// if a future version of react-lifecycles-compat DOES trigger on state changes
// the isOldReact() call must be removed
if (!isOldReact() && state.lastNotify !== state.notifyNestedSubs) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No reason to do the string splitting every time this runs. We can just do it once when connect() is called, or even once at the top of the file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

duh. fixing :)

ret = { lastNotify: state.notifyNestedSubs }
if (shallowEqual(props, state.props) || state.error) {
return ret
Expand Down