feat: expand input types with coercion to work with ngtsc input type checking #17528

devversion · 2019-10-28T19:28:20Z

Related to #17495

devversion · 2019-10-28T21:54:59Z

Blocked on:

bazel: unable to enable ngtsc strict template type checking angular#33452. We can workaround this and patch @angular/bazel in the meanwhile
Resolution: There will be no changes to @angular/bazel since we need to audit how type checking should be configurable in Bazel. We will work around this from our side.
Input coercion static members cannot be used with NGC and strictMetadataEmit enabled angular#33451. We could disable strictMetadataEmit for the legacy build. This opens up the other question: why is this not enabled within Bazel?
Possible solutions: We could add @dynamic to every class with such coercion members, or we set the members to null! (downside of generating code), or just add an exception to NGC metadata collector.
NgModel has an input called disabled that conflicts with the coerced disabled input if used on the same element. e.g. <mat-slider disabled ngModel>
Possible solutions: Updating NgModel to also support coercion of the disabled input. Though IMO this is more like a work around. The error is actually valid and it shows that two inputs conflict in type, and only a value intersecting both types can be assigned. This makes sense to me. Since for NgModel it can be common that someone relies on the empty disabled attribute, it could make sense to update the input type.. though it can be a general problem for other directives too I guess

For now, I just disabled the type checking on attributes which apply to inputs. That's exactly the use-case on why this strictness flag exists. We can re-enable this once we figured out how what to do with NgModel.

Adds a custom tslint rule to enforce that properties which use coercion in a setter also declare a static property to indicate the accepted types to ngtsc. Also handles inherited setters and properties coming from an interface being implemented (necessary to support mixins). Relates to angular#17528.

jelbourn

LGTM

andrewseguin · 2019-10-31T17:20:23Z

Needs rebase

…checking

…members See: angular/angular#33451

…king Workaround for: angular/angular#33452

Workaround for angular/angular#33451. This makes sense at the current time since the Bazel output does *never* work with strict metadata emit... and considering that with Ivy there is no metadata anyway, it could be disabled in the meanwhile.

We are temporarily disabling strict attribute type checking since we have a few templates that set a coerced input while the `NgModel` directive is applied. In those cases, the empty string for the `disabled` input, is not assignable to the `NgModel#disabled` input, since it does not do coercion. We need to figure out what to do in those scenarios.

devversion · 2019-10-31T18:24:38Z

@andrewseguin Rebased. I had to fix a couple of newly introduced coercion inputs, so I think as soon as this is green, it would be good to merge it.

Adds a custom tslint rule to enforce that properties which use coercion in a setter also declare a static property to indicate the accepted types to ngtsc. Also handles inherited setters and properties coming from an interface being implemented (necessary to support mixins). Relates to angular#17528.

manklu · 2019-11-01T10:10:03Z

There is still a problem with async pipe:

TS2326: Types of property 'opened' are incompatible.
  Type 'boolean | null' is not assignable to type 'string | boolean | undefined'.

#17536) Adds a custom tslint rule to enforce that properties which use coercion in a setter also declare a static property to indicate the accepted types to ngtsc. Also handles inherited setters and properties coming from an interface being implemented (necessary to support mixins). Relates to #17528.

Follow-up from angular#17528 that: * Turns on the `coercion-types` rule. * Fixes the rule not detecting abstract directives properly. * Fixes the rule incorrectly flagging coercion functions used inside of callbacks which are inside of setters. * Either fixes or works around a final set of failures.

Follow-up from #17528 that: * Turns on the `coercion-types` rule. * Fixes the rule not detecting abstract directives properly. * Fixes the rule incorrectly flagging coercion functions used inside of callbacks which are inside of setters. * Either fixes or works around a final set of failures.

angular-automatic-lock-bot · 2019-12-02T00:05:37Z

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 PR author has agreed to Google's Contributor License Agreement label Oct 28, 2019

devversion force-pushed the accept-input-type branch from aec46dd to 98f0d18 Compare October 28, 2019 21:42

devversion force-pushed the accept-input-type branch 5 times, most recently from c6c7a1e to c163301 Compare October 29, 2019 10:23

crisbeto mentioned this pull request Oct 29, 2019

build: add lint rule to enforce coercion static properties for setters #17536

Merged

devversion force-pushed the accept-input-type branch 2 times, most recently from 90c626d to 5e696c1 Compare October 30, 2019 09:15

devversion marked this pull request as ready for review October 30, 2019 15:45

devversion requested review from andrewseguin, crisbeto, jelbourn, mmalerba and a team as code owners October 30, 2019 15:45

devversion added the target: major This PR is targeted for the next major release label Oct 30, 2019

devversion added this to the 9.0.0 milestone Oct 30, 2019

jelbourn approved these changes Oct 30, 2019

View reviewed changes

andrewseguin added pr: needs rebase and removed action: merge The PR is ready for merge by the caretaker labels Oct 31, 2019

devversion force-pushed the accept-input-type branch from 5e696c1 to 2b0a19e Compare October 31, 2019 18:17

devversion force-pushed the accept-input-type branch from 2b0a19e to e2f29ab Compare October 31, 2019 18:21

jelbourn and others added 5 commits October 31, 2019 19:23

feat: expand input types with coercion to work with ngtsc input type …

fc51dcc

…checking

refactor: add workaround for strict metadata emit of coercion static …

35430ff

…members See: angular/angular#33451

build: patch ng_module bazel rule to enable strict template type chec…

2ab27c6

…king Workaround for: angular/angular#33452

devversion force-pushed the accept-input-type branch from e2f29ab to d9d5683 Compare October 31, 2019 18:23

devversion added action: merge The PR is ready for merge by the caretaker and removed pr: needs rebase labels Oct 31, 2019

andrewseguin merged commit 8da64f4 into angular:master Oct 31, 2019

crisbeto mentioned this pull request Nov 4, 2019

build: enable coercion-types rule and fix leftover cases #17609

Merged

angular-automatic-lock-bot bot locked and limited conversation to collaborators Dec 2, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: expand input types with coercion to work with ngtsc input type checking #17528

feat: expand input types with coercion to work with ngtsc input type checking #17528

devversion commented Oct 28, 2019

devversion commented Oct 28, 2019 •

edited

jelbourn left a comment

andrewseguin commented Oct 31, 2019

devversion commented Oct 31, 2019 •

edited

manklu commented Nov 1, 2019

angular-automatic-lock-bot bot commented Dec 2, 2019

feat: expand input types with coercion to work with ngtsc input type checking #17528

feat: expand input types with coercion to work with ngtsc input type checking #17528

Conversation

devversion commented Oct 28, 2019

devversion commented Oct 28, 2019 • edited

jelbourn left a comment

Choose a reason for hiding this comment

andrewseguin commented Oct 31, 2019

devversion commented Oct 31, 2019 • edited

manklu commented Nov 1, 2019

angular-automatic-lock-bot bot commented Dec 2, 2019

devversion commented Oct 28, 2019 •

edited

devversion commented Oct 31, 2019 •

edited