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 (#47576)

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.

BREAKING CHANGE: setDisabledState will always be called when a `ControlValueAccessor` is attached. You can opt-out with `FormsModule.withConfig` or `ReactiveFormsModule.withConfig`.

PR Close #47576
  • Loading branch information
@dylhunn @thePunderWoman
dylhunn authored and thePunderWoman committed Oct 11, 2022
1 parent 1818c54 commit 96b7fe9
Show file tree
Hide file tree
Showing 15 changed files with 227 additions and 37 deletions.
2 changes: 1 addition & 1 deletion .circleci/config.yml
View file
Expand Up @@ -36,7 +36,7 @@ var_4_win: &cache_key_win_fallback v10-angular-win-node-16-{{ checksum "month.tx

# Cache key for the `components-repo-unit-tests` job. **Note** when updating the SHA in the
# cache keys also update the SHA for the "COMPONENTS_REPO_COMMIT" environment variable.
var_5: &components_repo_unit_tests_cache_key v2-angular-components-{{ checksum "month.txt" }}-72547a41d4230cea0c6a5448e85bd60cfc26bd35
var_5: &components_repo_unit_tests_cache_key v2-angular-components-{{ checksum "month.txt" }}-87eb708d162e89897e66809c371e3a1e079de962
var_6: &components_repo_unit_tests_cache_key_fallback v2-angular-components-{{ checksum "month.txt" }}

# Workspace initially persisted by the `setup` job, and then enhanced by `build-npm-packages`.
Expand Down
2 changes: 1 addition & 1 deletion .circleci/env.sh
View file
Expand Up @@ -73,7 +73,7 @@ setPublicVar COMPONENTS_REPO_TMP_DIR "/tmp/angular-components-repo"
setPublicVar COMPONENTS_REPO_URL "https://github.com/angular/components.git"
setPublicVar COMPONENTS_REPO_BRANCH "main"
# **NOTE**: When updating the commit SHA, also update the cache key in the CircleCI `config.yml`.
setPublicVar COMPONENTS_REPO_COMMIT "72547a41d4230cea0c6a5448e85bd60cfc26bd35"
setPublicVar COMPONENTS_REPO_COMMIT "87eb708d162e89897e66809c371e3a1e079de962"

####################################################################################################
# Create shell script in /tmp for Bazel actions to access CI envs without
Expand Down
25 changes: 16 additions & 9 deletions goldens/public-api/forms/index.md
View file
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 | 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 | 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 @@ -643,7 +646,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 | undefined);
addControl(dir: NgModel): void;
addFormGroup(dir: NgModelGroup): void;
get control(): FormGroup;
Expand Down Expand Up @@ -674,12 +677,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 | undefined);
// (undocumented)
readonly control: FormControl;
get formDirective(): any;
Expand All @@ -704,7 +707,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 @@ -799,7 +802,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 Expand Up @@ -845,6 +849,9 @@ export class SelectMultipleControlValueAccessor extends BuiltInControlValueAcces
static ɵfac: i0.ɵɵFactoryDeclaration<SelectMultipleControlValueAccessor, never>;
}

// @public
export type SetDisabledStateOption = 'whenDisabledForLegacyCode' | 'always';

// @public
export type UntypedFormArray = FormArray<any>;

Expand Down
4 changes: 2 additions & 2 deletions goldens/size-tracking/integration-payloads.json
View file
Expand Up @@ -41,7 +41,7 @@
"forms": {
"uncompressed": {
"runtime": 1060,
"main": 155712,
"main": 156411,
"polyfills": 33915
}
},
Expand All @@ -68,4 +68,4 @@
"bundle": 1214857
}
}
}
}
6 changes: 6 additions & 0 deletions packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json
View file
Expand Up @@ -56,6 +56,9 @@
{
"name": "BuiltInControlValueAccessor"
},
{
"name": "CALL_SET_DISABLED_STATE"
},
{
"name": "CHECKBOX_VALUE_ACCESSOR"
},
Expand Down Expand Up @@ -1418,6 +1421,9 @@
{
"name": "setDirectiveInputsWhichShadowsStyling"
},
{
"name": "setDisabledStateDefault"
},
{
"name": "setIncludeViewProviders"
},
Expand Down
6 changes: 6 additions & 0 deletions packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json
View file
Expand Up @@ -59,6 +59,9 @@
{
"name": "BuiltInControlValueAccessor"
},
{
"name": "CALL_SET_DISABLED_STATE"
},
{
"name": "CHECKBOX_VALUE_ACCESSOR"
},
Expand Down Expand Up @@ -1394,6 +1397,9 @@
{
"name": "setDirectiveInputsWhichShadowsStyling"
},
{
"name": "setDisabledStateDefault"
},
{
"name": "setIncludeViewProviders"
},
Expand Down
1 change: 1 addition & 0 deletions packages/forms/src/directives.ts
View file
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
View file
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) {
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
View file
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) {
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
10 changes: 6 additions & 4 deletions packages/forms/src/directives/reactive_directives/form_control_directive.ts
View file
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) {
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
12 changes: 7 additions & 5 deletions packages/forms/src/directives/reactive_directives/form_group_directive.ts
View file
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) {
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

0 comments on commit 96b7fe9

Please sign in to comment.