Skip to content

Commit

Permalink
perf(core): reduce the emitted modelChange events (#1999)
Browse files Browse the repository at this point in the history
  • Loading branch information
aitboudad committed Dec 25, 2019
1 parent 73e1651 commit b7567a1
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 40 deletions.
6 changes: 4 additions & 2 deletions src/core/src/lib/components/formly.form.spec.ts
Expand Up @@ -137,7 +137,7 @@ describe('FormlyForm Component', () => {
subscription.unsubscribe();
});

it('should emit `modelChange` twice when key is duplicated', () => {
it('should emit `modelChange` once when key is duplicated', () => {
app.fields = [
{ key: 'title', type: 'text' },
{ key: 'title', type: 'text' },
Expand All @@ -147,10 +147,12 @@ describe('FormlyForm Component', () => {
const spy = jasmine.createSpy('model change spy');
const subscription = fixture.componentInstance.formlyForm.modelChange.subscribe(spy);

app.form.get('title').patchValue('***');
app.form.get('title').patchValue('***');
app.form.get('title').patchValue('***');

fixture.detectChanges();
expect(spy).toHaveBeenCalledTimes(2);
expect(spy).toHaveBeenCalledTimes(1);
expect(spy).toHaveBeenCalledWith({ title: '***' });
subscription.unsubscribe();
});
Expand Down
59 changes: 27 additions & 32 deletions src/core/src/lib/components/formly.form.ts
Expand Up @@ -4,8 +4,8 @@ import { FormlyFieldConfig, FormlyFormOptions, FormlyFormOptionsCache } from './
import { FormlyFormBuilder } from '../services/formly.form.builder';
import { FormlyConfig } from '../services/formly.config';
import { assignModelValue, isNullOrUndefined, wrapProperty, clone, defineHiddenProp, getKeyPath } from '../utils';
import { Subscription, of } from 'rxjs';
import { debounceTime, startWith, pairwise, map, first, timeout, catchError } from 'rxjs/operators';
import { Subscription, of, Subject, timer } from 'rxjs';
import { debounceTime, first, timeout, catchError, debounce, switchMap, distinctUntilChanged } from 'rxjs/operators';

@Component({
selector: 'formly-form',
Expand Down Expand Up @@ -51,6 +51,22 @@ export class FormlyForm implements DoCheck, OnChanges, OnDestroy {
private _options: FormlyFormOptions;
private initialModel: any;
private modelChangeSubs: Subscription[] = [];
private useDebounce = false;
private modelChange$ = new Subject<void>();
private modelChangeSub = this.modelChange$.pipe(
debounce(() => this.useDebounce ? timer(100) : of()),
switchMap(() => this.form.valueChanges.pipe(
timeout(0),
catchError(() => of(null)),
first(),
)),
).subscribe(() => {
this.useDebounce = true;
this.checkExpressionChange();
this.cdRef.detectChanges();
this.modelChange.emit(clone(this.model));
this.useDebounce = false;
});

constructor(
private formlyBuilder: FormlyFormBuilder,
Expand Down Expand Up @@ -85,24 +101,13 @@ export class FormlyForm implements DoCheck, OnChanges, OnDestroy {
}

ngOnDestroy() {
this.modelChangeSub.unsubscribe();
this.clearModelSubscriptions();
}

changeModel({ key, value }: { key: string, value: any }) {
assignModelValue(this.model, key.split('.'), value);
this.form.valueChanges
.pipe(
timeout(0),
catchError(() => of(null)),
first(),
)
.subscribe(() => {
if (this.checkExpressionChange()) {
this.cdRef.detectChanges();
}

this.modelChange.emit(clone(this.model));
});
this.modelChange$.next();
}

setOptions() {
Expand Down Expand Up @@ -163,42 +168,32 @@ export class FormlyForm implements DoCheck, OnChanges, OnDestroy {

private checkExpressionChange() {
if (this.options && (<FormlyFormOptionsCache> this.options)._checkField) {
return (<FormlyFormOptionsCache> this.options)._checkField({
(<FormlyFormOptionsCache> this.options)._checkField({
fieldGroup: this.fields,
model: this.model,
formControl: this.form,
options: this.options,
});
}

return false;
}

private trackModelChanges(fields: FormlyFieldConfig[], rootKey: string[] = []) {
fields.forEach(field => {
if (field.key && !field.fieldGroup) {
const control = field.formControl;
let valueChanges = control.valueChanges;
let valueChanges = control.valueChanges.pipe(distinctUntilChanged());

const { updateOn, debounce } = field.modelOptions;
if ((!updateOn || updateOn === 'change') && debounce && debounce.default > 0) {
valueChanges = control.valueChanges.pipe(debounceTime(debounce.default));
}

// workaround for https://github.com/angular/angular/issues/13792
valueChanges = valueChanges.pipe(
startWith(control.value),
pairwise(),
map(([prevVal, value]) => {
if ((control as any)._onChange.length > 1 && prevVal !== value) {
control.patchValue(value, { emitEvent: false, onlySelf: true });
}

return value;
}),
);

this.modelChangeSubs.push(valueChanges.subscribe((value) => {
// workaround for https://github.com/angular/angular/issues/13792
if ((control as any)._onChange.length > 1) {
control.patchValue(value, { emitEvent: false, onlySelf: true });
}

if (field.parsers && field.parsers.length > 0) {
field.parsers.forEach(parserFn => value = parserFn(value));
}
Expand Down
Expand Up @@ -128,13 +128,8 @@ export class FieldExpressionExtension implements FormlyExtension {
.sort(f => f.hide ? -1 : 1)
.forEach(f => this.toggleFormControl(f, f.hide));

if (options._hiddenFieldsForCheck.length > 0) {
options._hiddenFieldsForCheck = [];
return true;
}
options._hiddenFieldsForCheck = [];
}

return false;
}

private checkFieldExpressionChange(field: FormlyFieldConfigCache, ignoreCache): boolean {
Expand Down

0 comments on commit b7567a1

Please sign in to comment.