Data: Avoid lingering listeners from stringified data-bound components

[aduth]

This pull request seeks to explore a resolution to an issue raised by @jsnajdr at #11777 (comment) in that our current approach to subscribing to store updates from withSelect is problematic in leaving lingering subscribers to a registry if the component doesn't complete the full mounting lifecycle (e.g. renderToString only constructs, never componentDidMount nor componentWillUnmount).

Status:

In its current form, this pull request only includes a failing test case illuminating the issue.

In experimenting toward a resolution, a few challenges have surfaced:

• Unlike react-redux, we do not require a top-level context provider, which introduces challenges in the direction of subscribing to an ancestor instance of the component rather than to the registry directly
• Since a registry can consist of several stores, it is not trivial to determine whether state has changed between a component's constructor and its componentDidMount.
  • The "trivial" way would be to attach a subscriber to the registry and track whether the callback is invoked, but again, since we have no guarantee that componentDidMount will actually be called, this introduces a very similar issue with lingering subscribers

The two general angles I've tried taking here include some combination of:

• Moving subscribe (whether to the registry or some context-provided ancestor proxy) into componentDidMount
  • Complicated by:
    • Parent > Child subscriber order (parent should be called before child)
    • If a change in state occurs between initial mergeProps being calculated in constructor and the componentDidMount
• Creating a separate context Provider/Consumer pair dedicated to subscriptions, where each instance of a withSelect manages subscriptions for all descendent instances, to assure proper ordering of callbacks, to manage changes in registry, and to help in limiting the total number of store subscribers (rather than each withSelect subscribing to registry, it just receives new context when registry updates).
  • There's still an issue of: If state changed between constructor and componentDidMount (presumably the latter is where the registry subscription is attached), how is this known?

cc @jsnajdr @youknowriad @Tug

[youknowriad]

Unlike react-redux, we do not require a top-level context provider, which introduces challenges in the direction of subscribing to an ancestor instance of the component rather than to the registry directly

Can we try to mimic the last version of react-redux, maybe something like that would work?

• Subscribe only in the provider if a provider is used.
• Subscribe only in the first withDispatch/withSelect if there's no parent provider.

[jsnajdr]

I created a little experimental app that creates a nested tree of context providers and consumers, where each WithRegistry (similar to connect) passes an updated context value to its children:

const Registry = React.createContext("root");

const WithRegistry = props => (
  <Registry.Consumer>
    {registry => (
      <Registry.Provider value={`${registry}/sub`}>
        {props.children(registry)}
      </Registry.Provider>
    )}
  </Registry.Consumer>
);

const App = () => (
  <WithRegistry>
    {registry => (
      <ul>
        <li>{registry}</li>
        <WithRegistry>{registry => <li>{registry}</li>}</WithRegistry>
      </ul>
    )}
  </WithRegistry>
);

And it works as expected: renders this list:

• root
• root/sub

Consumer that has no parent provider uses the default value of the context ("root"). Each WithRegistry uses the registry value passed to it in context. And its child WithRegistry components receive an updated context based on the parent one ("root/sub"). Exactly what we need to implement a react-redux@5-style tree of nested subscriptions with no top-level provider.

But if we could rely on a top-level provider being available and reacting to registry changes, the implementation could be even simpler. Like in react-redux@next, WithRegistry could be a trivial context consumer with no magic at all.

It it possible to add such a provider? We already need the EditorProvider for the editor to work at all, so one additional DataProvider can't hurt, can it?

[aduth]

This still needs to be resolved, but the pull request in its current form is not under active development. The test case can be cherry-picked into a subsequent pull request, or this one reopened at a future time when effort can be directed to it.

[aduth]

I didn't realize there was not a corresponding issue here, so I've created one to at least ensure it continues to be tracked as a bug.

See #13879