Conflict with passing a "context" prop

[jmakGH]

Upon upgrading React and React Redux to the latest versions, an app I am working on started crashing on render.

After some debugging I traced the issue to a connected component being passed a "context" prop that was being used to control some of the app's features and completely unrelated to React's Context API.

I couldn't find any notes about that being a protected prop anywhere in the docs or online and thought it might be a good idea to post this here in case anyone else happened to stumble across this issue.

The biggest takeaway was that the only error message came from React, which made debugging the issue kind of a hassle -- especially because everything was working prior to upgrading to the latest version (which makes sense since the version of React we were using was before their Context update).

Perhaps a note under the Troubleshooting section of the website could document this issue as a potential pitfall.

Issue is recreated at this CodeSandbox. The app crashes whenever a context prop is passed to ConnectedChild.

Which versions of React, ReactDOM/React Native, Redux, and React Redux are you using? Which browser and OS are affected by this issue? Did this work in previous versions of React Redux?
React 16.8.1
ReactDOM 16.8.1
React Redux 6.0.0

This issue wasn't happening with React v15 and React Redux v4

[markerikson]

It is mentioned in the docs a couple times, but not particularly highlighted:

https://react-redux.js.org/api/connect#context-object

https://react-redux.js.org/using-react-redux/accessing-store#providing-custom-context

Up through 5.x, store was effectively a "reserved" prop, allowing users to pass a specific store instance directly to a connected component.

In 6.0, store is no longer meaningful as a prop, but we throw errors to let users know they probably made a mistake when upgrading and should change their code (because it won't do what they expected it to do). We also added context as a "reserved prop", because we allow users to pass a custom context instance to a connected component.

Given that we're considering having our next version be a major version change anyway, perhaps we could consider renaming that prop to be something like customContext instead of just context.

[jmakGH]

My mistake, I had overlooked that in the docs. I'm perhaps at fault for using your comment on #1104 as a reference guide for the migration without completely reading the updated docs.

I think you guys should be fine leaving it as context seeing as how I had a difficult time finding any discussion from others facing this issue. I'd assume most people now generally reserve context for React in general to avoid conflicts in semantics/code. We just had just been using it as a prop for a long time before they updated their Context API.

Thank you for your response! I'll go ahead and close this issue.

[sakulstra]

not sure if it's to late to alter the breaking changes 🤷‍♂️ but would have been great to have this noted somewhere :D debugged this for more than an hour till i found the problem. context as a component prop is sth we appearently use relatively often so the update broke our app at multiple places with no obvious relation to each other.

[TheMoonDawg]

@markerikson Did y'all decided against renaming the prop to something like customContext? We also just ran into this same issue after upgrading.

Just curious. Thanks!

[strongui]

@markerikson @jmakGH What is the resolution to this? Can we change the name of the redux context prop? We use context all over our app, and it's not practical to rename it everywhere.

[markerikson]

Nope, there are not going to be any changes here. For one thing, we recommend using the hooks API instead of connect. Second, we just had a major version release (v8). No one brought this up during the extensive alpha/beta cycle, so it wasn't even a thing we could consider as a possible breaking change.

[strongui]

@markerikson With connect I was able to connect the same component with two different connect functions based on need. export fullComp = connect() vs export baseComp = connectBase(). Can something like this be replicated with the hooks?

[phryneas]

@strongui you will probably have to give more of an example what that was actually doing.

[strongui]

@phryneas I basically have a component that can be a top level Component, that gets all data/methods from redux. But it can also be used as a "sub" component, where it gets methods from redux, but data from the parent. So I was able to export the same component but with two separate connect methods.

I would just like to know if this can be replicated. Otherwise I have rename a prop across thousands of files because of this :/

[markerikson]

I'm sorry, but this is a public API and we can't / won't change it at this point. It's also been the same for several years now. Is there a specific reason why this is suddenly a problem?

Like Lenz said, if you can provide some actual example code demonstrating what you're doing, we may be able to offer other suggestions.

[strongui]

@markerikson we have a huge app, we haven't had time to update until recently.

const connector = connect(mapStateToProps, mapDispatchToProps);

const connectorCore = connect(null, mapDispatchToPropsCoreMethods);


const BaseComp = (props) => {
  return <div></div>
}

// This COMP will get all it's data from the redux store
export const FullComponent = connector(BaseComp);
// This COMP will demand data from the parent
export const NonDataComponent = connectorCore(BaseComp);

Basically this. Same component, but one gets data form redux, the other will get data from parent.

[phryneas]

@strongui something like this?

function NonDataComponent({ data }) {
  const dispatch = useDispatch()

  // just use dispatch here to dispatch your actions
}

function FullComponent (){
  const dataFromStore = useSelector(selectDataFromStore)
  return <NonDataComponent data={dataFromStore} />
}

[strongui]

@phryneas Nevermind I misread! I think that will work

[markerikson]

Out of curiosity, what version of React-Redux were you on prior to upgrading? Per the top of the context field has been there since React-Redux v6 at the end of 2018.

[strongui]

@markerikson

We are on:
"@types/react-redux": "7.1.22", "react-redux": "^7.2.8",
Trying to upgrade to:
"@types/react-redux": "^7.1.24", "react-redux": "^8.0.2",
During the update TS started throwing the context prop error. Technically I am not even sure if it is a problem, since we pass a context: string prop, but does that override whatever is provided by redux connect? I am not sure, I just want to do things right, so there are no errors and our packages stay up to date.

The biggest problem is that it means I can't upgrade one piece at a time, since this error is all over the place where we use connect(), so I'll have to update everything to use hooks to avoid this.

[markerikson]

Hmm. I'm starting to vaguely remember some bits and pieces of details here.

If I remember right, we did have some previous similar complaints, and I ended up implementing some complex internal checks to see "okay, you passed in a prop named context - is that really a React context instance, or is it plain data that we should forward through?".

AH. Found it - it was actually about a prop named store:

• Pass non-Redux-store values through the store prop #1447

so related but different.

Can you post an actual repo or CodeSandbox and show the actual error you're getting? I'm still not quite following where the type clash is happening.

[strongui]

@markerikson https://codesandbox.io/s/tender-mestorf-x272kj?file=/src/index.tsx are you able to see this?

[phryneas]

@strongui connect(Button) should be connect()(Button) - although there doesn't seem any use for connect at all?

[strongui]

@phryneas @markerikson sorry for the typo, anyway now you see the TS error:

Type '"LIST"' is not assignable to type '("LIST" | "CARD") & Context<ReactReduxContextValue<any, AnyAction>>'.ts(2322)

@phryneas although there doesn't seem any use for connect at all, this is just an example to show the error that I have. In our app "context" is used all over the place to signify "LIST" , "CARD", "SUMMARY" etc. So that is the TS error I am dealing with currency trying to update.

I am not opposed to updating all to use toolkit if that is what needs to be done. I just wish I could do it in steps, because this will be a massive undertaking for a prop name :/

[phryneas]

There is indeed a failsafe check in place to only use that context prop if it actually contains a Context-type object and that is not reflected in the types:

react-redux/src/components/connect.tsx

Lines 530 to 539 in fb8cfc0

	const ContextToUse: ReactReduxContextInstance = useMemo(() => {
	// Users may optionally pass in a custom context instance to use instead of our ReactReduxContext.
	// Memoize the check that determines which context instance we should use.
	return propsContext &&
	propsContext.Consumer &&
	// @ts-ignore
	isContextConsumer(<propsContext.Consumer />)
	? propsContext
	: Context
	}, [propsContext, Context])

So this could at that point be accommodated with a TS only change.

[markerikson]

Hmm. Interesting. If I fix the use of connector(Button), and then swap back to React-Redux v7, the wrapped Button component says that context is just that enum, not a ReactReduxContext instance.

I wonder what about our types changed there? It most likely had something to do with the TS conversion process.

The internal logic for "what do we do with props.context internally once we see it" hasn't changed, and if I'm reading this right, it ought to actually pass that through properly if it's not a context instance:

      const ContextToUse: ReactReduxContextInstance = useMemo(() => {
        // Users may optionally pass in a custom context instance to use instead of our ReactReduxContext.
        // Memoize the check that determines which context instance we should use.
        return propsContext &&
          propsContext.Consumer &&
          // @ts-ignore
          isContextConsumer(<propsContext.Consumer />)
          ? propsContext
          : Context
      }, [propsContext, Context])

      // Retrieve the store and ancestor subscription via context, if available
      const contextValue = useContext(ContextToUse)

It's probably this part:

export interface ConnectProps {
  /** A custom Context instance that the component can use to access the store from an alternate Provider using that same Context instance */
  context?: ReactReduxContextInstance
  /** A Redux store instance to be used for subscriptions instead of the store from a Provider */
  store?: Store
}

interface InternalConnectProps extends ConnectProps {
  reactReduxForwardedRef?: React.ForwardedRef<unknown>
}

    function ConnectFunction<TOwnProps>(
      props: InternalConnectProps & TOwnProps
    )  {
       // omit
    }

Given that, I think this would run, but it won't compile, which is an issue.

I don't immediately have a suggestion to work around this (also I'm context-switching (HAH!) from work to this thread every couple minutes, which isn't helping)

[strongui]

@markerikson @phryneas

So, if I'm reading this correctly, our context will continue to be there even with the update to react-redux 8. That means that technically the code would work, but like you said it won't compile because of the errors. I wonder if I could provide a TS override temporarily while I take the time to migrate everything over to toolkit, to resolve this?

[markerikson]

@strongui to be clear, this doesn't actually have anything to do with Redux Toolkit at all. This is strictly the TS types for connect with React-Redux v8.

[markerikson]

actually. I bet we can do some fancy footwork with props: InternalConnectProps & TOwnProps.

Like, "if TOwnProps has a field named context use that, otherwise use it from InternalConnectProps".

[strongui]

@markerikson oh are you saying the typings are not correct, since it technically is not reflecting what is actually happening in the code? It would be pretty awesome if it was just that as a fix, it would make upgrading a breeze! Who maintains those? How can I help?

[strongui]

I've investigated some more, and it doesn't seem like a fix to this is possible just in the typings @types/react-redux, the problem is here: in the react-redux package

diff --git a/node_modules/react-redux/es/components/connect.d.ts b/node_modules/react-redux/es/components/connect.d.ts
index 347b3b0..c844c92 100644
--- a/node_modules/react-redux/es/components/connect.d.ts
+++ b/node_modules/react-redux/es/components/connect.d.ts
@@ -6,7 +6,7 @@ import type { uSES } from '../utils/useSyncExternalStore';
 export declare const initializeConnect: (fn: uSES) => void;
 export interface ConnectProps {
     /** A custom Context instance that the component can use to access the store from an alternate Provider using that same Context instance */
-    context?: ReactReduxContextInstance;
+    context?: any;
     /** A Redux store instance to be used for subscriptions instead of the store from a Provider */
     store?: Store;
 }

For now I've just created a patch version in our app to deal with this.

[markerikson]

@strongui we actually have a likely fix merged :) See #1956 .

Can you try out the CodeSandbox CSB build from that PR and see if it works for you?

[strongui]

@markerikson I have updated my sandbox to use the version from the PR

https://codesandbox.io/s/tender-mestorf-x272kj

Unfortunately it does not seem to fix it.

[phryneas]

@markerikson different issue, different fix ;)

[markerikson]

Oh, whoops, misread that, sorry!

[markerikson]

@strongui @phryneas given that this issue has actually been closed for a couple years, I've created a new issue at #1957 - let's continue the discussion there.