Open jared-dykstra opened 5 years ago
We don't have any HMR support right now, but it is something that we could invest in. For your other questions, where does the module system fall short in accomplishing these goals?
where does the module system fall short
I don't think it does. I opened this PR to ensure that I'm using it correctly and making the most of it--I wasn't sure. I figured, if I had missed something, if nothing else I could PR against the documentation.
So far, I'm quite impressed with the library and using it in a CRA v2 project.
For example, I was expecting to have to setup Redux Devtools as an extra step, but was surprised to discover that it "just worked"
With regards to replaceReducer
, we do not expose that function. Instead, you can use modules to add/remove reducers.
Are you asking for a function like replaceModule
, which could replace a module at runtime? We dont have that today, but it is something we could potentially add to support hot reloading scenarios
Here's a concise description of the problem I'm seeing. I suspect this is user-error (an error in my code) I currently have this code in place for HMR, and my app is using react-router:
renderRootNode(() => <App {...props} />)
if (process.env.NODE_ENV !== 'production') {
if (module.hot) {
module.hot.accept('./App', () => {
// eslint-disable-next-line no-console
console.warn('[HMR] - Updating App')
// eslint-disable-next-line global-require
const NextApp = require('./App').default
renderRootNode(() => <NextApp {...props} />)
})
}
}
When I make a change, HMR kicks in. But then I notice two dispatched redux actions:
@@router/LOCATION_CHANGE
- I don't expect this to fire, but it does. The location is the same as it was before, and the state diff seems inconsequential. I think I can ignore this event.@@Internal/ModuleManager/ModuleRemoved
- Wipes out all state corresponding to this module@@Internal/ModuleManager/ModuleAdded
- Sets up the same module again from its initial stateOf course, this only affects development, but I'd love to avoid losing the module's state every time a development change is made. If some changes need to be made via a PR, I'm happy to look into it, but am not sure where to begin.
So, <App />
uses react-router, which eventually renders <Page />
which looks like this:
Page.js:
import React, { Suspense } from 'react'
const PageDynamic = React.lazy(async () =>
import(/* webpackChunkName: 'PageDynamic' */ './PageDynamic')
)
<Suspense fallback={...}>
<PageDynamic />
</Suspense>
PageDynamic.js:
import { DynamicModuleLoader } from 'redux-dynamic-modules'
import { module } from 'reduxStore/myModule'
import RealPage from './RealPage'
const PageDynamic = props => (
<DynamicModuleLoader modules={[module()]}>
<RealPage {...props} />
</DynamicModuleLoader>
)
Lastly, <RealPage />
is a regular react component, rendering the state of reduxStore/myModule
HMR would be killer for this project. 👍
The way I see it, there are a few things to do:
module.hot.accept()
added to it, listening for changes in it's own dependencies. Traditional use of module.hot.accept('./something')
relies on a full chain of dependencies from ./something
-- But that would create a dependency chain and defeat the purpose of this redux-dynamic-modulessaveState()
, createStore(prevState)
, restartSagas()
... Or, use of redux's replaceReducers()
to avoid creating a new store. I'm not sure which option is best. Terminating and restarting sagas could lead to some indeterminism, but should be ok? Ideally, this process would apply to each module individually to have a minimal effect.To be clear: I think this library is fantastic. I'm totally willing to help out with a PR, but would love to know what approach you would suggest.
Can you offer any advice?
Hi @abettadapur and @jared-dykstra, any update on this yet?
@jared-dykstra I can take a look, where do you see code for option 1 or 2 being called? I am not familiar with how runtime detects that code is being hot reloaded, please educate me a bit more.
@jared-dykstra ... so I did some reading, Option 2 above seems better to me, for the reasons you mentioned and it is very explicit. Ideally the ModuleManager should handle replacing reducer and extensions/middlewares/sagas. We need to find a way so ModuleManager can be notified that files belonging to a rdm-module is being reloaded.
Another alternative is that we detect hot-reloading in progress globally, and maintain the state and refresh artifacts related to all rdm-modules currently in store. We still need to consider a scenario where props for DynamicModuleLoader itself changes, not sure how will we handle it.
Any suggestions?
module : module in terms of a javascript module/file. rdm-module: A module as defined by redux-dynamic-modules.
Ok ... further reading, this is what we can do. 1) When a module detects it is hot reloaded, it dispatches a global event. 2) Someone listens to such events, gets the moduleid and module contents from the event and asks the store to replace the module while retaining the state and not dispatching initial/final actions
If you use react hot loader like me, there is a workaround that could help you.
function yourComponent() {
const _component = <DynamicModuleLoader strictMode={true} modules={[getHackerNewsModule()]}>
<ConnectedHackerNews />
</DynamicModuleLoader>
return module.hot && module.hot.active ? _component : _component
}
There are two important options to help react hot loader work properly in common cases:
module.hot.active
can make sure DynamicModuleLoader get updated rdm module not old onestrictMode
to ensures that DynamicModuleLoader removes rdm module before adds rdm modulestrictMode
default value is undefined, so DynamicModuleLoader will add rdm-module in constructor before it cleans up previous rdm-module, this behaviour could cause rdm-module not working as it should be reload. Because it follows this rule Module Reference Counting
module.hot.active
implies the chunk was updated, you can use new code, but before it is available,
you react component probably be invoked by react-hot-loader many times.
Two questions:
replaceReducer()
supported by the store, and is there a way to substitute a saga?