Skip to content
This repository has been archived by the owner on Jan 10, 2018. It is now read-only.

Provide multiple reducers from multiple NgModules #211

Closed
benneq opened this issue Sep 11, 2016 · 24 comments
Closed

Provide multiple reducers from multiple NgModules #211

benneq opened this issue Sep 11, 2016 · 24 comments

Comments

@benneq
Copy link

benneq commented Sep 11, 2016

I'm wondering, if there's any built-in functionality to register additional reducers.

Use Case:
My application has multiple modules. Each module has its own NgModule declaration. And some of these modules use the Store.

I now created my own Store module, which provides a single ngrx/store and then collects all my module stores via multi provider and then uses combineReducers to register them. This works fine. But I guess this is a pretty common scenario and it would be nice if ngrx/store would support this out of the box somehow.

Here's what I do at the moment:

export const STORE_PROVIDER_TOKEN = new OpaqueToken('StoreProvider');

export abstract class StoreProvider<T> {
    abstract name(): string;
    abstract reducer(state: T, action: Action): T;
}

@NgModule({
  imports: [ ngrxStoreModule.provideStore({}) ],
  providers: [
    {
      provide: INITIAL_REDUCER,
      deps: [ STORE_PROVIDER_TOKEN ],
      useFactory: function(providers:StoreProvider<any>[]) {
        var obj = providers.reduce((o, provider) => {
          o[provider.name()] = provider.reducer;
          return o;
        }, {});
        return combineReducers(obj);
      }
    }
  ]
})
export class MyStoreModule {}

Then if some module needs a store:

class MyStoreProvider extends StoreProvider<number> {
    name() { return "counter"; }

    reducer(state: number = 0, action: Action) {
      switch (action.type) {
        // ...
      }
    }
}

@NgModule({
  providers: [ {
    provide: STORE_PROVIDER_TOKEN,
    useClass: MyStoreProvider,
    multi: true
  } ]
})
export class MyModule {}

It's basically just an inversion of what your Usage Example ( https://github.com/ngrx/store#usage ) does.

Is there already something like this built in ngrx/store?

@robwormald
Copy link
Contributor

There isn't yet, but this is definitely something I want to implement.

Sketched in my head I have something like

//app.module.ts

imports: [
  StoreModule.provideRootStore({foos, bars})
]

//feature module
imports: [
  StoreModule.addReducers({ bazes })
]

This is reasonably straightforward to do if you're passing ngrx/store an object of your reducers, and significantly more difficult if you're combining reducers yourself.

Entirely open to suggestions

@benneq
Copy link
Author

benneq commented Sep 17, 2016

Yeah, this would be super useful in terms of modularity. So each module can be self contained and can be (re)moved easily.

Some things to consider:

  • What about Initial State? (I don't use it myself, but...) I think it should be more like addStore({ reducers }, { initialStates })
  • What about providing multiple reducers/states with the same name? I could imagine, that reducers will just be appended to the existing ones. Maybe provide some boolean to override existing reducers for the same key? Or just throw an exception? Or just let the user handle this situation (there could be some use case for this!?)

@littleStudent
Copy link

the reducer/state naming is a good point. If I import a feature module it would be nice if its not necessary to think about naming. Maybe at some point there are modules out there where I do not have control over the naming?

Over all I really like this idea. Would leverage the module system much more.

@benneq
Copy link
Author

benneq commented Sep 17, 2016

But you should also consider that you have some ContactModule and some EnhancedContactModule which extends the ContactModule and uses its store. Then it must be possible to use the same store from within both modules. Or maybe it should even be possible to override and/or extend the reducers that were given by the ContactModule. There are many possibilities to consider.

@robwormald
Copy link
Contributor

I've been tinkering with this today, and haven't found a decent way to make this work for both modes (pass an object and pass a combinedReducer), since the latter form means I never have an object of reducers I can augment or update.

I'd really like to hear use cases of why people are using combineReducers themselves, and see if there's an alternate way to handle this. cc @MikeRyan52

Collisions are definitely possible. We could switch to using a Map (rather than an object) internally, which would let us use classes or Symbols as keys, but again, is a bit breaky on the API surface.

@benneq
Copy link
Author

benneq commented Sep 19, 2016

Could you elaborate on "pass an object and pass a combinedReducer"? I don't understand that.

The reason will (probably) always be: Inversion of Control and clean modules

With the current version there has to be a global store configuration where you include and combine all reducers for the application. I think that's a bad architecture design in terms of modularization. Especially if there's an depencendy injection framework available.

Using IoC you could do something like npm install my-ng2-module-that-uses-a-ngrx-store, then include it within your app's @NgModule and you're done. Without IoC you'd have to also modify your global store config. This will make the code more complex.

@alexciesielski
Copy link

alexciesielski commented Sep 19, 2016

I've started to embrace the move to ng-modules and I like how organized everything becomes thanks to that. I have refactored my app to make use of modules lazy loading, unfortunately I don't know how to do the same with reducers/actions/effects.

Ideally I would like to be able to move these services into the corresponding module folders and have them lazy load together with the rest (just declare them in that module with .forChild() or something similar).

My app has the following usecase (as probably most apps have):
LoginModule to provide login functionality and later DashboardModule for authorized users with additional functionality/pages.

@KwintenP
Copy link

KwintenP commented Sep 21, 2016

I've been playing with this and I came up with the following. (jsbin: http://jsbin.com/junini/38/edit?js,console)

If you pass an object (or a combined reducer) to your store, you'll reducer tree will look like this.
image

Let's say you want to add something to this reducer tree. Instead of thinking about this as passing an extra reducer to the current reducer tree, you could think of it as creating a separate tree for the thing you are adding and combine the result of those two trees. Conceptually speaking this would look like this.

image

This way, you do not really need to know anything about the previous reducers and it doesn't matter if it was created by calling the combineReducers method directly or inside the store.

The 'only' thing that needs to be added to implement this is another meta-reducer called enhanceReducers (or something similar). What it will do, when called, is slice up the state tree and pass the part of the tree that belongs to the first reducer to the first, and the second to the second. The results are then again combined into a single object and returned.

To know what part of the state tree needs to be passed to which reducer, it could, at creation time, send a dummy action to the current reducer and the new reducer. These will both return objects from which you can get the keys. These keys can later be used to slice up the current state object into the correct two slices to pass them to both reducers.

To make this easier to understand. Let's divide the steps for the second image.

  • a method is called with the currentReducer (top combinedReducer) and the new one (bottomCombinedReducer), from lazy loading f.e.
  • the method will send an action 'ENHANCE_REDUCERS' to both reducers
  • the first will return {ui: {}, data: {}}, the second {extra: []}
  • the method will extract the keys, the first ["ui","data"], the second ["extra"]
  • the method will return the enhanceReducer

Let's say an action is dispatched to the enhancedReducer:

  • the enhancedReducer will take the current state object passed and, using the keys, slice it up into two objects. One having the 'ui' and 'data' property, one having the 'extra' property.
  • It will call the firstReducer with the 'ui-data' object and the second with the 'extra' object
  • It will Object.assign the result of both calls onto the same object
  • it will return that object

The jsbin example implements a first draft of this enhancedReducer as a proof of concept.

Advantages:

  • You can enhance the reducer tree with new methods
  • It doesn't matter if they called combinedReducers themselves or not
  • You could throw an error if you have keys which are colliding for the old and new reducer! This warns the user of naming collisions at development time.

Disadvantages:

  • You do not rule out name collisions, you can only warn about them.

Hope this makes some sense :). Let me know what you think...

Edit: The jsbin example works with a todos- and usersReducer. These do not match the images above.

@emilianosantucci
Copy link

@robwormald your proposal is really easy and flexible.

@KwintenP had a right workaround approach, I've implemented a similar one:

// app.module.ts
import { ActionReducer, combineReducers, StoreModule } from '@ngrx/store';
import { appRoutes } from './app.routes.ts';
// imports

@NgModule({
    StoreModule.provideStore(AppModule.reducers()),
    // other stuff
}
export class AppModule {
    static reducers(): ActionReducer<any> {
        return combineReducers(Object.assign({},
            appReducer,
            {router: routerReducer},
            appRoutes.filter(route => (route.data && route.data['reducers'])).map(route => route.data['reducers']).pop()
        ));
    }
}

// app.routes.ts
import { Routes } from '@angular/router';
import HomeModule from '../+home/home.module';

export const appRoutes: Routes = [
    { path: '', redirectTo: '/home', pathMatch: 'full' },
    {
        path: 'home',
        loadChildren: (() => new Promise((resolve, reject) => {
            return resolve(require('../+home/home.module').default)
        })),
        data: {
            reducers: HomeModule.reducers()
        }
    }
];

// home.module.ts
import { ActionReducer } from '@ngrx/store';
import { HomeComponent } from './home.component';
// imports

@NgModule({
    // stuff
})
export default class HomeModule {
    static reducers(): {[key: string]: ActionReducer<any>} {
        return {home: homeReducer};
    }
}

Combine modules with routing allow to import childs modules once.

@tomwanzek
Copy link

First of all, thanks for ngrx 👍

Just wondering, is there an update on the go-forward/proposed pattern for handling ngrx in a multi-module setting with lazy-loading?

I tend to recall, I came across a talk by @robwormald recorded some time earlier this year, where this question came up. I wish my memory served me better as to the when & where, but I think there was chatter of still looking into the necessary changes "without breaking too much" for the installed base...

Thanks in advance.

@robwormald
Copy link
Contributor

Update on this: @MikeRyan52 has an implementation that LGTM, so we'll go with that

@MikeRyanDev
Copy link
Member

Here is a brief overview of the changes we will be making to @ngrx/store: https://gist.github.com/MikeRyan52/5d361681ed0c81e38775dd2db15ae202

@eXaminator
Copy link

As I don't see a mention of this in the document: Please keep lazy loaded modules in mind. :)

@snarun
Copy link

snarun commented Feb 6, 2017

It works, but does it compromises the chunking of modules? How a module can be lazily loaded in the form of chunks with combineReduce option. I am using webpack for chunking.

@alaycock
Copy link

alaycock commented Feb 6, 2017

@arunsn43 You could use something like the createReducer in this comment #197 (comment)

In your lazy loaded module you would use createReducer, while passing in your additional lazy-loaded reducers, then use replaceReducer to properly merge it with the existing store.

@robwormald
Copy link
Contributor

Just an update on this, I'm back onto working on this API. Chatted w/ @victorsavkin a bit and we have a slightly tweaked implementation of Mike's PR that's a little more in keeping with how Angular's DI works - closer to how @KwintenP is describing it above.

This would handle lazy loading of reducers in the same fashion the router handles modules, and keeps the same semantics as far as visibility.

@snarun
Copy link

snarun commented Feb 8, 2017

@alaycock / @robwormald Thank you! I've used the similar approach. It worked and now the modules are build and loaded in chunks.

Also, added a method to remove the reducers as well as the state, on a need basis. But since the loaded angular module cannot be unloaded (unloading a lazily loaded module) , wondering what is the best option to fit this? (Components can be destroyed but not the angular module). My take is to associate with ngOnDestroy() of Angular module! Any thoughts??

@BrainCrumbz
Copy link

I'd really like to hear use cases of why people are using combineReducers themselves

Maybe late to the party, anyway if it might help: currently we call combineReducers in some places:

  • within a sub-module, we have the state for that module composed of two separate objects (think e.g. data and ui, or remoteState and localState). For each of those parts we have separate reducers, hence to build a reducer for the sub-module we call combineReducers. Something like:
const fooModuleReducer: ActionReducer<FooModuleState> = combineReducers({
  remote: fooRemoteReducer,
  local: fooLocalReducer,
});
  • within the root application module, we import all sub-module reducers (like the one mentioned above) and call combineReducers to create the root app reducer. Something like:
const rootReducerSet = {
  foo: fooModuleReducer,
  bar: barModuleReducer,
  baz: bazModuleReducer,
};

const rootReducer: ActionReducer<RootState> = combineReducers(originalRootReducers);
  • within a lazy-loaded sub-model, beside what already mentioned, we also call combineReducers to build the new augmented root reducer that will replace the original one for currently lazy loaded module. Something like:
// same as non lazy-loaded module
const lazyModuleReducer: ActionReducer<LazyModuleState> = combineReducers({
  remote: lazyRemoteReducer,
  local: lazyLocalReducer,
});

const rootReducerPlusLazy: ActionReducer<RootStatePlusLazy> = combineReducers({
  ...rootReducerSet,
  lazy: lazyModuleReducer,
});

@kylecordes
Copy link
Contributor

@robwormald I'm also late to the party. Here is my use case for calling combineReducers in app code, rather than passing the object full of reduces to Store for it to combine: when coding as shown in the README of store-freeze:

https://github.com/codewareio/ngrx-store-freeze

(Side note - it sure would help the ergonomics of new developers adopting Store, if freezing was in the box and on by default, rather than being something to learn of eventually by looking around.)

@thecaddy
Copy link

Is there some kind of time table for when this feature will be released? I see this conversation started September of last year. As our app has grown keeping all the reducers in the root module has been problematic.

@robwormald
Copy link
Contributor

We're targeting July 1 for launch. In the meantime, see https://twitter.com/MikeRyanDev/status/856349359805485056 - we're publishing nightlies already if you want to try it out. Development will be moving (has moved) to the https://github.com/ngrx/platform/ repo

@thecaddy
Copy link

@robwormald great to hear, thanks@!

@csutorasr
Copy link

@robwormald
Hi!

Any info about a later release date? I have just started a new project and I would like to use the new ngrx. (I find it hard to code in the old one, and the examples of the new is really good in modularity.) I am asking beacuse if you could finish them e.g. within a week then I would use that instead of refactoring the code in 2 months.

@robwormald
Copy link
Contributor

This is supported and implemented in v4 - https://github.com/ngrx/platform. Thanks!

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

No branches or pull requests