Skip to content

Commit 3c222f2

Browse files
committed
performance optimizations: HEADS UP API change too
* React.forwardRef is VERY slow, on the order of 2x slower in our benchmark. So we only enable it if withRef="forwardRef" folks using withRef=true will get an error telling them to update and not rely on getWrappedInstance() but just to use the ref directly * renderCountProp is removed, as this is natively supported in React dev tools now * all usages of shallowEquals are removed for pure components, it was unnecessary. * instead of allowing passing in a custom Context consumer in props, it is now required to be passed in via connect options at declaration time.
1 parent 68c1ffa commit 3c222f2

File tree

2 files changed

+93
-68
lines changed

2 files changed

+93
-68
lines changed

src/components/connectAdvanced.js

Lines changed: 71 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,11 @@ import hoistStatics from 'hoist-non-react-statics'
22
import invariant from 'invariant'
33
import React, { Component, PureComponent } from 'react'
44
import propTypes from 'prop-types'
5-
import shallowEqual from 'shallow-equals'
65
import { isValidElementType } from 'react-is'
76

87
import Context from './Context'
98

10-
const Consumer = Context.Consumer
9+
const ReduxConsumer = Context.Consumer
1110

1211
export default function connectAdvanced(
1312
/*
@@ -52,14 +51,14 @@ export default function connectAdvanced(
5251
withRef = false,
5352

5453
// the context consumer to use
55-
consumer = Consumer,
54+
consumer = ReduxConsumer,
5655

5756
// additional options are passed through to the selectorFactory
5857
...connectOptions
5958
} = {}
6059
) {
61-
invariant(!withRef,
62-
`withRef is removed. To access the wrapped instance, simply pass in ref`
60+
invariant(renderCountProp === undefined,
61+
`renderCountProp is removed. render counting is built into the latest React dev tools profiling extension`
6362
)
6463

6564
invariant(storeKey === 'store',
@@ -69,26 +68,39 @@ export default function connectAdvanced(
6968
'ConnectedComponent consumer={context.Consumer} /></Provider>'
7069
)
7170

71+
const Consumer = consumer
72+
7273
return function wrapWithConnect(WrappedComponent) {
7374
invariant(
7475
isValidElementType(WrappedComponent),
7576
`You must pass a component to the function returned by ` +
7677
`${methodName}. Instead received ${JSON.stringify(WrappedComponent)}`
7778
)
79+
invariant(!withRef || withRef === 'forwardRef',
80+
'withRef must be set to the text "forwardRef." Reference uses React.forwardRef and you may now access ref ' +
81+
`directly instead of using getWrappedInstance() in component ${wrappedComponentName}`
82+
)
7883

7984
const wrappedComponentName = WrappedComponent.displayName
8085
|| WrappedComponent.name
8186
|| 'Component'
8287

8388
const displayName = getDisplayName(wrappedComponentName)
8489

85-
class PureWrapper extends PureComponent {
90+
class PureWrapper extends Component {
91+
shouldComponentUpdate(nextProps) {
92+
return nextProps.derivedProps !== this.props.derivedProps
93+
}
94+
8695
render() {
87-
const { forwardRef, ...props } = this.props
88-
return <WrappedComponent {...props} ref={forwardRef} />
96+
let { forwardRef, derivedProps } = this.props
97+
return <WrappedComponent {...derivedProps} ref={forwardRef} />
8998
}
9099
}
100+
91101
PureWrapper.propTypes = {
102+
count: propTypes.object,
103+
derivedProps: propTypes.object,
92104
forwardRef: propTypes.oneOfType([
93105
propTypes.func,
94106
propTypes.object
@@ -108,19 +120,31 @@ export default function connectAdvanced(
108120
WrappedComponent
109121
}
110122

111-
class Connect extends Component {
123+
const OuterBase = connectOptions.pure ? PureComponent : Component
124+
125+
class Connect extends OuterBase {
112126
constructor(props) {
113127
super(props)
114-
invariant(!props[storeKey],
115-
'Passing redux store in props has been removed and does not do anything. ' +
116-
'To use a custom redux store for a single component, ' +
117-
'create a custom React context with React.createContext() and pass the Provider to react-redux\'s provider ' +
118-
'and the Consumer to this component as in <Provider context={context.Provider}><' +
119-
wrappedComponentName + ' consumer={context.Consumer} /></Provider>'
120-
)
128+
if (withRef) {
129+
invariant(!props.props[storeKey],
130+
'Passing redux store in props has been removed and does not do anything. ' +
131+
'To use a custom redux store for a single component, ' +
132+
'create a custom React context with React.createContext() and pass the Provider to react-redux\'s provider ' +
133+
'and the Consumer to this component as in <Provider context={context.Provider}><' +
134+
wrappedComponentName + ' consumer={context.Consumer} /></Provider>'
135+
)
136+
} else {
137+
invariant(!props[storeKey],
138+
'Passing redux store in props has been removed and does not do anything. ' +
139+
'To use a custom redux store for a single component, ' +
140+
'create a custom React context with React.createContext() and pass the Provider to react-redux\'s provider ' +
141+
'and the Consumer to this component as in <Provider context={context.Provider}><' +
142+
wrappedComponentName + ' consumer={context.Consumer} /></Provider>'
143+
)
144+
145+
}
121146
this.memoizeDerivedProps = this.makeMemoizer()
122147
this.renderWrappedComponent = this.renderWrappedComponent.bind(this)
123-
this.renderCount = 0
124148
}
125149

126150
makeMemoizer() {
@@ -132,7 +156,7 @@ export default function connectAdvanced(
132156
let called = false
133157
return (state, props, store) => {
134158
if (called) {
135-
const sameProps = connectOptions.pure && shallowEqual(lastProps, props)
159+
const sameProps = connectOptions.pure && lastProps === props
136160
const sameState = lastState === state
137161
if (sameProps && sameState) {
138162
return lastDerivedProps
@@ -146,7 +170,7 @@ export default function connectAdvanced(
146170
lastProps = props
147171
lastState = state
148172
const nextProps = sourceSelector(state, props)
149-
if (shallowEqual(lastDerivedProps, nextProps)) {
173+
if (lastDerivedProps === nextProps) {
150174
return lastDerivedProps
151175
}
152176
lastDerivedProps = nextProps
@@ -156,46 +180,56 @@ export default function connectAdvanced(
156180

157181
renderWrappedComponent(value) {
158182
invariant(value,
159-
`Could not find "store" in either the context of ` +
183+
`Could not find "store" in the context of ` +
160184
`"${displayName}". Either wrap the root component in a <Provider>, ` +
161185
`or pass a custom React context provider to <Provider> and the corresponding ` +
162-
`React context consumer to ${displayName}.`
186+
`React context consumer to ${displayName} in connect options.`
163187
)
164188
const { state, store } = value
165-
const { forwardRef, ...otherProps } = this.props
166-
let derivedProps = this.memoizeDerivedProps(state, otherProps, store)
189+
if (withRef) {
190+
const { forwardRef, props } = this.props
191+
let derivedProps = this.memoizeDerivedProps(state, props, store)
192+
if (connectOptions.pure) {
193+
return <PureWrapper derivedProps={derivedProps} forwardRef={forwardRef} />
194+
}
167195

168-
if (renderCountProp) {
169-
derivedProps = { ...derivedProps, [renderCountProp]: this.renderCount++ }
196+
return <WrappedComponent {...derivedProps} ref={forwardRef} />
170197
}
198+
let derivedProps = this.memoizeDerivedProps(state, this.props, store)
171199
if (connectOptions.pure) {
172-
return <PureWrapper {...derivedProps} forwardRef={forwardRef}/>
200+
return <PureWrapper derivedProps={derivedProps} />
173201
}
174-
return <WrappedComponent {...derivedProps} ref={forwardRef} />
202+
203+
return <WrappedComponent {...derivedProps} />
175204
}
176205

177206
render() {
178-
const MyConsumer = this.props.consumer || consumer
179207
return (
180-
<MyConsumer>
208+
<Consumer>
181209
{this.renderWrappedComponent}
182-
</MyConsumer>
210+
</Consumer>
183211
)
184212
}
185213
}
186214

187215
Connect.WrappedComponent = WrappedComponent
188216
Connect.displayName = displayName
189-
Connect.propTypes = {
190-
consumer: propTypes.object,
191-
forwardRef: propTypes.oneOfType([
192-
propTypes.func,
193-
propTypes.object
194-
])
217+
if (withRef) {
218+
Connect.propTypes = {
219+
props: propTypes.object,
220+
forwardRef: propTypes.oneOfType([
221+
propTypes.func,
222+
propTypes.object
223+
])
224+
}
225+
}
226+
227+
if (!withRef) {
228+
return hoistStatics(Connect, WrappedComponent)
195229
}
196230

197231
function forwardRef(props, ref) {
198-
return <Connect {...props} forwardRef={ref} />
232+
return <Connect props={props} forwardRef={ref} />
199233
}
200234

201235
const forwarded = React.forwardRef(forwardRef)

test/components/connect.spec.js

Lines changed: 22 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1280,7 +1280,7 @@ describe('React', () => {
12801280
{makeContainer(() => ({}), () => ({}), () => 1)}
12811281
</ProviderMock>
12821282
)
1283-
expect(spy).toHaveBeenCalledTimes(1)
1283+
expect(spy).toHaveBeenCalledTimes(2)
12841284
expect(spy.mock.calls[0][0]).toMatch(
12851285
/mergeProps\(\) in Connect\(Container\) must return a plain object/
12861286
)
@@ -1293,7 +1293,7 @@ describe('React', () => {
12931293
{makeContainer(() => ({}), () => ({}), () => 'hey')}
12941294
</ProviderMock>
12951295
)
1296-
expect(spy).toHaveBeenCalledTimes(1)
1296+
expect(spy).toHaveBeenCalledTimes(2)
12971297
expect(spy.mock.calls[0][0]).toMatch(
12981298
/mergeProps\(\) in Connect\(Container\) must return a plain object/
12991299
)
@@ -1390,7 +1390,7 @@ describe('React', () => {
13901390
const decorator = connect(state => {
13911391
actualState = state
13921392
return {}
1393-
})
1393+
}, undefined, undefined, { consumer: context.Consumer })
13941394
const Decorated = decorator(Container)
13951395
const mockStore = {
13961396
dispatch: () => {},
@@ -1468,7 +1468,7 @@ describe('React', () => {
14681468
}
14691469
}
14701470

1471-
const decorator = connect(state => state, null, null)
1471+
const decorator = connect(state => state, undefined, undefined, { withRef: 'forwardRef' })
14721472
const Decorated = decorator(Container)
14731473

14741474
const ref = React.createRef()
@@ -2174,19 +2174,19 @@ describe('React', () => {
21742174
const store2 = createStore((state = 0, action) => (action.type === 'INC' ? state + 1 : state))
21752175
const customContext = React.createContext()
21762176

2177-
@connect(state => ({ count: state }))
2177+
@connect(state => ({ count: state }), undefined, undefined, { consumer: customContext.Consumer })
21782178
class A extends Component {
2179-
render() { return <B consumer={customContext.Consumer} /> }
2179+
render() { return <B /> }
21802180
}
21812181

21822182
const mapStateToPropsB = jest.fn(state => ({ count: state }))
2183-
@connect(mapStateToPropsB)
2183+
@connect(mapStateToPropsB, undefined, undefined, { consumer: customContext.Consumer })
21842184
class B extends Component {
21852185
render() { return <C {...this.props} /> }
21862186
}
21872187

21882188
const mapStateToPropsC = jest.fn(state => ({ count: state }))
2189-
@connect(mapStateToPropsC)
2189+
@connect(mapStateToPropsC, undefined, undefined, { consumer: customContext.Consumer })
21902190
class C extends Component {
21912191
render() { return <D /> }
21922192
}
@@ -2244,6 +2244,16 @@ describe('React', () => {
22442244
expect(spy).not.toHaveBeenCalled()
22452245
})
22462246

2247+
it('should error on withRef=true (must be "forwardRef")', () => {
2248+
class Container extends Component {
2249+
render() {
2250+
return <div>hi</div>
2251+
}
2252+
}
2253+
expect(() => connect(undefined, undefined, undefined, { withRef: true })(Container))
2254+
.toThrow(/withRef must be set/)
2255+
})
2256+
22472257
it('should error on receiving a custom store key', () => {
22482258
const connectOptions = { storeKey: 'customStoreKey' }
22492259

@@ -2258,17 +2268,6 @@ describe('React', () => {
22582268
}).toThrow(/storeKey has been removed/)
22592269
})
22602270

2261-
it('should error on withRef', () => {
2262-
function Container() {
2263-
return <div>hi</div>
2264-
}
2265-
expect(() => {
2266-
connect(undefined, undefined, undefined, { withRef: true })(Container)
2267-
}).toThrow(
2268-
'withRef is removed. To access the wrapped instance, simply pass in ref'
2269-
)
2270-
})
2271-
22722271
it('should error on custom store', () => {
22732272
function Comp() {
22742273
return <div>hi</div>
@@ -2282,21 +2281,13 @@ describe('React', () => {
22822281
}).toThrow(/Passing redux store/)
22832282
})
22842283

2285-
it('should add a renderCount prop if specified in connect options', () => {
2286-
let value = 0
2287-
const store = createStore(() => ({ value: value++}))
2284+
it('should error on renderCount prop if specified in connect options', () => {
22882285
function Comp(props) {
22892286
return <div>{props.count}</div>
22902287
}
2291-
const Container = connect(undefined,undefined,undefined,{ renderCountProp: 'count' })(Comp)
2292-
const tester = rtl.render(
2293-
<ProviderMock store={store}>
2294-
<Container/>
2295-
</ProviderMock>
2296-
)
2297-
expect(tester.queryByText('0')).not.toBe(null)
2298-
store.dispatch({ type: 'hi' })
2299-
expect(tester.queryByText('1')).not.toBe(null)
2288+
expect(() => {
2289+
connect(undefined,undefined,undefined,{ renderCountProp: 'count' })(Comp)
2290+
}).toThrow(/renderCountProp is removed/)
23002291
})
23012292
})
23022293
})

0 commit comments

Comments
 (0)