connect's storeSubscription prop propagation when custom store is provided

[HeberLZ]

redux: 4.0.0
react-redux: 5.0.7
description: (updated to make more sense to ppl arriving late to the issue)

So here is the contrived example, hopefully it's simple enough.

Basically what happens is that connect when provided a store property will create a storeSubscription and propagate it as a prop downwards, which means that if we have two HoC's with custom store, one wrapping another, the second one will try to use the storeSubscription from the first one even though it's creating its own one too. This results in actions fired by the second HoC not propagating changes until an action on the first HoC happens.

I do know that this is not a common use-case and that the tool is actually intended to be used with Provider, where this behaviour does not happen because the storeSubscription does not get propagated as a prop downward, but due to the nature of the projects I'm working on I'm encountering this issue/non-issue. I already have a workaround which is to instead of returning the connect hoc directly, return a wrapper on top of the connect hoc which discards both storeSubscription input prop and output propagated props.

Mainly i'd like to know if it is a bug or if this is the intended behaviour of connect, and what's the reasoning behind it on the latter as it might make sense to work like that due to a design decision.

https://codesandbox.io/s/qlk98486n9

index.js

import React from "react";
import { render } from "react-dom";
import { compose, mapProps } from "recompose";

import createPlugin from "./createPlugin";

const { HoC: plugin1HoC, action: plugin1Action } = createPlugin(
  "attributeFromHoC1"
);
const { HoC: plugin2HoC, action: plugin2Action } = createPlugin(
  "attributeFromHoC2"
);

const NestedPluginsComponent = compose(plugin1HoC, plugin2HoC)(props => (
  <div>
    <h2>props: {Object.keys(props).join(", ")}</h2>
    <div>attributeFromHoC1: {props.attributeFromHoC1}</div>
    <button onClick={plugin1Action}>plugin1Action</button>
    <div>attributeFromHoC2: {props.attributeFromHoC2}</div>
    <button onClick={plugin2Action}>plugin2Action</button>
  </div>
));

const App = () => (
  <div>
    <NestedPluginsComponent />
  </div>
);

render(<App />, document.getElementById("root"));

createPlugin.jsx

import React from "react";
import { createStore } from "redux";
import { connect } from "react-redux";
import { compose, withProps } from "recompose";

export default function createPluginHoC(name) {
  const store = createStore(
    (s, a) =>
      a.type === "randomize"
        ? { [name]: Math.floor(Math.random() * 100 + 1) }
        : s,
    {}
  );
  return {
    HoC: compose(withProps({ store }), connect(s => s)),
    action: () => store.dispatch({ type: "randomize" })
  };
}

https://github.com/reactjs/react-redux/blob/57128d5a20d15f43513acd13251465966426d956/src/components/connectAdvanced.js#L199

To give some context, I'm currently working on autonomous embedded apps and plugins with custom stores that are used across multiple apps.

Is this an expected behaviour? should this be documented in that case? should this props not be propagated? or should the storeSubscription prop be ignored when the store property is provided?

Cheers!

[markerikson]

store as a prop directly to a connected component is a bit of an escape hatch that's not really meant for use in an application directly. It's primarily beneficial for testing a connected component.

Is there any reason why you can't put a <Provider> around that portion of your app?

[HeberLZ]

I’m basically working on embedded apps and plugins, that’s mainly why i went with the custom store approach, otherwise i’d have sticked with the single store. I could wrap those connects on two providers to override the context and then resume it, but it would add more layers, or i could set custom keys for store and storesubscription but it would still be error prone and more difficult as i’m using an abstraction on top of redux

On Mon, Apr 30, 2018 at 5:09 PM Mark Erikson ***@***.***> wrote: store as a prop directly to a connected component is a bit of an escape hatch that's not really meant for use in an application directly. It's primarily beneficial for testing a connected component. Is there any reason why you can't put a <Provider> around that portion of your app? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#942 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABZhal-PHpBMDuwUBNxIc0woA7fQvVA9ks5tt6epgaJpZM4TtY6J> .

-- Heber López | Google Developers Group Lead Organizer, Mendoza | +HeberLZ <https://plus.google.com/+HeberLopez> | +54 261 5342773 |

[HeberLZ]

For the embedded apps the provider would suffice, but for the reusable plugins we'd prefer to provide those components and hocs as decoupled as possible from the apps, so that if we want to change the underlying data layer then the app does not have to update anything, and using a regular connect as the HoC provides the issues i mentioned before when used in conjunction with other connects

[markerikson]

The difference here is that using <Provider> sets the store for all nested components under it, while passing the store as a prop only sets it for that specific component. You're saying that the latter behavior is what you want in this case - only having it apply to this component, not any of its descendants?

[markerikson]

Actually, going back to your original question, I'm not sure I fully understand the issue you're describing. Can you provide an example snippet or project?

[HeberLZ]

Sure, give me a min to remove all the indirections and provide a minimal sample.

[HeberLZ]

Writing the contrived example I'm realizing a few more things. The only issue is that whenever a connect HoC is provided a 'store' prop, it will create a storeSubscription and pass it downwards as a prop. On the other hand if a connect is provided a store and a storeSubscription that don't match (due to store being injected manually and storeSubscription coming from another plugin HoC) then is when the weird behaviour happens. It's an internal state problem most likely as the new storeSubscription prop output will now match the provided store.

I'll have to dig more into it as just doing a strict comparison wasn't enough, i'll have to add actions that don't propagate correctly, will work on that tomorrow

[HeberLZ]

So here is the contrived example, hopefully it's simple enough.

Basically what happens is that connect when provided a store property will create a storeSubscription and propagate it as a prop downwards, which means that if we have two HoC's with custom store, one wrapping another, the second one will try to use the storeSubscription from the first one even though it's creating it's own one too. This results in actions fired by the second HoC not propagating changes until an action on the first HoC happens.

I do know that this is not a common usecase and that the tool is actually intended to be used with Provider, where this behaviour does not happen because the storeSubscription does not get propagated as a prop downward, but due to the nature of the projects i'm working on i'm encountering this issue/nonissue. I already have a workaround which is to instead of returning the connect hoc directly, return a wrapper on top of the connect hoc which discards both storeSubscription input prop and output propagated props.

Mainly i'd like to know if it is a bug or if this is the intended behaviour of connect, and what's the reasoning behind it on the latter as it might make sense to work like that due to a design decision.

Cheers!

https://codesandbox.io/s/qlk98486n9

index.js

import React from "react";
import { render } from "react-dom";
import { compose, mapProps } from "recompose";

import createPlugin from "./createPlugin";

const { HoC: plugin1HoC, action: plugin1Action } = createPlugin(
  "attributeFromHoC1"
);
const { HoC: plugin2HoC, action: plugin2Action } = createPlugin(
  "attributeFromHoC2"
);

const NestedPluginsComponent = compose(plugin1HoC, plugin2HoC)(props => (
  <div>
    <h2>props: {Object.keys(props).join(", ")}</h2>
    <div>attributeFromHoC1: {props.attributeFromHoC1}</div>
    <button onClick={plugin1Action}>plugin1Action</button>
    <div>attributeFromHoC2: {props.attributeFromHoC2}</div>
    <button onClick={plugin2Action}>plugin2Action</button>
  </div>
));

const App = () => (
  <div>
    <NestedPluginsComponent />
  </div>
);

render(<App />, document.getElementById("root"));

createPlugin.jsx

import React from "react";
import { createStore } from "redux";
import { connect } from "react-redux";
import { compose, withProps } from "recompose";

export default function createPluginHoC(name) {
  const store = createStore(
    (s, a) =>
      a.type === "randomize"
        ? { [name]: Math.floor(Math.random() * 100 + 1) }
        : s,
    {}
  );
  return {
    HoC: compose(withProps({ store }), connect(s => s)),
    action: () => store.dispatch({ type: "randomize" })
  };
}

[markerikson]

Thanks for putting that together. Paging @jimbolla ...

[HeberLZ]

Sidenote: the storeSubscription prop propagation cannot be stopped by providing a custom mergeProps fn to connect either

[jimbolla]

You could try specifying a different storeKey

[timdorr]

Yeah, just following along from the sidelines, but that would appear to be the issue here. The storeKey is overlapping. That's the point of that particular config option.

[HeberLZ]

Providing a custom storeKey works and it's way better than what I'm doing now to circumvent this issue so I'm going to update the approach. Nevertheless, I'd love to know what was the reasoning behind propagating connect's autogenerated storeSubscription as a property into the wrapped component.

The downsides I see on providing the storeKey are that internal redux attributes will still be getting exposed to wrapped components, and it doesn't completely fix the issue as there could still be naming collisions due to the fact that I don't really know anything about the applications using the plugins I'm making, and would also like to make them unaware of whatever state management solution I might be using.

Cheers!

[markerikson]

Jim wrote that section, so he can speak to the intent best there.

FWIW, it's likely the separate "subscription" prop and setup would be going away in the next major version of React-Redux, as we'll probably be switching the internal handling of subscription logic to use a different approach.

[HeberLZ]

I forgot to close the story, sorry about that. To be more specific about my question, on the following line, if the subscription is treated as null on propsMode then everything should work as expected, which for a different reason is how it's being handled for context propagation in order to prevent breaking the context chain.

https://github.com/reactjs/react-redux/blob/57128d5a20d15f43513acd13251465966426d956/src/components/connectAdvanced.js#L199

[HeberLZ]

It doesn't really matter anymore, I just saw what you were talking about, with the new implementation on #856 it will no longer be an issue anymore once the next major version gets released. Thanks!

[markerikson]

That particular implementation is still an experiment, as is #898 . However, whatever we finally put together will probably look something like one of those, yes.