Skip to content

Commit

Permalink
Ensure that component prop 'context' really contains a React context … (
Browse files Browse the repository at this point in the history
#1134)

* Ensure that component prop 'context' really contains a React context before using it

after switching to react-redux 6.0.0, we've had a lot of errors stating

Warning: React.createElement: type is invalid -- expected a string (for built-in components) or a class/function (for composite components) but got: undefined. You likely forgot to export your component from the file it's defined in, or you might have mixed up default and named imports.

all over in our app. After digging deeper, we've discovered that we use a lot of (connected) components using a property named 'context' which conflicts with your new connectAdvanced code. So maybe you can improve the check on this.props.context a little bit to ensure it is used only if it really contains a valid React context.

* Update connectAdvanced.js

* Update connectAdvanced.js

* improved check whether context given as a prop is a real ReactContext

* added test for ignoring non-react-context values passed as a prop to the component

* Use react-is checks

* Just check Consumer. Good enough!

* improved check for context.Consumer

* added missing export 'isContextConsumer' in rollup config
  • Loading branch information
casdevs authored and timdorr committed Jan 8, 2019
1 parent e7661b3 commit 5199d9d
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 5 deletions.
9 changes: 6 additions & 3 deletions rollup.config.js
Expand Up @@ -2,7 +2,7 @@ import nodeResolve from 'rollup-plugin-node-resolve'
import babel from 'rollup-plugin-babel'
import replace from 'rollup-plugin-replace'
import commonjs from 'rollup-plugin-commonjs'
import {uglify} from 'rollup-plugin-uglify'
import { uglify } from 'rollup-plugin-uglify'
import pkg from './package.json'

const env = process.env.NODE_ENV
Expand All @@ -22,14 +22,17 @@ const config = {
nodeResolve(),
babel({
exclude: '**/node_modules/**',
runtimeHelpers: true,
runtimeHelpers: true
}),
replace({
'process.env.NODE_ENV': JSON.stringify(env)
}),
commonjs({
namedExports: {
'node_modules/react-is/index.js': ['isValidElementType'],
'node_modules/react-is/index.js': [
'isValidElementType',
'isContextConsumer'
]
}
})
]
Expand Down
9 changes: 7 additions & 2 deletions src/components/connectAdvanced.js
@@ -1,7 +1,7 @@
import hoistStatics from 'hoist-non-react-statics'
import invariant from 'invariant'
import React, { Component, PureComponent } from 'react'
import { isValidElementType } from 'react-is'
import { isValidElementType, isContextConsumer } from 'react-is'

import { ReactReduxContext } from './Context'

Expand Down Expand Up @@ -213,7 +213,12 @@ export default function connectAdvanced(
}

render() {
const ContextToUse = this.props.context || Context
const ContextToUse =
this.props.context &&
this.props.context.Consumer &&
isContextConsumer(<this.props.context.Consumer />)
? this.props.context
: Context

return (
<ContextToUse.Consumer>
Expand Down
30 changes: 30 additions & 0 deletions test/components/connect.spec.js
Expand Up @@ -1564,6 +1564,36 @@ describe('React', () => {
expect(actualState).toEqual(expectedState)
})

it('should ignore non-react-context values that are passed as a prop to the component', () => {
class Container extends Component {
render() {
return <Passthrough />
}
}

const nonContext = { someProperty: {} }

let actualState

const expectedState = { foos: {} }

const decorator = connect(state => {
actualState = state
return {}
})
const Decorated = decorator(Container)

const store = createStore(() => expectedState)

rtl.render(
<ProviderMock store={store}>
<Decorated context={nonContext} />
</ProviderMock>
)

expect(actualState).toEqual(expectedState)
})

it('should throw an error if the store is not in the props or context', () => {
const spy = jest.spyOn(console, 'error').mockImplementation(() => {})

Expand Down

0 comments on commit 5199d9d

Please sign in to comment.