refactor(forms): deprecate ngModel usage on same field as formControl #22633

kara · 2018-03-07T19:30:30Z

Support for using the ngModel input property and ngModelChange event with reactive form directives has been deprecated in Angular v6 and will be removed in Angular v7.

Now deprecated:

<input [formControl]="control" [(ngModel)]="value">

this.value = 'some value';

This is being deprecated for a few reasons. First, developers have found this pattern confusing. It seems like the actual ngModel directive is being used, but in fact it's an input/output property named ngModel on the reactive form directive that simply approximates (some of) its behavior. Specifically, it allows getting/setting the value and intercepting value events. However, some of ngModel's other features - like delaying updates withngModelOptions or exporting the directive - simply don't work, which has understandably caused some confusion.

In addition, this pattern mixes template-driven and reactive forms strategies, which we generally don't recommend because it doesn't take advantage of the full benefits of either strategy. Setting the value in the template violates the template-agnostic principles behind reactive forms, whereas adding a FormControl/FormGroup layer in the class removes the convenience of defining forms in the template.

To update your code before v7, you'll want to decide whether to stick with reactive form directives (and get/set values using reactive forms patterns) or switch over to template-driven directives.

After (choice 1 - use reactive forms):

<input [formControl]="control">

this.control.setValue('some value');

After (choice 2 - use template-driven forms):

<input [(ngModel)]="value">

this.value = 'some value';

This PR adds a deprecation warning that will show once in dev mode when using the pattern above.
You can choose to silence this warning by providing a config for ReactiveFormsModule at import time:

imports: [
  ReactiveFormsModule.withConfig({warnOnNgModelWithFormControl: 'never'});
]

Alternatively, you can choose to surface a separate warning for each instance of this pattern with a config value of "always". This may help to track down where in the code the pattern is being used as the code is being updated.

Note: warnOnNgModelWithFormControl is set up as deprecated so that it can be removed in v7 when it is no longer needed. This will not display properly in API docs yet because dgeni doesn't yet support deprecating properties in object literals, but we have an open issue to resolve the discrepancy here.

mary-poppins · 2018-03-07T19:39:31Z

You can preview 7bf5eb5 at https://pr22633-7bf5eb5.ngbuilds.io/.

mary-poppins · 2018-03-07T20:27:18Z

You can preview 37e266c at https://pr22633-37e266c.ngbuilds.io/.

JoostK · 2018-03-07T20:35:53Z

packages/forms/src/form_providers.ts

@@ -35,4 +36,10 @@ export class FormsModule {
  exports: [InternalFormsSharedModule, REACTIVE_DRIVEN_DIRECTIVES]
 })
 export class ReactiveFormsModule {
+  static withConfig(opts: {silenceNgModelWarning: boolean}): ModuleWithProviders {


Should we deprecate the silenceNgModelWarning option immediately? It will allow for removal with Angular 7 once the property does not make sense anymore.

Discussed with @IgorMinar and we think this is probably a good idea.

IgorMinar · 2018-03-07T23:22:36Z

tools/public_api_guard/forms/forms.d.ts

@@ -491,6 +491,9 @@ export declare class RadioControlValueAccessor implements ControlValueAccessor,

 /** @stable */
 export declare class ReactiveFormsModule {
+    static withConfig(opts: {
+        silenceNgModelWarning: boolean;


I suggest that you change the option to something very explicit. e.g. warnOnNgModelWithFormControl and make it a string with options: "never" | "once" | "always"

IgorMinar · 2018-03-07T23:23:37Z

btw awesome commit message. thanks for documenting this change well.

mary-poppins · 2018-03-08T01:24:45Z

You can preview fd25c6f at https://pr22633-fd25c6f.ngbuilds.io/.

IgorMinar

otherwise lgtm

IgorMinar · 2018-03-08T01:23:37Z

packages/forms/src/form_providers.ts

@@ -35,4 +36,15 @@ export class FormsModule {
  exports: [InternalFormsSharedModule, REACTIVE_DRIVEN_DIRECTIVES]
 })
 export class ReactiveFormsModule {
+  static withConfig(opts: {
+    /** @deprecated from v6 */ warnOnNgModelWithFormControl: 'never' | 'once' | 'always'


from => as of

IgorMinar · 2018-03-08T01:25:44Z

packages/forms/src/directives/reactive_errors.ts

@@ -74,4 +74,31 @@ export class ReactiveErrors {
      });
    `);
  }
+
+  static ngModelWarning(directiveName: string): void {
+    console.warn(`


will we be able to tree-shake/DCE this? I think only closure will be able to do it. am I wrong?

I suggest you make the error message shorter and just add a link to our docs where we explain this in greater detail.

mary-poppins · 2018-03-08T01:30:36Z

You can preview 89285b9 at https://pr22633-89285b9.ngbuilds.io/.

mary-poppins · 2018-03-08T02:07:35Z

You can preview 7a04ccf at https://pr22633-7a04ccf.ngbuilds.io/.

kara · 2018-03-08T02:11:56Z

presubmit

mary-poppins · 2018-03-08T02:14:21Z

You can preview 3704a23 at https://pr22633-3704a23.ngbuilds.io/.

mary-poppins · 2018-03-08T02:19:06Z

You can preview edf15f3 at https://pr22633-edf15f3.ngbuilds.io/.

Support for using the `ngModel` input property and `ngModelChange` event with reactive form directives has been deprecated in Angular v6 and will be removed in Angular v7. Now deprecated: ```html <input [formControl]="control" [(ngModel)]="value"> ``` ```ts this.value = 'some value'; ``` This has been deprecated for a few reasons. First, developers have found this pattern confusing. It seems like the actual `ngModel` directive is being used, but in fact it's an input/output property named `ngModel` on the reactive form directive that simply approximates (some of) its behavior. Specifically, it allows getting/setting the value and intercepting value events. However, some of `ngModel`'s other features - like delaying updates with`ngModelOptions` or exporting the directive - simply don't work, which has understandably caused some confusion. In addition, this pattern mixes template-driven and reactive forms strategies, which we generally don't recommend because it doesn't take advantage of the full benefits of either strategy. Setting the value in the template violates the template-agnostic principles behind reactive forms, whereas adding a FormControl/FormGroup layer in the class removes the convenience of defining forms in the template. To update your code before v7, you'll want to decide whether to stick with reactive form directives (and get/set values using reactive forms patterns) or switch over to template-driven directives. After (choice 1 - use reactive forms): ```html <input [formControl]="control"> ``` ```ts this.control.setValue('some value'); ``` After (choice 2 - use template-driven forms): ```html <input [(ngModel)]="value"> ``` ```ts this.value = 'some value'; ``` You can also choose to silence this warning by providing a config for `ReactiveFormsModule` at import time: ```ts imports: [ ReactiveFormsModule.withConfig({warnOnNgModelWithFormControl: 'never'}); ] ``` Alternatively, you can choose to surface a separate warning for each instance of this pattern with a config value of `"always"`. This may help to track down where in the code the pattern is being used as the code is being updated. Note: `warnOnNgModelWithFormControl` is set up as deprecated so that it can be removed in v7 when it is no longer needed. This will not display properly in API docs yet because dgeni doesn't yet support deprecating properties in object literals, but we have an open issue to resolve the discrepancy here: angular#22640.

mary-poppins · 2018-03-08T02:57:30Z

You can preview 7a309cc at https://pr22633-7a309cc.ngbuilds.io/.

…angular#22633) Support for using the `ngModel` input property and `ngModelChange` event with reactive form directives has been deprecated in Angular v6 and will be removed in Angular v7. Now deprecated: ```html <input [formControl]="control" [(ngModel)]="value"> ``` ```ts this.value = 'some value'; ``` This has been deprecated for a few reasons. First, developers have found this pattern confusing. It seems like the actual `ngModel` directive is being used, but in fact it's an input/output property named `ngModel` on the reactive form directive that simply approximates (some of) its behavior. Specifically, it allows getting/setting the value and intercepting value events. However, some of `ngModel`'s other features - like delaying updates with`ngModelOptions` or exporting the directive - simply don't work, which has understandably caused some confusion. In addition, this pattern mixes template-driven and reactive forms strategies, which we generally don't recommend because it doesn't take advantage of the full benefits of either strategy. Setting the value in the template violates the template-agnostic principles behind reactive forms, whereas adding a FormControl/FormGroup layer in the class removes the convenience of defining forms in the template. To update your code before v7, you'll want to decide whether to stick with reactive form directives (and get/set values using reactive forms patterns) or switch over to template-driven directives. After (choice 1 - use reactive forms): ```html <input [formControl]="control"> ``` ```ts this.control.setValue('some value'); ``` After (choice 2 - use template-driven forms): ```html <input [(ngModel)]="value"> ``` ```ts this.value = 'some value'; ``` You can also choose to silence this warning by providing a config for `ReactiveFormsModule` at import time: ```ts imports: [ ReactiveFormsModule.withConfig({warnOnNgModelWithFormControl: 'never'}); ] ``` Alternatively, you can choose to surface a separate warning for each instance of this pattern with a config value of `"always"`. This may help to track down where in the code the pattern is being used as the code is being updated. Note: `warnOnNgModelWithFormControl` is set up as deprecated so that it can be removed in v7 when it is no longer needed. This will not display properly in API docs yet because dgeni doesn't yet support deprecating properties in object literals, but we have an open issue to resolve the discrepancy here: angular#22640. PR Close angular#22633

…rectives BREAKING CHANGE: In v6, we deprecated support for the ngModel and ngModelChange inputs on reactive form directives. We are now removing these deprecated APIs for v8. See the original change here for more info on why: angular#22633.

…rectives Closes angular#12708. BREAKING CHANGE: In v6, we deprecated support for the ngModel and ngModelChange inputs on reactive form directives. We are now removing these deprecated APIs for v8. See the original change here for more info on why: angular#22633.

angular-automatic-lock-bot · 2019-09-14T04:22:57Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

googlebot added the cla: yes label Mar 7, 2018

kara added the state: WIP label Mar 7, 2018

kara requested review from StephenFluin and IgorMinar March 7, 2018 19:30

kara added the area: forms label Mar 7, 2018

StephenFluin approved these changes Mar 7, 2018

View reviewed changes

kara force-pushed the deprecate-ngModel branch from 7bf5eb5 to 37e266c Compare March 7, 2018 20:18

kara added action: review The PR is still awaiting reviews from at least one requested reviewer refactoring Issue that involves refactoring or code-cleanup and removed state: WIP labels Mar 7, 2018

JoostK reviewed Mar 7, 2018

View reviewed changes

IgorMinar suggested changes Mar 7, 2018

View reviewed changes

kara force-pushed the deprecate-ngModel branch 2 times, most recently from fd25c6f to 89285b9 Compare March 8, 2018 01:20

IgorMinar approved these changes Mar 8, 2018

View reviewed changes

kara force-pushed the deprecate-ngModel branch 2 times, most recently from 7a04ccf to 3704a23 Compare March 8, 2018 02:06

kara force-pushed the deprecate-ngModel branch from 3704a23 to edf15f3 Compare March 8, 2018 02:11

kara added target: major This PR is targeted for the next major release and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Mar 8, 2018

kara force-pushed the deprecate-ngModel branch from edf15f3 to 7a309cc Compare March 8, 2018 02:47

kara added the action: merge The PR is ready for merge by the caretaker label Mar 8, 2018

kara closed this in 8fb34bc Mar 8, 2018

trotyl mentioned this pull request Mar 16, 2018

Should not allow to use ngModel as input value when using ReactiveFormsModule #12708

Closed

chunghha mentioned this pull request May 2, 2018

deprecate ngModel usage on same field as formControl angular/angular-update-guide#11

Closed

kara deleted the deprecate-ngModel branch October 13, 2018 01:09

kara mentioned this pull request Feb 27, 2019

fix(forms): remove deprecated support for ngModel on reactive form directives #28996

Closed

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 14, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(forms): deprecate ngModel usage on same field as formControl #22633

refactor(forms): deprecate ngModel usage on same field as formControl #22633

kara commented Mar 7, 2018 •

edited

mary-poppins commented Mar 7, 2018

mary-poppins commented Mar 7, 2018

JoostK Mar 7, 2018

kara Mar 8, 2018

IgorMinar Mar 7, 2018

IgorMinar commented Mar 7, 2018

mary-poppins commented Mar 8, 2018

IgorMinar left a comment

IgorMinar Mar 8, 2018

IgorMinar Mar 8, 2018

IgorMinar Mar 8, 2018

mary-poppins commented Mar 8, 2018

mary-poppins commented Mar 8, 2018

kara commented Mar 8, 2018

mary-poppins commented Mar 8, 2018

mary-poppins commented Mar 8, 2018

mary-poppins commented Mar 8, 2018

angular-automatic-lock-bot bot commented Sep 14, 2019

refactor(forms): deprecate ngModel usage on same field as formControl #22633

refactor(forms): deprecate ngModel usage on same field as formControl #22633

Conversation

kara commented Mar 7, 2018 • edited

mary-poppins commented Mar 7, 2018

mary-poppins commented Mar 7, 2018

JoostK Mar 7, 2018

Choose a reason for hiding this comment

kara Mar 8, 2018

Choose a reason for hiding this comment

IgorMinar Mar 7, 2018

Choose a reason for hiding this comment

IgorMinar commented Mar 7, 2018

mary-poppins commented Mar 8, 2018

IgorMinar left a comment

Choose a reason for hiding this comment

IgorMinar Mar 8, 2018

Choose a reason for hiding this comment

IgorMinar Mar 8, 2018

Choose a reason for hiding this comment

IgorMinar Mar 8, 2018

Choose a reason for hiding this comment

mary-poppins commented Mar 8, 2018

mary-poppins commented Mar 8, 2018

kara commented Mar 8, 2018

mary-poppins commented Mar 8, 2018

mary-poppins commented Mar 8, 2018

mary-poppins commented Mar 8, 2018

angular-automatic-lock-bot bot commented Sep 14, 2019

kara commented Mar 7, 2018 •

edited