Skip to content

Commit

Permalink
fix(store): duplication of deferred states (#888)
Browse files Browse the repository at this point in the history
  • Loading branch information
splincode authored Mar 4, 2019

Unverified

This user has not yet uploaded their public signing key.
1 parent 465e379 commit 103604a
Showing 3 changed files with 112 additions and 25 deletions.
52 changes: 31 additions & 21 deletions packages/store/src/internal/state-factory.ts
Original file line number Diff line number Diff line change
@@ -84,17 +84,22 @@ export class StateFactory {
return value;
}

private static checkStatesAreValid(stateClasses: StateClass[]): void {
stateClasses.forEach(StoreValidators.getValidStateMeta);
}

/**
* Add a new state to the global defs.
*/
add(stateClasses: StateClass[]): MappedStore[] {
this.checkStatesAreValid(stateClasses);
this.checkForDuplicateStateNames(stateClasses);
StateFactory.checkStatesAreValid(stateClasses);
const { newStates } = this.addToStatesMap(stateClasses);
if (!newStates.length) return [];

const stateGraph: StateKeyGraph = buildGraph(stateClasses);
const stateGraph: StateKeyGraph = buildGraph(newStates);
const sortedStates: string[] = topologicalSort(stateGraph);
const depths: ObjectKeyMap<string> = findFullParentPath(stateGraph);
const nameGraph: ObjectKeyMap<StateClass> = nameToState(stateClasses);
const nameGraph: ObjectKeyMap<StateClass> = nameToState(newStates);
const bootstrappedStores: MappedStore[] = [];

for (const name of sortedStates) {
@@ -115,7 +120,7 @@ export class StateFactory {
// ensure our store hasn't already been added
// but don't throw since it could be lazy
// loaded from different paths
if (this.hasBeenMounted(name, depth)) {
if (!this.hasBeenMountedAndBootstrapped(name, depth)) {
bootstrappedStores.push(stateMap);
}

@@ -125,24 +130,13 @@ export class StateFactory {
return bootstrappedStores;
}

private checkForDuplicateStateNames(stateClasses: StateClass[]) {
for (const stateClass of stateClasses) {
const stateName = StoreValidators.checkStateNameIsUnique(stateClass, this.statesByName);
this.statesByName[stateName] = stateClass;
}
}

private checkStatesAreValid(stateClasses: StateClass[]) {
stateClasses.forEach(StoreValidators.getValidStateMeta);
}

/**
* Add a set of states to the store and return the defaults
*/
addAndReturnDefaults(stateClasses: StateClass[]): StatesAndDefaults {
const classes: StateClass[] = stateClasses || [];

const states = this.add(classes);
const states: MappedStore[] = this.add(classes);
const defaults = states.reduce(
(result: any, meta: MappedStore) => setValue(result, meta.depth, meta.defaults),
{}
@@ -218,6 +212,22 @@ export class StateFactory {
return forkJoin(results);
}

private addToStatesMap(stateClasses: StateClass[]): { newStates: StateClass[] } {
const newStates: StateClass[] = [];
const statesMap: StatesByName = this.statesByName;

for (const stateClass of stateClasses) {
const stateName: string = StoreValidators.checkStateNameIsUnique(stateClass, statesMap);
const unmountedState: boolean = !statesMap[stateName];
if (unmountedState) {
newStates.push(stateClass);
statesMap[stateName] = stateClass;
}
}

return { newStates };
}

private addRuntimeInfoToMeta(meta: MetaDataModel, depth: string): void {
meta.path = depth;
meta.selectFromAppState = propGetter(depth.split('.'), this._config);
@@ -228,10 +238,10 @@ export class StateFactory {
* the method checks if the state has already been added to the tree
* and completed the life cycle
* @param name
* @param depth
* @param path
*/
private hasBeenMounted(name: string, depth: string): boolean {
const valueIsBootstrapped: boolean = getValue(this.stateTreeRef, depth) !== undefined;
return !(this.statesByName[name] && valueIsBootstrapped);
private hasBeenMountedAndBootstrapped(name: string, path: string): boolean {
const valueIsBootstrapped: boolean = getValue(this.stateTreeRef, path) !== undefined;
return this.statesByName[name] && valueIsBootstrapped;
}
}
9 changes: 5 additions & 4 deletions packages/store/src/module.ts
Original file line number Diff line number Diff line change
@@ -78,10 +78,11 @@ export class NgxsFeatureModule {
// add stores to the state graph and return their defaults
const results = factory.addAndReturnDefaults(flattenedStates);

internalStateOperations.setStateToTheCurrentWithNew(results);

// dispatch the update action and invoke init and bootstrap functions after
lifecycleStateManager.ngxsBootstrap(new UpdateState(), results);
if (results.states.length) {
internalStateOperations.setStateToTheCurrentWithNew(results);
// dispatch the update action and invoke init and bootstrap functions after
lifecycleStateManager.ngxsBootstrap(new UpdateState(), results);
}
}
}

76 changes: 76 additions & 0 deletions packages/store/tests/lazy-load-late-init.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import { NgModule, NgModuleFactoryLoader } from '@angular/core';
import { CommonModule } from '@angular/common';
import { fakeAsync, TestBed, tick } from '@angular/core/testing';
import { RouterTestingModule, SpyNgModuleFactoryLoader } from '@angular/router/testing';
import { Router } from '@angular/router';

import { Action, NgxsModule, State, StateContext, Store } from '../src/public_api';

describe('Lazy loading with duplicate bootstrap states', () => {
let store: Store;
let router: Router;
let loader: SpyNgModuleFactoryLoader;

class CounterAction {
public static type = 'increment';
}

@State<number>({
name: 'counter',
defaults: 0
})
class CounterValueState {
@Action(CounterAction)
openAlert({ setState }: StateContext<number>): void {
setState((state: number) => ++state);
}
}

@NgModule({ imports: [CommonModule, NgxsModule.forFeature([CounterValueState])] })
class LazyModuleA {}

@NgModule({ imports: [CommonModule, NgxsModule.forFeature([CounterValueState])] })
class LazyModuleB {}

@NgModule({ imports: [CommonModule, NgxsModule.forFeature([CounterValueState])] })
class LazyModuleC {}

beforeEach(() => {
TestBed.configureTestingModule({
imports: [RouterTestingModule, NgxsModule.forRoot([])]
});
});

beforeEach(() => {
store = TestBed.get(Store);
router = TestBed.get(Router);
loader = TestBed.get(NgModuleFactoryLoader);
loader.stubbedModules = {
lazyModuleA: LazyModuleA,
lazyModuleB: LazyModuleB,
lazyModuleC: LazyModuleC
};

router.resetConfig([
{ path: 'pathA', loadChildren: 'lazyModuleA' },
{ path: 'pathB', loadChildren: 'lazyModuleB' },
{ path: 'pathC', loadChildren: 'lazyModuleC' }
]);
});

it('should be correct initial lazy state', fakeAsync(async () => {
await router.navigateByUrl('/pathA');
store.dispatch(new CounterAction());
tick(1000);

await router.navigateByUrl('/pathB');
store.dispatch(new CounterAction());
tick(1000);

await router.navigateByUrl('/pathC');
store.dispatch(new CounterAction());
tick(1000);

expect(store.snapshot()).toEqual({ counter: 3 });
}));
});

0 comments on commit 103604a

Please sign in to comment.