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

fix(Store): Handling of @ngrx/store/update-reducers Action on Startup leads to strange behaviour for initialStates #909

Closed
wants to merge 3 commits into from

Conversation

berthertogen
Copy link

  • assign the new state to the overall initial state instead of returning the new state only
  • Add test to simulate the following situation:
    Module MA and MB with initial state IA and IB in module.
    Overall initial state RootIAB { IB } with only state of MB being
    initiated.
    When action '@ngrx/store/update-reducers' is fired for MA the RootIAB should be merged with IA so we don't lose the IB initialization in RootIAB.
  • Fixed failing test to expect 0 instead of undefined.

Closes #906

leads to strange behaviour for initialState
 - assign the new state to the overall initial state instead of
 returning the new state only
 - Add test to simulate the following situation:
 	Module MA and MB with initial state IA and IB in module
	Overall initial state RootIAB { IB } with only state of MB being
	initiated
	When action '@ngrx/store/update-reducers' is fired for MA the
	RootIAB should be merged with IA so we don't lose the IB
	initialization in RootIAB
- Fixed failing test to expect 0 instead of undefined
 with only state of MB being initiated
 When action '@ngrx/store/update-reducers' is fired for MA the RootIAB
 should be merged with IA so we don't lose the IB initialization in
 RootIAB
- Fixed failing test to expect 0 instead of undefined
@coveralls
Copy link

coveralls commented Mar 14, 2018

Coverage Status

Coverage remained the same at 93.533% when pulling 0f31877 on berthertogen:issue-branch-906 into f17a032 on ngrx:master.

@berthertogen berthertogen changed the title #906 Store: Handling of @ngrx/store/update-reducers Action on Startup leads to strange behaviour for initialStates fix(Store): Handling of @ngrx/store/update-reducers Action on Startup leads to strange behaviour for initialStates Mar 14, 2018
@MikeRyanDev
Copy link
Member

If I'm understanding this change correctly it won't be possible to remove state keys by removing reducers, right? I'm not sure if I want to merge in the fix if that's the reprecussion.

The harder fix would be to change the way the Store bootstraps itself. Technically it could load all forFeature() calls that exist in the same injector and resolve them right away as part of the root module. That way you don't get update-reducers calls on bootstrap, only for lazy loaded modules.

@Bretto
Copy link

Bretto commented Jun 11, 2018

What's the status on this ? Should we worry about all the @ngrx/store/update-reducers in the console ? Once the feature modules are loaded we don't get those actions fired multiple times only the first time. Thanks

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

Successfully merging this pull request may close these issues.

4 participants