Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Module is remove, when clicking react router links, inside the same module #79

Open
alfnielsen opened this issue Apr 23, 2019 · 11 comments

Comments

@alfnielsen
Copy link

Hey,
Maybe there is something I dont know of the inner workings,
But when I have a ReactRouter NavLink that point to another page,
within the same module, a '@@Internal/ModuleManager/ModuleRemoved'
is still dispactehed, which remove the data for the module,
and break our site.
When I hard reload the browser the module is again loadet correct.

So it seems that react router links dont really works inside a module ?

It seems like a bug?

Is it posible to turn of 'ModuleRemoved' ?

Redux history:

[
{
  type: '@@router/LOCATION_CHANGE',
  payload: {
    location: {
      pathname: '/contracts/activities',
      search: '',
      hash: '',
      key: '0yjca4'
    },
    action: 'PUSH',
    isFirstRendering: false
  }
},
{
  type: '@@Internal/ModuleManager/ModuleRemoved',
  payload: 'contract'
}
]
@abettadapur
Copy link
Collaborator

Hi, thanks for using our library!

How have you added the 'contract' module to your store? Are you using the DynamicModuleLoader component to add it?

I am not 100% sure about how ReactRouter works, but if you have one route which renders DML with the contract module, and switch to another route that doesn't, the 'contract' module will be removed because the DML component was unmounted.

If you want a module that is present for the entire life of the store, add it as an argument to createStore

@alfnielsen
Copy link
Author

Hi Abettadapur
Yes we are using react-loadable and your DynamicModuleLoader.
We want to load the module only when its needed, but under the contract area of our solution,
and we have multiple end points (urls) that our usrers can end up on, so we need a

 <DynamicModuleLoader modules={[getContractModule()]}>
    (...)
  </DynamicModuleLoader>

for each endpoint, to be sure that the module logic (redux and saga) is loaded.

We dont want all content to be loaded from the start (or life the entire session),
but we need to have different entries into the application under the same module.
And the user can jump between these pages insite one module after the initial page load.

Yes, react-router unload one DynamicModuleLoader but it then load another DynamicModuleLoader
(Both the DynamicModuleLoader having the Contract module),
but there is only a call for RemoveModule when this happens.

(We do have shared modules that are added directly in the createStore)

Do you have any suggestion to how we can achieve this?

Is there another way to use the DynamicModuleLoader where we dont need one for each endpoint,
or can we turn off the RemoveModule
(That would be acceptable, mush better when loading everything up front)

best regards Alf

@navneet-g
Copy link
Contributor

So you want to keep the module in store, when the component that added it to store unmounted?
The only scenario we support is to add it when creating the store. Or you can have a higher level component that is not unmounted when user switches to different route, and that higher level component can add the module to the store.
If you have codesandbox with your example we can fork and talk in terms of concrete code.

@alfnielsen
Copy link
Author

No it's not what we optimal want.
(It is just an acceptable solution to keep the loaded modules in the store after unmount)

Right now the module is removed,
but it is not re-loaded when the new DynamicModuleLoader is mounted ?

Just to understand:
What you say is that if a DynamicModuleLoader is unmounted,
then that modules is never excepted to be loaded again in the same session?
(Or its at least not supported right now?)

I will try to make a codesandbox with the problem, but it will take a little time to create,
so i'll can properble first have it done tomorrow.

@navneet-g
Copy link
Contributor

Thanks for working with us on this, the module should be reloaded if a new DML is mounted with same module.

@alfnielsen
Copy link
Author

alfnielsen commented Apr 25, 2019

Hey,
I made this codesandbox that replicate the problem:
https://codesandbox.io/s/91k27k1rxr

If you have the Redux Chrome plugin in enabled,
you can open the example in full screen mode (https://91k27k1rxr.codesandbox.io/)
and see the redux history.

We can see that this is whats actually happening:

  1. Click Page1
    1. React-router dispath a change location
    1. redux-dynamic-modules dispatch a addModule

The page is loadet correct with data from the store loaded by the module

  1. Click Page2
    1. React-router dispath a change location
    1. redux-dynamic-modules dispatch a removeModule

Now an Error occur telling os that it cannot compile the Page2,
because the store data is not there.

This error is the reason why we missing:
2. 3. redux-dynamic-modules dispatch a addModule

But this means that the Page 2 is compiled before the module is loaded (again)

const DynamicPage2 = () => (
  <DynamicModuleLoader modules={[getModule1()]}>
    <Page2 />
  </DynamicModuleLoader>
);

This could be because rect-loadable somehow tries to compile this DynamicPage2 and Page2,
before the addModule is dispatched (And the module is loaded)!

Or it means that DynamicModuleLoader is compiling (call render) before the module is loaded!

Is this a totally wrong approach to use your DynamicModuleLoader?

How would you normally do the lazy loading of components?
Would you have a Lazy/Loadable inside your DynamicModuleLoader as well?

(I know you maybe not using rect-loadable but some other lazy load module)

I will take a look at react-loadbale source code, but it does not make sense that it would not deletage the react 'children' compiling to the root react component (In this case DynamicModuleLoader)

@navneet-g
Copy link
Contributor

navneet-g commented Apr 25, 2019

I see a different issue here, Seems like store do not have state when the first module is unloading, note mapStatesToProps is failing on Unmount of the first page (In this repro I first visited Page2). I need to spend more time to debug and see why the store do not have right state.

Uncaught TypeError: Cannot read property 'testValue' of undefined
at Function.mapStateToProps [as mapToProps] (Page2.tsx? [sm]:10)
at proxy (wrapMapToProps.js:53)
at handleNewState (selectorFactory.js:56)
at handleSubsequentCalls (selectorFactory.js:70)
at eval (selectorFactory.js:75)
at Subscription.checkForUpdates [as onStateChange] (connectAdvanced.js:256)
at Subscription._proto.handleChangeWrapper (Subscription.js:76)
at Object.dispatch (redux.js:225)
at e (:1:40553)
at Object.eval [as dispatch] (index.js:42)
at dispatch (:1:28545)
at Array.forEach ()
at _dispatchActions (ModuleManager.js:25)
at eval (ModuleManager.js:157)
at Array.forEach ()
at Object.remove (ModuleManager.js:136)
at eval (RefCountedManager.js:62)
at Array.forEach ()
at Object.ret.remove (RefCountedManager.js:57)
at Object.remove (ModuleStore.js:51)
at DynamicModuleLoaderImpl.componentWillUnmount (DynamicModuleLoader.js:189)

@RavilM
Copy link

RavilM commented Dec 4, 2019

Hello!
@navneet-g Have you any news?

@navneet-g
Copy link
Contributor

@RavilM I did not get time to further debug this.

@RavilM
Copy link

RavilM commented Mar 2, 2020

@abettadapur @navneet-g
Hi guys! Have you got any news about this issue or code review of my solution?

@FraserKillip
Copy link
Contributor

I am having this exact same issue. My problem is as follows.

2 pages:
Page A on route admin/a requires [module1]
Page B on route admin/b requires [module1, module2]

Go to Page A then click a button which dispatches push and navigates the user to Page B. Upon navigation to Page B, the page renders fine but DML does not load the required modules.

Expected result:
module1 is removed (because the page A DML was removed) then modules module1 AND module2 gets loaded by the new DML component.

Actual Result:
Page navigation occurs and neither module1 is remove nor module1 and module2 is added.

Symptom
Error in react saying trying to read value of undefined. This is because the selector is trying to read from the state but it has not been loaded/defined.
The example linked here is the exact issue, it just presents as another bug.

Issue
It looks like React is not disposing on the DML component and instead re-using it for the new page. It appears that the props are updating which is causing the render to occur.

Workaround
Add the key prop to your DML instances with unique values to to prevent react from reusing the component instance.

<DynamicModuleLoader key='loader1' modules={[myModuleDefinition]}>
  <MyChildComponent />
</DynamicModuleLoader>

cc @alfnielsen @navneet-g @RavilM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants