Skip to content

Commit

Permalink
fix(forms): call setDisabledState on ControlValueAcessor when con…
Browse files Browse the repository at this point in the history
…trol is enabled

Previously, `setDisabledState` was never called when attached if the control is enabled. This PR fixes the bug, and creates a configuration option to opt-out of the fix.

Fixes #35309.
  • Loading branch information
dylhunn committed Sep 30, 2022
1 parent 382330c commit f6f5e07
Show file tree
Hide file tree
Showing 12 changed files with 170 additions and 35 deletions.
22 changes: 13 additions & 9 deletions goldens/public-api/forms/index.md
Expand Up @@ -340,7 +340,7 @@ export const FormControl: ɵFormControlCtor;

// @public
export class FormControlDirective extends NgControl implements OnChanges, OnDestroy {
constructor(validators: (Validator | ValidatorFn)[], asyncValidators: (AsyncValidator | AsyncValidatorFn)[], valueAccessors: ControlValueAccessor[], _ngModelWarningConfig: string | null);
constructor(validators: (Validator | ValidatorFn)[], asyncValidators: (AsyncValidator | AsyncValidatorFn)[], valueAccessors: ControlValueAccessor[], _ngModelWarningConfig: string | null, callSetDisabledState?: SetDisabledStateOption | null | undefined);
get control(): FormControl;
form: FormControl;
set isDisabled(isDisabled: boolean);
Expand All @@ -358,7 +358,7 @@ export class FormControlDirective extends NgControl implements OnChanges, OnDest
// (undocumented)
static ɵdir: i0.ɵɵDirectiveDeclaration<FormControlDirective, "[formControl]", ["ngForm"], { "form": "formControl"; "isDisabled": "disabled"; "model": "ngModel"; }, { "update": "ngModelChange"; }, never, never, false, never>;
// (undocumented)
static ɵfac: i0.ɵɵFactoryDeclaration<FormControlDirective, [{ optional: true; self: true; }, { optional: true; self: true; }, { optional: true; self: true; }, { optional: true; }]>;
static ɵfac: i0.ɵɵFactoryDeclaration<FormControlDirective, [{ optional: true; self: true; }, { optional: true; self: true; }, { optional: true; self: true; }, { optional: true; }, { optional: true; }]>;
}

// @public
Expand Down Expand Up @@ -466,7 +466,7 @@ export class FormGroup<TControl extends {

// @public
export class FormGroupDirective extends ControlContainer implements Form, OnChanges, OnDestroy {
constructor(validators: (Validator | ValidatorFn)[], asyncValidators: (AsyncValidator | AsyncValidatorFn)[]);
constructor(validators: (Validator | ValidatorFn)[], asyncValidators: (AsyncValidator | AsyncValidatorFn)[], callSetDisabledState?: SetDisabledStateOption | null | undefined);
addControl(dir: FormControlName): FormControl;
addFormArray(dir: FormArrayName): void;
addFormGroup(dir: FormGroupName): void;
Expand Down Expand Up @@ -494,7 +494,7 @@ export class FormGroupDirective extends ControlContainer implements Form, OnChan
// (undocumented)
static ɵdir: i0.ɵɵDirectiveDeclaration<FormGroupDirective, "[formGroup]", ["ngForm"], { "form": "formGroup"; }, { "ngSubmit": "ngSubmit"; }, never, never, false, never>;
// (undocumented)
static ɵfac: i0.ɵɵFactoryDeclaration<FormGroupDirective, [{ optional: true; self: true; }, { optional: true; self: true; }]>;
static ɵfac: i0.ɵɵFactoryDeclaration<FormGroupDirective, [{ optional: true; self: true; }, { optional: true; self: true; }, { optional: true; }]>;
}

// @public
Expand Down Expand Up @@ -551,6 +551,9 @@ export interface FormRecord<TControl> {

// @public
export class FormsModule {
static withConfig(opts: {
callSetDisabledState?: SetDisabledStateOption;
}): ModuleWithProviders<ReactiveFormsModule>;
// (undocumented)
static ɵfac: i0.ɵɵFactoryDeclaration<FormsModule, never>;
// (undocumented)
Expand Down Expand Up @@ -631,7 +634,7 @@ export class NgControlStatusGroup extends AbstractControlStatus {

// @public
export class NgForm extends ControlContainer implements Form, AfterViewInit {
constructor(validators: (Validator | ValidatorFn)[], asyncValidators: (AsyncValidator | AsyncValidatorFn)[]);
constructor(validators: (Validator | ValidatorFn)[], asyncValidators: (AsyncValidator | AsyncValidatorFn)[], callSetDisabledState?: SetDisabledStateOption | null | undefined);
addControl(dir: NgModel): void;
addFormGroup(dir: NgModelGroup): void;
get control(): FormGroup;
Expand Down Expand Up @@ -662,12 +665,12 @@ export class NgForm extends ControlContainer implements Form, AfterViewInit {
// (undocumented)
static ɵdir: i0.ɵɵDirectiveDeclaration<NgForm, "form:not([ngNoForm]):not([formGroup]),ng-form,[ngForm]", ["ngForm"], { "options": "ngFormOptions"; }, { "ngSubmit": "ngSubmit"; }, never, never, false, never>;
// (undocumented)
static ɵfac: i0.ɵɵFactoryDeclaration<NgForm, [{ optional: true; self: true; }, { optional: true; self: true; }]>;
static ɵfac: i0.ɵɵFactoryDeclaration<NgForm, [{ optional: true; self: true; }, { optional: true; self: true; }, { optional: true; }]>;
}

// @public
export class NgModel extends NgControl implements OnChanges, OnDestroy {
constructor(parent: ControlContainer, validators: (Validator | ValidatorFn)[], asyncValidators: (AsyncValidator | AsyncValidatorFn)[], valueAccessors: ControlValueAccessor[], _changeDetectorRef?: ChangeDetectorRef | null | undefined);
constructor(parent: ControlContainer, validators: (Validator | ValidatorFn)[], asyncValidators: (AsyncValidator | AsyncValidatorFn)[], valueAccessors: ControlValueAccessor[], _changeDetectorRef?: ChangeDetectorRef | null | undefined, callSetDisabledState?: SetDisabledStateOption | null | undefined);
// (undocumented)
readonly control: FormControl;
get formDirective(): any;
Expand All @@ -692,7 +695,7 @@ export class NgModel extends NgControl implements OnChanges, OnDestroy {
// (undocumented)
static ɵdir: i0.ɵɵDirectiveDeclaration<NgModel, "[ngModel]:not([formControlName]):not([formControl])", ["ngModel"], { "name": "name"; "isDisabled": "disabled"; "model": "ngModel"; "options": "ngModelOptions"; }, { "update": "ngModelChange"; }, never, never, false, never>;
// (undocumented)
static ɵfac: i0.ɵɵFactoryDeclaration<NgModel, [{ optional: true; host: true; }, { optional: true; self: true; }, { optional: true; self: true; }, { optional: true; self: true; }, { optional: true; }]>;
static ɵfac: i0.ɵɵFactoryDeclaration<NgModel, [{ optional: true; host: true; }, { optional: true; self: true; }, { optional: true; self: true; }, { optional: true; self: true; }, { optional: true; }, { optional: true; }]>;
}

// @public
Expand Down Expand Up @@ -787,7 +790,8 @@ export class RangeValueAccessor extends BuiltInControlValueAccessor implements C
// @public
export class ReactiveFormsModule {
static withConfig(opts: {
warnOnNgModelWithFormControl: 'never' | 'once' | 'always';
warnOnNgModelWithFormControl?: 'never' | 'once' | 'always';
callSetDisabledState?: SetDisabledStateOption;
}): ModuleWithProviders<ReactiveFormsModule>;
// (undocumented)
static ɵfac: i0.ɵɵFactoryDeclaration<ReactiveFormsModule, never>;
Expand Down
4 changes: 2 additions & 2 deletions goldens/size-tracking/integration-payloads.json
Expand Up @@ -41,7 +41,7 @@
"forms": {
"uncompressed": {
"runtime": 1060,
"main": 155712,
"main": 157903,
"polyfills": 33915
}
},
Expand All @@ -68,4 +68,4 @@
"bundle": 1214857
}
}
}
}
Expand Up @@ -56,6 +56,9 @@
{
"name": "BuiltInControlValueAccessor"
},
{
"name": "CALL_SET_DISABLED_STATE"
},
{
"name": "CHECKBOX_VALUE_ACCESSOR"
},
Expand Down
Expand Up @@ -59,6 +59,9 @@
{
"name": "BuiltInControlValueAccessor"
},
{
"name": "CALL_SET_DISABLED_STATE"
},
{
"name": "CHECKBOX_VALUE_ACCESSOR"
},
Expand Down
1 change: 1 addition & 0 deletions packages/forms/src/directives.ts
Expand Up @@ -43,6 +43,7 @@ export {FormGroupDirective} from './directives/reactive_directives/form_group_di
export {FormArrayName, FormGroupName} from './directives/reactive_directives/form_group_name';
export {NgSelectOption, SelectControlValueAccessor} from './directives/select_control_value_accessor';
export {NgSelectMultipleOption, SelectMultipleControlValueAccessor} from './directives/select_multiple_control_value_accessor';
export {CALL_SET_DISABLED_STATE} from './directives/shared';

export const SHARED_FORM_DIRECTIVES: Type<any>[] = [
NgNoValidate,
Expand Down
10 changes: 6 additions & 4 deletions packages/forms/src/directives/ng_form.ts
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {AfterViewInit, Directive, EventEmitter, forwardRef, Inject, Input, Optional, Self} from '@angular/core';
import {AfterViewInit, Directive, EventEmitter, forwardRef, inject, Inject, Input, Optional, Self} from '@angular/core';

import {AbstractControl, FormHooks} from '../model/abstract_model';
import {FormControl} from '../model/form_control';
Expand All @@ -18,7 +18,7 @@ import {Form} from './form_interface';
import {NgControl} from './ng_control';
import {NgModel} from './ng_model';
import {NgModelGroup} from './ng_model_group';
import {setUpControl, setUpFormContainer, syncPendingControls} from './shared';
import {CALL_SET_DISABLED_STATE, SetDisabledStateOption, setUpControl, setUpFormContainer, syncPendingControls} from './shared';
import {AsyncValidator, AsyncValidatorFn, Validator, ValidatorFn} from './validators';

export const formDirectiveProvider: any = {
Expand Down Expand Up @@ -135,7 +135,9 @@ export class NgForm extends ControlContainer implements Form, AfterViewInit {
constructor(
@Optional() @Self() @Inject(NG_VALIDATORS) validators: (Validator|ValidatorFn)[],
@Optional() @Self() @Inject(NG_ASYNC_VALIDATORS) asyncValidators:
(AsyncValidator|AsyncValidatorFn)[]) {
(AsyncValidator|AsyncValidatorFn)[],
@Optional() @Inject(CALL_SET_DISABLED_STATE) private callSetDisabledState?:
SetDisabledStateOption|null) {
super();
this.form =
new FormGroup({}, composeValidators(validators), composeAsyncValidators(asyncValidators));
Expand Down Expand Up @@ -191,7 +193,7 @@ export class NgForm extends ControlContainer implements Form, AfterViewInit {
const container = this._findContainer(dir.path);
(dir as {control: FormControl}).control =
<FormControl>container.registerControl(dir.name, dir.control);
setUpControl(dir.control, dir);
setUpControl(dir.control, dir, this.callSetDisabledState);
dir.control.updateValueAndValidity({emitEvent: false});
this._directives.add(dir);
});
Expand Down
10 changes: 6 additions & 4 deletions packages/forms/src/directives/ng_model.ts
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {ChangeDetectorRef, Directive, EventEmitter, forwardRef, Host, Inject, Input, OnChanges, OnDestroy, Optional, Output, Self, SimpleChanges, ɵcoerceToBoolean as coerceToBoolean} from '@angular/core';
import {ChangeDetectorRef, Directive, EventEmitter, forwardRef, Host, inject, Inject, Input, OnChanges, OnDestroy, Optional, Output, Self, SimpleChanges, ɵcoerceToBoolean as coerceToBoolean} from '@angular/core';

import {FormHooks} from '../model/abstract_model';
import {FormControl} from '../model/form_control';
Expand All @@ -18,7 +18,7 @@ import {ControlValueAccessor, NG_VALUE_ACCESSOR} from './control_value_accessor'
import {NgControl} from './ng_control';
import {NgForm} from './ng_form';
import {NgModelGroup} from './ng_model_group';
import {controlPath, isPropertyUpdated, selectValueAccessor, setUpControl} from './shared';
import {CALL_SET_DISABLED_STATE, controlPath, isPropertyUpdated, selectValueAccessor, SetDisabledStateOption, setUpControl} from './shared';
import {formGroupNameException, missingNameException, modelParentException} from './template_driven_errors';
import {AsyncValidator, AsyncValidatorFn, Validator, ValidatorFn} from './validators';

Expand Down Expand Up @@ -210,7 +210,9 @@ export class NgModel extends NgControl implements OnChanges, OnDestroy {
@Optional() @Self() @Inject(NG_ASYNC_VALIDATORS) asyncValidators:
(AsyncValidator|AsyncValidatorFn)[],
@Optional() @Self() @Inject(NG_VALUE_ACCESSOR) valueAccessors: ControlValueAccessor[],
@Optional() @Inject(ChangeDetectorRef) private _changeDetectorRef?: ChangeDetectorRef|null) {
@Optional() @Inject(ChangeDetectorRef) private _changeDetectorRef?: ChangeDetectorRef|null,
@Optional() @Inject(CALL_SET_DISABLED_STATE) private callSetDisabledState?:
SetDisabledStateOption|null) {
super();
this._parent = parent;
this._setValidators(validators);
Expand Down Expand Up @@ -295,7 +297,7 @@ export class NgModel extends NgControl implements OnChanges, OnDestroy {
}

private _setUpStandalone(): void {
setUpControl(this.control, this);
setUpControl(this.control, this, this.callSetDisabledState);
this.control.updateValueAndValidity({emitEvent: false});
}

Expand Down
Expand Up @@ -6,14 +6,14 @@
* found in the LICENSE file at https://angular.io/license
*/

import {Directive, EventEmitter, forwardRef, Inject, InjectionToken, Input, OnChanges, OnDestroy, Optional, Output, Self, SimpleChanges} from '@angular/core';
import {Directive, EventEmitter, forwardRef, Inject, inject, InjectionToken, Input, OnChanges, OnDestroy, Optional, Output, Self, SimpleChanges} from '@angular/core';

import {FormControl} from '../../model/form_control';
import {NG_ASYNC_VALIDATORS, NG_VALIDATORS} from '../../validators';
import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '../control_value_accessor';
import {NgControl} from '../ng_control';
import {disabledAttrWarning} from '../reactive_errors';
import {_ngModelWarning, cleanUpControl, isPropertyUpdated, selectValueAccessor, setUpControl} from '../shared';
import {_ngModelWarning, CALL_SET_DISABLED_STATE, cleanUpControl, isPropertyUpdated, selectValueAccessor, setDisabledStateDefault, SetDisabledStateOption, setUpControl} from '../shared';
import {AsyncValidator, AsyncValidatorFn, Validator, ValidatorFn} from '../validators';


Expand Down Expand Up @@ -108,7 +108,9 @@ export class FormControlDirective extends NgControl implements OnChanges, OnDest
(AsyncValidator|AsyncValidatorFn)[],
@Optional() @Self() @Inject(NG_VALUE_ACCESSOR) valueAccessors: ControlValueAccessor[],
@Optional() @Inject(NG_MODEL_WITH_FORM_CONTROL_WARNING) private _ngModelWarningConfig: string|
null) {
null,
@Optional() @Inject(CALL_SET_DISABLED_STATE) private callSetDisabledState?:
SetDisabledStateOption|null) {
super();
this._setValidators(validators);
this._setAsyncValidators(asyncValidators);
Expand All @@ -122,7 +124,7 @@ export class FormControlDirective extends NgControl implements OnChanges, OnDest
if (previousForm) {
cleanUpControl(previousForm, this, /* validateControlPresenceOnChange */ false);
}
setUpControl(this.form, this);
setUpControl(this.form, this, this.callSetDisabledState);
this.form.updateValueAndValidity({emitEvent: false});
}
if (isPropertyUpdated(changes, this.viewModel)) {
Expand Down
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {Directive, EventEmitter, forwardRef, Inject, Input, OnChanges, OnDestroy, Optional, Output, Self, SimpleChanges} from '@angular/core';
import {Directive, EventEmitter, forwardRef, Inject, inject, Input, OnChanges, OnDestroy, Optional, Output, Self, SimpleChanges} from '@angular/core';

import {FormArray} from '../../model/form_array';
import {FormControl, isFormControl} from '../../model/form_control';
Expand All @@ -15,7 +15,7 @@ import {NG_ASYNC_VALIDATORS, NG_VALIDATORS} from '../../validators';
import {ControlContainer} from '../control_container';
import {Form} from '../form_interface';
import {missingFormException} from '../reactive_errors';
import {cleanUpControl, cleanUpFormContainer, cleanUpValidators, removeListItem, setUpControl, setUpFormContainer, setUpValidators, syncPendingControls} from '../shared';
import {CALL_SET_DISABLED_STATE, cleanUpControl, cleanUpFormContainer, cleanUpValidators, removeListItem, SetDisabledStateOption, setUpControl, setUpFormContainer, setUpValidators, syncPendingControls} from '../shared';
import {AsyncValidator, AsyncValidatorFn, Validator, ValidatorFn} from '../validators';

import {FormControlName} from './form_control_name';
Expand Down Expand Up @@ -96,7 +96,9 @@ export class FormGroupDirective extends ControlContainer implements Form, OnChan
constructor(
@Optional() @Self() @Inject(NG_VALIDATORS) validators: (Validator|ValidatorFn)[],
@Optional() @Self() @Inject(NG_ASYNC_VALIDATORS) asyncValidators:
(AsyncValidator|AsyncValidatorFn)[]) {
(AsyncValidator|AsyncValidatorFn)[],
@Optional() @Inject(CALL_SET_DISABLED_STATE) private callSetDisabledState?:
SetDisabledStateOption|null) {
super();
this._setValidators(validators);
this._setAsyncValidators(asyncValidators);
Expand Down Expand Up @@ -164,7 +166,7 @@ export class FormGroupDirective extends ControlContainer implements Form, OnChan
*/
addControl(dir: FormControlName): FormControl {
const ctrl: any = this.form.get(dir.path);
setUpControl(ctrl, dir);
setUpControl(ctrl, dir, this.callSetDisabledState);
ctrl.updateValueAndValidity({emitEvent: false});
this.directives.push(dir);
return ctrl;
Expand Down Expand Up @@ -312,7 +314,7 @@ export class FormGroupDirective extends ControlContainer implements Form, OnChan
// taken care of in the `removeControl` method invoked when corresponding `formControlName`
// directive instance is being removed (invoked from `FormControlName.ngOnDestroy`).
if (isFormControl(newCtrl)) {
setUpControl(newCtrl, dir);
setUpControl(newCtrl, dir, this.callSetDisabledState);
(dir as {control: FormControl}).control = newCtrl;
}
}
Expand Down
27 changes: 23 additions & 4 deletions packages/forms/src/directives/shared.ts
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {ɵRuntimeError as RuntimeError} from '@angular/core';
import {Inject, InjectionToken, ɵRuntimeError as RuntimeError} from '@angular/core';

import {RuntimeErrorCode} from '../errors';
import {AbstractControl} from '../model/abstract_model';
Expand All @@ -25,6 +25,22 @@ import {FormArrayName} from './reactive_directives/form_group_name';
import {ngModelWarning} from './reactive_errors';
import {AsyncValidatorFn, Validator, ValidatorFn} from './validators';

/**
* Token to provide to allow SetDisabledState to always be called when a CVA is added, regardless of
* whether the control is disabled or enabled.
*/
export const CALL_SET_DISABLED_STATE = new InjectionToken('CallSetDisabledState');

/**
* The type for CALL_SET_DISABLED_STATE.
*/
export type SetDisabledStateOption = 'whenDisabledForLegacyCode'|'always';

/**
* Whether to use the fixed setDisabledState behavior by default.
*/
export const setDisabledStateDefault = 'always';

export function controlPath(name: string|null, parent: ControlContainer): string[] {
return [...parent.path!, name!];
}
Expand All @@ -36,7 +52,10 @@ export function controlPath(name: string|null, parent: ControlContainer): string
* @param control Form control instance that should be linked.
* @param dir Directive that should be linked with a given control.
*/
export function setUpControl(control: FormControl, dir: NgControl): void {
export function setUpControl(
control: FormControl, dir: NgControl,
callSetDisabledState: SetDisabledStateOption|null = setDisabledStateDefault): void {
if (callSetDisabledState == null) callSetDisabledState = setDisabledStateDefault;
if (typeof ngDevMode === 'undefined' || ngDevMode) {
if (!control) _throwError(dir, 'Cannot find control with');
if (!dir.valueAccessor) _throwError(dir, 'No value accessor for form control with');
Expand All @@ -46,8 +65,8 @@ export function setUpControl(control: FormControl, dir: NgControl): void {

dir.valueAccessor!.writeValue(control.value);

if (control.disabled) {
dir.valueAccessor!.setDisabledState?.(true);
if (control.disabled || callSetDisabledState === 'always') {
dir.valueAccessor!.setDisabledState?.(control.disabled);
}

setUpViewChangePipeline(control, dir);
Expand Down

0 comments on commit f6f5e07

Please sign in to comment.