DynamicModuleLoader's children mounts twice

[microsoft]

DynamicModuleLoader's children mounts twice

[navneet-g]

@anilanar thanks for reporting, Can you give a codesandbox with repro?

[anilanar]

https://codesandbox.io/s/8kx2jr7762

This behaviour is expected. If you do the following, Comp will mount twice because React re-creates the whole element tree if the root's type is different than before.

let count = 0;
const App = () => (
  if (count++) return <div><Comp /></div>;
  else return <Comp />;
);

[navneet-g]

Sorry I didn't get a chance to look at it yet. I will take a look this week and get back to you.

[navneet-g]

@anilanar I am not able to open the sandbox, I get following error. I will try again later today

[anilanar]

Perhaps codesandbox had problems when you tried it, I can open it.

[navneet-g]

I can repro this and this is what I see, the storeState changes and thus React rerenders children
We need to debug more to see why the storeState is not upto date in first render after the module is loaded. Most probabbly this is another side effect of React's new context API, but for sure this is not a bug in redux-dynamic-modules. It takes a long time to debug react, I will give it a try later in the week if I get sometime.

[shockk]

I have this trouble too. This issue blocks releasing our app (

[anilanar]

@navneet-g State not being up to date after store is updated (by redux-dynamic-modules) is not a bug in redux-dynamic-modules. But using two different component roots (to overcome aforementioned problem) is a bug in redux-dynamic-modules.

[navneet-g]

@shockk in our app we are using following version of react-redux, and redux and don't see this issue. We are using it with React 16.8. Please try these versions for now to unblock the release.
react-redux": "^5.1.1",
"redux": "^4.0.0",

[navneet-g]

@anilanar which two different component roots are you mentioning? Please elaborate.

[anilanar]

@navneet-g

First the following:

https://github.com/Microsoft/redux-dynamic-modules/blob/master/packages/redux-dynamic-modules/src/DynamicModuleLoader.tsx#L135-L148

and

https://github.com/Microsoft/redux-dynamic-modules/blob/master/packages/redux-dynamic-modules/src/DynamicModuleLoader.tsx#L116-L125

_getLatestState will be true on the first render and will be false on the second render. Thus, on the first render, it will render something like:

<ReactReduxContext.Provider ...>
  {children}
</ReactReduxContext.Provider>

On the second render, it will just render the {children}. Because the root of the component changes from ReactReduxContext.Provider to children's root component, whatever is inside {children} will mount twice no matter what.

In my fork, I removed _getLatestState and unconditionally call _renderWithReactReduxContext which works for the first render and also for subsequent renders without a problem.

[navneet-g]

Thanks @anilanar ... @abettadapur can you take a look?

[abettadapur]

@anilanar @shockk Sorry for the delay here. Could you guys try this version of the package and let me know if you still have issues?

[email protected]

[anilanar]

@abettadapur It works. To understand the effects of strictMode, I created the following sandbox with bunch of console.logs, checking state update order of connected components inside and outside DynamicModuleLoader and also assigning refs to DynamicModuleLoader's children from outside.

https://codesandbox.io/s/73r20x4jzj

I think there's a difference only when mounting and not during updates. strictMode changes the order of state updates when mounting. When it's turned off, DynamicModuleLoaders children's connected state updates before anyone else.

[abettadapur]

correct, the only difference should be at mount time. I don't think we can get around this for strict consumers. Thanks for verifying and the example!

@navneet-g Let's move to publish the new version

[navneet-g]

Thanks @anilanar and @abettadapur .
@abettadapur I left a comment on your PR, let me know you want to address before merging or in a separate PR.