From 12acf751eecf19eec8e66ede7e9de723f32028f5 Mon Sep 17 00:00:00 2001 From: Gabriel Schuster Date: Sat, 11 Nov 2017 13:47:12 +0100 Subject: [PATCH 01/12] Fix for reactive forms, custom input debounce --- source/connect/connect-base.ts | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/source/connect/connect-base.ts b/source/connect/connect-base.ts index c4c1ecc..5e06b7c 100644 --- a/source/connect/connect-base.ts +++ b/source/connect/connect-base.ts @@ -17,6 +17,8 @@ import 'rxjs/add/operator/debounceTime'; import { FormStore } from '../form-store'; import { State } from '../state'; +import { Iterable } from 'immutable'; + export interface ControlPair { path: Array; control: AbstractControl; @@ -25,11 +27,15 @@ export interface ControlPair { export class ConnectBase { @Input('connect') connect: () => (string | number) | Array; + @Input('debounce') debounce: number; private stateSubscription: Unsubscribe; private formSubscription: Subscription; protected store: FormStore; protected form: any; + protected get changeDebounce(): number { + return 'number' === typeof this.debounce || ('string' === typeof this.debounce && String(this.debounce).match(/^[0-9]+(\.[0-9]+)?$/)) ? Number(this.debounce) : 0; + } public get path(): Array { const path = typeof this.connect === 'function' @@ -63,13 +69,13 @@ export class ConnectBase { ngAfterContentInit() { Promise.resolve().then(() => { - this.resetState(); + this.resetState(false); - this.stateSubscription = this.store.subscribe(() => this.resetState()); + this.stateSubscription = this.store.subscribe(() => this.resetState(true)); Promise.resolve().then(() => { this.formSubscription = (this.form.valueChanges) - .debounceTime(0) + .debounceTime(this.changeDebounce) .subscribe((values: any) => this.publish(values)); }); }); @@ -97,11 +103,12 @@ export class ConnectBase { throw new Error(`Unknown type of form element: ${formElement.constructor.name}`); } - return pairs.filter(p => (p.control)._parent === this.form.control); + return pairs.filter(p => (p.control)._parent === this.form.control || (p.control)._parent === this.form); } - private resetState() { + private resetState(emitEvent: boolean) { var formElement; + if (this.form.control === undefined) { formElement = this.form; } @@ -114,12 +121,12 @@ export class ConnectBase { children.forEach(c => { const { path, control } = c; - const value = State.get(this.getState(), this.path.concat(c.path)); + const value = State.get(this.getState(), this.path.concat(path)); if (control.value !== value) { const phonyControl = { path: path }; - this.form.updateModel(phonyControl, value); + control.setValue(value, {emitEvent}); } }); } From feb36d6aea525b231b423b8ecf4f437c4ea8c3e5 Mon Sep 17 00:00:00 2001 From: Gabriel Schuster Date: Sat, 11 Nov 2017 13:49:50 +0100 Subject: [PATCH 02/12] Removed unused immutable import --- source/connect/connect-base.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/source/connect/connect-base.ts b/source/connect/connect-base.ts index 5e06b7c..c659762 100644 --- a/source/connect/connect-base.ts +++ b/source/connect/connect-base.ts @@ -17,8 +17,6 @@ import 'rxjs/add/operator/debounceTime'; import { FormStore } from '../form-store'; import { State } from '../state'; -import { Iterable } from 'immutable'; - export interface ControlPair { path: Array; control: AbstractControl; From 225be29969e70b374347db6c1a806b8513cffbef Mon Sep 17 00:00:00 2001 From: Gabriel Schuster Date: Sat, 11 Nov 2017 14:00:44 +0100 Subject: [PATCH 03/12] Version bump due to new debounce --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index bf1ba61..d52f4c7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@angular-redux/form", - "version": "6.6.0", + "version": "6.7.0", "description": "Build Angular 2+ forms with Redux", "dependencies": { "immutable": "^3.8.1" From 1616da4411593dc510ee04bd0dde323356250348 Mon Sep 17 00:00:00 2001 From: Gabriel Schuster Date: Sat, 11 Nov 2017 14:01:57 +0100 Subject: [PATCH 04/12] Update CHANGELOG.md --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a3c7f87..b806d8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +# 6.7.0 - Fix reactive forms, support custom debounce time for form inputs + +* https://github.com/angular-redux/form/pull/48 + # 6.5.1 - Support typescript unused checks * https://github.com/angular-redux/form/pull/32 From 48923ff3fd83432cc16a5ca016c8b9c0050cf523 Mon Sep 17 00:00:00 2001 From: Gabriel Schuster Date: Sat, 11 Nov 2017 14:05:32 +0100 Subject: [PATCH 05/12] Update README.md --- README.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/README.md b/README.md index 7e3ba61..8d721dd 100644 --- a/README.md +++ b/README.md @@ -122,6 +122,14 @@ Both `NgRedux` and `Redux.Store` conform to this shape. If you have a more complicated use-case that is not covered here, you could even create your own store shim as long as it conforms to the shape of `AbstractStore`. +#### Input debounce + +To debounce emitted FORM_CHANGED actions simply specify the desired debounce time in milliseconds on the form: + +```html +
+``` + ### How the bindings work The bindings work by inspecting the shape of your form and then binding to a Redux From f7ccc9fc32c85a45d183e5c735770269023f9cb6 Mon Sep 17 00:00:00 2001 From: Gabriel Schuster Date: Sun, 12 Nov 2017 12:47:35 +0100 Subject: [PATCH 06/12] Added isEmpty check before setting control value ...to prevent event emission when there in fact is no change. "null" and "undefined" (possibly valid in your state) in fact are empty but unequal to "empty string" (in fact the only valid "empty value" on forms) and thus would trigger "form changed" event. --- source/connect/connect-base.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/source/connect/connect-base.ts b/source/connect/connect-base.ts index c659762..613345d 100644 --- a/source/connect/connect-base.ts +++ b/source/connect/connect-base.ts @@ -119,12 +119,12 @@ export class ConnectBase { children.forEach(c => { const { path, control } = c; - const value = State.get(this.getState(), this.path.concat(path)); + const value = State.get(this.getState(), this.path.concat(path)); + const newValueIsEmpty: boolean = 'undefined' === typeof value || null === value || ('string' === typeof value && '' === value); + const oldValueIsEmpty: boolean = 'undefined' === typeof control.value || null === control.value || ('string' === typeof control.value && '' === control.value); - if (control.value !== value) { - const phonyControl = { path: path }; - - control.setValue(value, {emitEvent}); + if (oldValueIsEmpty !== newValueIsEmpty && control.value !== value) { + control.setValue(newValueIsEmpty ? '' : value, {emitEvent}); } }); } From d4026c1d5acd1479cf94db52683663f5f5c0367f Mon Sep 17 00:00:00 2001 From: Gabriel Schuster Date: Sun, 12 Nov 2017 12:53:00 +0100 Subject: [PATCH 07/12] Update connect-base.ts --- source/connect/connect-base.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/connect/connect-base.ts b/source/connect/connect-base.ts index 613345d..25c38bb 100644 --- a/source/connect/connect-base.ts +++ b/source/connect/connect-base.ts @@ -123,7 +123,7 @@ export class ConnectBase { const newValueIsEmpty: boolean = 'undefined' === typeof value || null === value || ('string' === typeof value && '' === value); const oldValueIsEmpty: boolean = 'undefined' === typeof control.value || null === control.value || ('string' === typeof control.value && '' === control.value); - if (oldValueIsEmpty !== newValueIsEmpty && control.value !== value) { + if (oldValueIsEmpty !== newValueIsEmpty || (!oldValueIsEmpty && !newValueIsEmpty && control.value !== value)) { control.setValue(newValueIsEmpty ? '' : value, {emitEvent}); } }); From 6d861fd89e89d3eda4cc6c178d36be7164c4de1d Mon Sep 17 00:00:00 2001 From: Gabriel Schuster Date: Sun, 12 Nov 2017 13:02:22 +0100 Subject: [PATCH 08/12] Added comments to clarify code flow --- source/connect/connect-base.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/source/connect/connect-base.ts b/source/connect/connect-base.ts index 25c38bb..3b42e2b 100644 --- a/source/connect/connect-base.ts +++ b/source/connect/connect-base.ts @@ -67,8 +67,10 @@ export class ConnectBase { ngAfterContentInit() { Promise.resolve().then(() => { + // This is the first "change" of the form (setting initial values from the store) and thus should not emit a "changed" event this.resetState(false); + // Any further changes on the state are due to application flow (e.g. user interaction triggering state changes) and thus have to trigger "changed" events this.stateSubscription = this.store.subscribe(() => this.resetState(true)); Promise.resolve().then(() => { @@ -104,7 +106,7 @@ export class ConnectBase { return pairs.filter(p => (p.control)._parent === this.form.control || (p.control)._parent === this.form); } - private resetState(emitEvent: boolean) { + private resetState(emitEvent: boolean = true) { var formElement; if (this.form.control === undefined) { @@ -123,8 +125,15 @@ export class ConnectBase { const newValueIsEmpty: boolean = 'undefined' === typeof value || null === value || ('string' === typeof value && '' === value); const oldValueIsEmpty: boolean = 'undefined' === typeof control.value || null === control.value || ('string' === typeof control.value && '' === control.value); + // setValue() should only be called upon "real changes", meaning "null" and "undefined" should be treated equal to "" (empty string) + // newValueIsEmpty: true, oldValueIsEmpty: true => no change + // newValueIsEmpty: true, oldValueIsEmpty: false => change + // newValueIsEmpty: false, oldValueIsEmpty: true => change + // newValueIsEmpty: false, oldValueIsEmpty: false => + // control.value === value => no change + // control.value !== value => change if (oldValueIsEmpty !== newValueIsEmpty || (!oldValueIsEmpty && !newValueIsEmpty && control.value !== value)) { - control.setValue(newValueIsEmpty ? '' : value, {emitEvent}); + control.setValue(newValueIsEmpty ? '' : value, {emitEvent: !!emitEvent}); } }); } From 996dba4b9bc5ee32427ad0d93f8239c3c24ba94f Mon Sep 17 00:00:00 2001 From: Gabriel Schuster Date: Sun, 12 Nov 2017 13:13:13 +0100 Subject: [PATCH 09/12] Update connect-base.ts --- source/connect/connect-base.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/source/connect/connect-base.ts b/source/connect/connect-base.ts index 3b42e2b..ac4e4ca 100644 --- a/source/connect/connect-base.ts +++ b/source/connect/connect-base.ts @@ -107,6 +107,8 @@ export class ConnectBase { } private resetState(emitEvent: boolean = true) { + emitEvent = !!emitEvent ? true : false; + var formElement; if (this.form.control === undefined) { @@ -133,7 +135,7 @@ export class ConnectBase { // control.value === value => no change // control.value !== value => change if (oldValueIsEmpty !== newValueIsEmpty || (!oldValueIsEmpty && !newValueIsEmpty && control.value !== value)) { - control.setValue(newValueIsEmpty ? '' : value, {emitEvent: !!emitEvent}); + control.setValue(newValueIsEmpty ? '' : value, {emitEvent}); } }); } From f2a03a5bd2fa9315705f9a28906fe0c2a7b65130 Mon Sep 17 00:00:00 2001 From: Gabriel Schuster Date: Tue, 21 Nov 2017 00:38:34 +0100 Subject: [PATCH 10/12] Update connect-base.ts --- source/connect/connect-base.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/connect/connect-base.ts b/source/connect/connect-base.ts index ac4e4ca..92220c1 100644 --- a/source/connect/connect-base.ts +++ b/source/connect/connect-base.ts @@ -127,7 +127,7 @@ export class ConnectBase { const newValueIsEmpty: boolean = 'undefined' === typeof value || null === value || ('string' === typeof value && '' === value); const oldValueIsEmpty: boolean = 'undefined' === typeof control.value || null === control.value || ('string' === typeof control.value && '' === control.value); - // setValue() should only be called upon "real changes", meaning "null" and "undefined" should be treated equal to "" (empty string) + // patchValue() should only be called upon "real changes", meaning "null" and "undefined" should be treated equal to "" (empty string) // newValueIsEmpty: true, oldValueIsEmpty: true => no change // newValueIsEmpty: true, oldValueIsEmpty: false => change // newValueIsEmpty: false, oldValueIsEmpty: true => change @@ -135,7 +135,7 @@ export class ConnectBase { // control.value === value => no change // control.value !== value => change if (oldValueIsEmpty !== newValueIsEmpty || (!oldValueIsEmpty && !newValueIsEmpty && control.value !== value)) { - control.setValue(newValueIsEmpty ? '' : value, {emitEvent}); + control.patchValue(newValueIsEmpty ? '' : value, {emitEvent}); } }); } From 6b0b4ddd733a71534a7fc37ed53c4fb1ec54edf5 Mon Sep 17 00:00:00 2001 From: Gabriel Schuster Date: Wed, 22 Nov 2017 09:21:49 +0100 Subject: [PATCH 11/12] Fix descendants Only fetch fields as descendants, not FormGroups or FormArrays as otherwise patchValue() runs and endless loop because of strict comparison of old and new value always returning false because FromGroup/FormArray objects fetched from form and store are never the same. --- source/connect/connect-base.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/source/connect/connect-base.ts b/source/connect/connect-base.ts index 92220c1..0c90b4e 100644 --- a/source/connect/connect-base.ts +++ b/source/connect/connect-base.ts @@ -93,7 +93,13 @@ export class ConnectBase { } else if (formElement instanceof FormGroup) { for (const k of Object.keys(formElement.controls)) { - pairs.push({ path: path.concat([k]), control: formElement.controls[k] }); + // If the control is a FormGroup or FormArray get the descendants of the the control instead of the control itself to always patch fields, not groups/arrays + if(formElement.controls[k] instanceof FormArray || formElement.controls[k] instanceof FormGroup) { + pairs.push(...this.descendants(path.concat([k]), formElement.controls[k])); + } + else { + pairs.push({ path: path.concat([k]), control: formElement.controls[k] }); + } } } else if (formElement instanceof NgControl || formElement instanceof FormControl) { @@ -103,7 +109,7 @@ export class ConnectBase { throw new Error(`Unknown type of form element: ${formElement.constructor.name}`); } - return pairs.filter(p => (p.control)._parent === this.form.control || (p.control)._parent === this.form); + return pairs; } private resetState(emitEvent: boolean = true) { From 7bc8c2450666606276eb2fad6d10ab442ed712de Mon Sep 17 00:00:00 2001 From: actra-gschuster Date: Wed, 29 Nov 2017 01:03:46 +0100 Subject: [PATCH 12/12] Check for valid object when inspecting state --- .gitignore | 107 +++++++++++++++++++++++++----------------------- source/state.ts | 6 ++- 2 files changed, 60 insertions(+), 53 deletions(-) diff --git a/.gitignore b/.gitignore index 9e50955..1531846 100644 --- a/.gitignore +++ b/.gitignore @@ -1,52 +1,55 @@ -# Logs -logs -*.log -**/npm-debug.log* - -.idea - -# Runtime data -pids -*.pid -*.seed - -# Temporary editor files -*.swp -*~ - -# Directory for instrumented libs generated by jscoverage/JSCover -lib-cov - -# Coverage directory used by tools like istanbul -coverage - -# nyc test coverage -.nyc_output - -# Grunt intermediate storage (http://gruntjs.com/creating-plugins#storing-task-files) -.grunt - -# node-waf configuration -.lock-wscript - -# Compiled binary addons (http://nodejs.org/api/addons.html) -build/Release - -# Dependency directories -node_modules -jspm_packages - -# Optional npm cache directory -.npm - -# Optional REPL history -.node_repl_history - -# Build output -.awcache -dist - -source/*ngfactory.ts -source/*ngsummary.json - -*.tgz +# Logs +logs +*.log +**/npm-debug.log* + +.idea +.settings + +# Runtime data +pids +*.pid +*.seed + +# Temporary editor files +*.swp +*~ + +# Directory for instrumented libs generated by jscoverage/JSCover +lib-cov + +# Coverage directory used by tools like istanbul +coverage + +# nyc test coverage +.nyc_output + +# Grunt intermediate storage (http://gruntjs.com/creating-plugins#storing-task-files) +.grunt + +# node-waf configuration +.lock-wscript + +# Compiled binary addons (http://nodejs.org/api/addons.html) +build/Release + +# Dependency directories +node_modules +jspm_packages + +# Optional npm cache directory +.npm + +# Optional REPL history +.node_repl_history + +# Build output +.awcache +dist + +source/*ngfactory.ts +source/*ngsummary.json + +*.tgz +/.project +/.tern-project diff --git a/source/state.ts b/source/state.ts index 671d288..5a9a71c 100644 --- a/source/state.ts +++ b/source/state.ts @@ -36,9 +36,13 @@ export abstract class State { else if (deepValue instanceof Map) { deepValue = (> deepValue).get(k); } - else { + else if('object' === typeof deepValue && !Array.isArray(deepValue) && null !== deepValue) { deepValue = (deepValue as any)[k]; } + else { + return undefined; + } + if (typeof fn === 'function') { const transformed = fn(parent, k, path.slice(path.indexOf(k) + 1), deepValue);