Ensure that component prop 'context' really contains a React context … #1134
Conversation
…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.
|
Deploy preview for react-redux-docs ready! Built with commit 50fd894 |
|
Can you add a test? And please run you code through Prettier. |
|
ok, I'm going to add tests within the next few days |
|
improved checks & added test |
|
maybe it would even be better to check for this.props.context.$$typeof === REACT_CONTEXT_TYPE, but react doesn't seem to export its type symbols... |
|
We do have |
|
we could check for But a dedicated method like isContext doesn't seem to be available from 'react-is'. |
|
react-is has specific functions for those checks: ReactIs.isContextConsumer(<ThemeContext.Consumer />); // true
ReactIs.isContextProvider(<ThemeContext.Provider />); // true |
|
Added that for you now. |
|
great, thanks! |
|
Yeah, I removed that. It was also complaining about isContextProvider not being exported from react-is. Weird. |
|
hm, it doesn't seem to know |
|
And now isContextConsumer. wut? |
|
weird, when running the tests locally, however, the check must be done this way, otherwise other test cases fail: seems to be overhead, so maybe it would be better to simply check for |
|
Happy new year to all of you! |
|
Ah, I missed the Rollup config! I knew it was something obvious. |
|
OK, this looks good to me. Thanks! |
|
great, thanks for merging that in! |
|
When do you plan on cutting a new release with this change? |
|
@timdorr ? |
reduxjs#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
…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 already using a property named
contextwhich conflicts with your new connectAdvanced code. So maybe you can improve the check onthis.props.contexta little bit to ensure it is used only if it really contains a valid React context.