Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(forms): expand NgModel disabled type to work with strict template type checking #34438

Conversation

@devversion
Copy link
Member

devversion commented Dec 16, 2019

NgModel internally coerces any arbitrary value that will assigned
to the disabled @Input to a boolean. This has been done to
support the common case where developers set the disabled attribute
without a value. For example:

<input type="checkbox" [(ngModel)]="value" disabled>

This worked in View Engine without any errors because inputs were
not strictly checked. In Ivy though, developers can opt-in into
strict template type checking where the attribute would be flagged.

This is because the NgModel#isDisabled property type-wise only
accepts a boolean. To ensure that the common pattern described
above can still be used, and to reflect the actual runtime behavior,
we should add an acceptance member that makes it work without type
checking errors.

Using a coercion member means that this is not a breaking change.

Note: I tried creating a test for this.. but it looks like there is no way to AOT compile a component (replicating the scenario above) with strict template checking enabled. Also see #33452

@kara
kara approved these changes Dec 16, 2019
Copy link
Contributor

kara left a comment

LGTM once typo is fixed and CI is green

packages/forms/src/directives/ng_model.ts Outdated Show resolved Hide resolved
…e type checking

NgModel internally coerces any arbitrary value that will assigned
to the `disabled` `@Input` to a boolean. This has been done to
support the common case where developers set the disabled attribute
without a value. For example:

```html
<input type="checkbox" [(ngModel)]="value" disabled>
```

This worked in View Engine without any errors because inputs were
not strictly checked. In Ivy though, developers can opt-in into
strict template type checking where the attribute would be flagged.

This is because the `NgModel#isDisabled` property type-wise only
accepts a `boolean`. To ensure that the common pattern described
above can still be used, and to reflect the actual runtime behavior,
we should add an acceptance member that makes it work without type
checking errors.

Using a coercion member means that this is not a breaking change.
@devversion devversion force-pushed the devversion:fix/forms-allow-dissabled-input-coercion branch from af51e01 to 6d7ea64 Dec 16, 2019
@devversion devversion requested a review from angular/fw-public-api as a code owner Dec 16, 2019
@alxhub
alxhub approved these changes Dec 16, 2019
@devversion

This comment has been minimized.

Copy link
Member Author

devversion commented Dec 16, 2019

@kara can you have another look? I need a global review or from fw-forms and fw-public-api.

@kara
kara approved these changes Dec 16, 2019
Copy link
Contributor

kara left a comment

LGTM (global approval)

@kara kara removed their assignment Dec 16, 2019
@kara

This comment has been minimized.

Copy link
Contributor

kara commented Dec 16, 2019

kara added a commit that referenced this pull request Dec 16, 2019
…e type checking (#34438)

NgModel internally coerces any arbitrary value that will assigned
to the `disabled` `@Input` to a boolean. This has been done to
support the common case where developers set the disabled attribute
without a value. For example:

```html
<input type="checkbox" [(ngModel)]="value" disabled>
```

This worked in View Engine without any errors because inputs were
not strictly checked. In Ivy though, developers can opt-in into
strict template type checking where the attribute would be flagged.

This is because the `NgModel#isDisabled` property type-wise only
accepts a `boolean`. To ensure that the common pattern described
above can still be used, and to reflect the actual runtime behavior,
we should add an acceptance member that makes it work without type
checking errors.

Using a coercion member means that this is not a breaking change.

PR Close #34438
@kara kara closed this in 5cecd97 Dec 16, 2019
@angular-automatic-lock-bot

This comment has been minimized.

Copy link

angular-automatic-lock-bot bot commented Jan 16, 2020

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jan 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants
You can’t perform that action at this time.