From 3ecc3b45ce311116fd7cfd794688e571fe0e9008 Mon Sep 17 00:00:00 2001 From: Lukas Kurz Date: Sun, 22 Nov 2020 11:23:01 +0100 Subject: [PATCH] fix(store): merge options with default options (#1686) * test: add selector that uses container injection * test: add text for missing container injection * feat: add option merging to ngxsmodule * chore: remove debugging code * test: add launch option for jest * test: fix selectoroptions in test * chore: remove comment * chore: remove useless var * test: add launch config for jest current file * chore: add more documentation to code * test: add tests for mergeDeep * fix: change rest parameter type * chore: change bundlesize to fit changes --- .vscode/launch.json | 41 +++++++++++- bundlesize.config.json | 4 +- integration/app/app.component.html | 1 + integration/app/app.component.ts | 2 + integration/app/store/store.module.ts | 5 +- integration/app/store/todos/todos.state.ts | 9 +++ packages/store/src/module.ts | 3 +- packages/store/src/utils/utils.ts | 37 +++++++++++ packages/store/tests/selector.spec.ts | 2 +- packages/store/tests/utils/utils.spec.ts | 72 +++++++++++++++++++++- 10 files changed, 169 insertions(+), 7 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 569a75148..6d51274a2 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -12,6 +12,45 @@ "outFiles": [ "${workspaceFolder}/**/*.js" ] + }, + { + "type": "node", + "request": "launch", + "name": "Jest All", + "program": "${workspaceFolder}/node_modules/cross-env/dist/bin/cross-env.js", + "cwd": "${workspaceFolder}", + "args": [ + "CI=true", + "ng", + "test", + "--project", + "ngxs", + "--colors", + "--runInBand" + ], + "console": "integratedTerminal", + "internalConsoleOptions": "neverOpen", + "disableOptimisticBPs": true, + }, + { + "type": "node", + "request": "launch", + "name": "Jest Current File", + "program": "${workspaceFolder}/node_modules/cross-env/dist/bin/cross-env.js", + "cwd": "${workspaceFolder}", + "args": [ + "CI=true", + "ng", + "test", + "--project", + "ngxs", + "--testPathPattern=${fileBasenameNoExtension}", + "--colors", + "--runInBand" + ], + "console": "integratedTerminal", + "internalConsoleOptions": "neverOpen", + "disableOptimisticBPs": true, } ] -} \ No newline at end of file +} diff --git a/bundlesize.config.json b/bundlesize.config.json index b70f13cdc..6229122f6 100644 --- a/bundlesize.config.json +++ b/bundlesize.config.json @@ -32,14 +32,14 @@ "path": "./@ngxs/store/fesm2015/ngxs-store.js", "package": "@ngxs/store", "target": "es2015", - "maxSize": "112.12KB", + "maxSize": "113.25KB", "compression": "none" }, { "path": "./@ngxs/store/fesm5/ngxs-store.js", "package": "@ngxs/store", "target": "es5", - "maxSize": "131.77KB", + "maxSize": "133.10KB", "compression": "none" }, { diff --git a/integration/app/app.component.html b/integration/app/app.component.html index c7a84274d..d20e2490a 100644 --- a/integration/app/app.component.html +++ b/integration/app/app.component.html @@ -1,6 +1,7 @@

Reactive Form

+

{{ injected$ | async }}

Toppings:
Crust
diff --git a/integration/app/app.component.ts b/integration/app/app.component.ts index 76c54ed2a..c6db1a2f1 100644 --- a/integration/app/app.component.ts +++ b/integration/app/app.component.ts @@ -22,9 +22,11 @@ import { Extras, Pizza, Todo } from '@integration/store/todos/todos.model'; export class AppComponent implements OnInit { public allExtras: Extras[]; public pizzaForm: FormGroup; + public greeting: string; @Select(TodoState) public todos$: Observable; @Select(TodoState.pandas) public pandas$: Observable; @Select(TodosState.pizza) public pizza$: Observable; + @Select(TodosState.injected) public injected$: Observable; constructor(private store: Store, private formBuilder: FormBuilder) {} diff --git a/integration/app/store/store.module.ts b/integration/app/store/store.module.ts index 8de95a218..f70c0f38c 100644 --- a/integration/app/store/store.module.ts +++ b/integration/app/store/store.module.ts @@ -17,7 +17,10 @@ import { environment as env } from '../../environments/environment'; NgxsLoggerPluginModule.forRoot({ logger: console, collapsed: false }), NgxsReduxDevtoolsPluginModule.forRoot({ disabled: env.production }), NgxsRouterPluginModule.forRoot(), - NgxsModule.forRoot([TodosState, TodoState], { developmentMode: !env.production }) + NgxsModule.forRoot([TodosState, TodoState], { + developmentMode: !env.production, + selectorOptions: {} // empty object to test option merging + }) ], exports: [ NgxsFormPluginModule, diff --git a/integration/app/store/todos/todos.state.ts b/integration/app/store/todos/todos.state.ts index bab76ff1d..ad5fd7935 100644 --- a/integration/app/store/todos/todos.state.ts +++ b/integration/app/store/todos/todos.state.ts @@ -8,6 +8,7 @@ import { TodoState } from '@integration/store/todos/todo/todo.state'; import { Pizza, TodoStateModel } from '@integration/store/todos/todos.model'; import { LoadData, SetPrefix } from '@integration/store/todos/todos.actions'; import { Injectable } from '@angular/core'; +import { ListState } from '@integration/list/list.state'; const TODOS_TOKEN: StateToken = new StateToken('todos'); @@ -26,6 +27,14 @@ export class TodosState { return state.pizza; } + @Selector([ListState.hello]) + public static injected(state: TodoStateModel, hello: string): string { + if (state.todo == null || hello == null) { + return 'container injection failed or is disabled'; + } + return `${hello}! i have ${state.todo.length} todos`; + } + @Action(SetPrefix) public setPrefix({ setState }: StateContext) { setState( diff --git a/packages/store/src/module.ts b/packages/store/src/module.ts index d51510409..5a29ff075 100644 --- a/packages/store/src/module.ts +++ b/packages/store/src/module.ts @@ -41,6 +41,7 @@ import { DispatchOutsideZoneNgxsExecutionStrategy } from './execution/dispatch-o import { InternalNgxsExecutionStrategy } from './execution/internal-ngxs-execution-strategy'; import { HostEnvironment } from './host-environment/host-environment'; import { ConfigValidator } from './internal/config-validator'; +import { mergeDeep } from './utils/utils'; /** * Ngxs Module @@ -152,7 +153,7 @@ export class NgxsModule { } private static ngxsConfigFactory(options: NgxsModuleOptions): NgxsConfig { - return Object.assign(new NgxsConfig(), options); + return mergeDeep(new NgxsConfig(), options); } private static appBootstrapListenerFactory(bootstrapper: NgxsBootstrapper): Function { diff --git a/packages/store/src/utils/utils.ts b/packages/store/src/utils/utils.ts index c4e294ce3..9132b4c88 100644 --- a/packages/store/src/utils/utils.ts +++ b/packages/store/src/utils/utils.ts @@ -60,3 +60,40 @@ export const setValue = (obj: any, prop: string, val: any) => { */ export const getValue = (obj: any, prop: string): any => prop.split('.').reduce((acc: any, part: string) => acc && acc[part], obj); + +/** + * Simple object check. + * + * isObject({a:1}) //=> true + * isObject(1) //=> false + * + * @ignore + */ +export const isObject = (item: any) => { + return item && typeof item === 'object' && !Array.isArray(item); +}; + +/** + * Deep merge two objects. + * + * mergeDeep({a:1, b:{x: 1, y:2}}, {b:{x: 3}, c:4}) //=> {a:1, b:{x:3, y:2}, c:4} + * + * @param base base object onto which `sources` will be applied + */ +export const mergeDeep = (base: any, ...sources: any[]): any => { + if (!sources.length) return base; + const source = sources.shift(); + + if (isObject(base) && isObject(source)) { + for (const key in source) { + if (isObject(source[key])) { + if (!base[key]) Object.assign(base, { [key]: {} }); + mergeDeep(base[key], source[key]); + } else { + Object.assign(base, { [key]: source[key] }); + } + } + } + + return mergeDeep(base, ...sources); +}; diff --git a/packages/store/tests/selector.spec.ts b/packages/store/tests/selector.spec.ts index 9a286532a..3d1aca137 100644 --- a/packages/store/tests/selector.spec.ts +++ b/packages/store/tests/selector.spec.ts @@ -892,7 +892,7 @@ describe('Selector', () => { TestBed.configureTestingModule({ imports: [ NgxsModule.forRoot([ContactsState], { - selectorOptions: { suppressErrors: false } + selectorOptions: { suppressErrors: false, injectContainerState: false } }) ] }); diff --git a/packages/store/tests/utils/utils.spec.ts b/packages/store/tests/utils/utils.spec.ts index 6d46b6e95..3cf1f9ea8 100644 --- a/packages/store/tests/utils/utils.spec.ts +++ b/packages/store/tests/utils/utils.spec.ts @@ -1,6 +1,6 @@ import { PlainObject } from '@ngxs/store/internals'; import { propGetter } from '../../src/internal/internals'; -import { setValue } from '../../src/utils/utils'; +import { setValue, isObject, mergeDeep } from '../../src/utils/utils'; import { NgxsConfig } from '../../src/symbols'; describe('utils', () => { @@ -79,4 +79,74 @@ describe('utils', () => { expect(propGetter(['a', 'b'], config)(target)).toEqual({ c: 100 }); }); }); + + describe('isObject', () => { + it('should correctly identify objects', () => { + const object = { a: 1 }; + const constructedObject = new Object(1); + const array = [1, 2, 3]; + const string = 'asdasd'; + const number = 12; + + expect(isObject(object)).toBeTruthy(); + expect(isObject(constructedObject)).toBeTruthy(); + expect(isObject(array)).toBeFalsy(); + expect(isObject(string)).toBeFalsy(); + expect(isObject(number)).toBeFalsy(); + + expect(isObject(null)).toBeFalsy(); + expect(isObject(undefined)).toBeFalsy(); + }); + }); + + describe('mergeDeep', () => { + it('should merge properties from two objects', () => { + const base = { a: 1, b: 2 }; + const source = { c: 3 }; + + expect(mergeDeep(base, source)).toEqual({ a: 1, b: 2, c: 3 }); + }); + + it('should merge properties from multiple objects', () => { + const base = { a: 1, b: 2 }; + const sources = [{ c: 3 }, { d: 4 }, { e: 5 }]; + + expect(mergeDeep(base, sources[0], sources[1], sources[2])).toEqual({ + a: 1, + b: 2, + c: 3, + d: 4, + e: 5 + }); + }); + + it('should merge subproperties from two objects ', () => { + const base = { a: 1, b: { x: 2, z: { c: 1 } } }; + const source = { b: { y: 3, z: { d: 2 }, w: { f: 1 } } }; + + expect(mergeDeep(base, source)).toEqual({ + a: 1, + b: { x: 2, y: 3, z: { c: 1, d: 2 }, w: { f: 1 } } + }); + }); + + it('should overwrite properties of base object with source object', () => { + const base = { a: 1, b: 2 }; + const source = { c: 3, b: 4 }; + + expect(mergeDeep(base, source)).toEqual({ a: 1, b: 4, c: 3 }); + }); + + it('should overwrite properties in the correct order', () => { + const base = { a: 1, b: 2 }; + const sources = [{ c: 3, b: 4, d: 6 }, { c: 5 }, { b: 7 }]; + + expect(mergeDeep(base, sources[0], sources[1], sources[2])).toEqual({ + a: 1, + b: 7, + c: 5, + d: 6 + }); + }); + }); });