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

where should i put my enhancers? #24

Closed
geminiyellow opened this issue Nov 1, 2018 · 16 comments
Closed

where should i put my enhancers? #24

geminiyellow opened this issue Nov 1, 2018 · 16 comments
Assignees
Labels
How to Issues that ask How to do something in redux-dynamic-modules

Comments

@geminiyellow
Copy link

geminiyellow commented Nov 1, 2018

hi all, i use redux's createStore and compose like this

const enhancers = [firebaseEnhancer, middlewareEnhancer, offlineEnhancer];
const createEnhanceStore = compose(...enhancers)(createStore);

export const createReduxStore = (initialState) => createEnhanceStore(makeRootReducer(), initialState);

and in redux-dynamic-modules , i check the code, i can put middleware in a new extension like redux-dynamic-modules-thunk , but where i should put the other enhancers?

the ModuleStore only compose middleware ,

https://github.com/Microsoft/redux-dynamic-modules/blob/10e7526f037dba0f1260a913f241504ea41f51ad/packages/redux-dynamic-modules/src/ModuleStore.ts#L43

am i wrong ?

@navneet-g
Copy link
Contributor

Thanks for the question @geminiyellow , it is a really good one. We will push an update to allow this scenario, please give us a day or two.

@geminiyellow
Copy link
Author

geminiyellow commented Nov 1, 2018

woo, thank you @navneet-g . ok , two ; ) .

@navneet-g
Copy link
Contributor

Just a quick update, I have started a small refacotr/redesign to better support it, the changes are in following branch, I need to test them out and write some test harness. I hope to get some time over the weekend to finish it off, but no promise :)
Please check it out if you are interested.

https://github.com/Microsoft/redux-dynamic-modules/blob/users/navneetg/moduleenhancer/packages/redux-dynamic-modules/src/ModuleEnhancer.ts

@navneet-g navneet-g self-assigned this Nov 3, 2018
@navneet-g
Copy link
Contributor

@geminiyellow I have a PR out to support enhancers. Thanks for requesting this feature it helped me improve the design.
#25

To see how you can use it see the unit test where I am using moduleEnhancer with applyMiddleware enhancer
https://github.com/Microsoft/redux-dynamic-modules/pull/25/files#diff-c5cc69a8f5bebda3075b38d7f75aa7ed

@geminiyellow
Copy link
Author

woo, thank you @navneet-g

@abettadapur
Copy link
Collaborator

@geminiyellow Published v2.0.0, which contains this new fix

@navneet-g
Copy link
Contributor

@geminiyellow did you get a chance to use v2.0.0? Please let us know if you have any feedback.

@geminiyellow
Copy link
Author

geminiyellow commented Nov 8, 2018

hi @abettadapur , @navneet-g , sorry i have no time to test it yesterday.
i read the PR code, knew that moduleEnhancer is a enhancer method,
but looks like that i cannot use it directly, and what i can use is the createStore.

hmm, the configureStore is changed to createStore,
name is same as the redux#createStore but interface is changed,
and it cannot use withredux#compose,

i dont think that can help i understand how to use it.
when i got the name, i will try to use it as the origin createStore, but i cannot, right?
could you give me some code show how to use the moduleEnhancer ? and how to add others enhancers to the moduleStore ?

@navneet-g
Copy link
Contributor

navneet-g commented Nov 8, 2018

@geminiyellow I just published a new version (2.0.1)

  1. Pull the new version
  2. import module enhancer import { moduleEnhancer } from "redux-dynamic-modules";
  3. use createStore from redux, and the composed enhancers
createStore(rootRecuer, initialState, compose(moduleEnhancer(), applyMiddleware(...))); 

@geminiyellow
Copy link
Author

@navneet-g good, thank you. let me try it.

@navneet-g
Copy link
Contributor

@geminiyellow I have also updated widgets example which now uses moduleEnhancer
https://github.com/Microsoft/redux-dynamic-modules/blob/master/packages/widgets-example/src/App.js

@geminiyellow
Copy link
Author

@navneet-g yes, i notice that, ok, here is my code


const dynamicModuleEnhancer = moduleEnhancer();
const enhancers = [firebaseEnhancer, middlewareEnhancer, offlineEnhancer, dynamicModuleEnhancer];
const createEnhanceStore = compose(...enhancers)(createStore);

export const createReduxStore = initialState => createEnhanceStore(makeRootReducer(), initialState);

and i copy some code from hacker-news modules

import React from 'react';
import { Text, View } from 'react-native';
import { DynamicModuleLoader } from "redux-dynamic-modules";

export const weatherState = state => (state || {});
const getHackerNewsModule = () => ({
  id: "weather",
  reducerMap: { weatherState },
});

const EntranceScreen = () => (
  <DynamicModuleLoader modules={[getHackerNewsModule()]}>
    <View><Text>123</Text></View>
  </DynamicModuleLoader>
);

export default EntranceScreen;

and put it in react-navigation.

but when i start up it

Unexpected keys "offline", "HOME", "FIREBASE", "FIRESTORE" found in previous state received by the reducer. Expected to find one of the known reducer keys instead: "weatherState". Unexpected keys will be ignored.

@navneet-g
Copy link
Contributor

@geminiyellow This error means that in the store there are some keys that do not have any associated reducers. Do you know how the reducers for offline, home, firebase and firestore are assigned? may be the enhancers for those middlewares provider the reducers. I suggest you try by changing the order for enhancers
e.g. const enhancers = [dynamicModuleEnhancer, firebaseEnhancer, middlewareEnhancer, offlineEnhancer];

if you can create a minimal repro and provide me a gist to link to a github repo I can debug as well.

@ghost
Copy link

ghost commented Nov 8, 2018

@geminiyellow I was able to repro this and my analysis was correct above.
Please move the dynamicModuleEnhancer as the left most enhancer you use in compose so other enhancers get a chance to register their keys.

e.g. const enhancers = [dynamicModuleEnhancer, firebaseEnhancer, middlewareEnhancer, offlineEnhancer];

@ghost
Copy link

ghost commented Nov 8, 2018

I have also updated the widgets-example to illustrate the same and used offlineEnhancer as an example, please take a look here
https://github.com/Microsoft/redux-dynamic-modules/blob/master/packages/widgets-example/src/App.js

@geminiyellow
Copy link
Author

@navneet-g yes, i had try to change the order before comment.
@navneetg-msft got, but the error still here.

we can recreate it use the latest widgets-example

first , let us copy the code from redux official doc here: https://redux.js.org/api/combinereducers#example

then add something like this:

+ const makeReducers = () => combineReducers({
+   todos,
+   counter
+ })

this.store = createStore(
+   makeReducers(),
-   (state, action) => state || {}, 

and

error

@navneet-g navneet-g reopened this Nov 9, 2018
@navneet-g navneet-g added the How to Issues that ask How to do something in redux-dynamic-modules label Dec 21, 2018
@navneet-g navneet-g pinned this issue Dec 21, 2018
@navneet-g navneet-g unpinned this issue Dec 21, 2018
@navneet-g navneet-g pinned this issue Dec 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
How to Issues that ask How to do something in redux-dynamic-modules
Projects
None yet
Development

No branches or pull requests

3 participants