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

fix(ivy): don't infer template context types when in full mode #33537

Closed
wants to merge 1 commit into from

Conversation

@alxhub
Copy link
Contributor

alxhub commented Nov 1, 2019

fix(ivy): don't infer template context types when in full mode

The Ivy template type-checker is capable of inferring the type of a
structural directive (such as NgForOf). Previously, this was done with
fullTemplateTypeCheck: true, even if strictTemplates was false. View Engine
previously did not do this inference, and so this causes breakages if the
type of the template context is not what the user expected.

In particular, consider the template:

<div *ngFor="let user of users as all">
  {{user.index}} out of {{all.length}}
</div>

As long as users is an array, this seems reasonable, because it appears
that all is an alias for the users array. However, this is misleading.

In reality, NgForOf is rendered with a template context that contains
both a $implicit value (for the loop variable user) as well as a
ngForOf value, which is the actual value assigned to all. The type of
NgForOf's template context is NgForContext<T>, which declares ngForOf's
type to be NgIterable<T>, which does not have a length property (due to
its incorporation of the Iterable type).

This commit stops the template type-checker from inferring template context
types unless strictTemplates is set (and strictInputTypes is not disabled).

Fixes #33527.

@alxhub alxhub requested review from angular/fw-compiler as code owners Nov 1, 2019
@googlebot googlebot added the cla: yes label Nov 1, 2019
@ngbot ngbot bot added this to the needsTriage milestone Nov 4, 2019
@JoostK JoostK mentioned this pull request Nov 5, 2019
3 of 14 tasks complete
@alxhub alxhub force-pushed the alxhub:ngtsc/ttc/context-inference branch 2 times, most recently from 82d0bc6 to 022c5d7 Nov 18, 2019
@alxhub alxhub requested a review from angular/dev-infra-framework as a code owner Nov 18, 2019
The Ivy template type-checker is capable of inferring the type of a
structural directive (such as NgForOf<T>). Previously, this was done with
fullTemplateTypeCheck: true, even if strictTemplates was false. View Engine
previously did not do this inference, and so this causes breakages if the
type of the template context is not what the user expected.

In particular, consider the template:

```html
<div *ngFor="let user of users as all">
  {{user.index}} out of {{all.length}}
</div>
```

As long as `users` is an array, this seems reasonable, because it appears
that `all` is an alias for the `users` array. However, this is misleading.

In reality, `NgForOf` is rendered with a template context that contains
both a `$implicit` value (for the loop variable `user`) as well as a
`ngForOf` value, which is the actual value assigned to `all`. The type of
`NgForOf`'s template context is `NgForContext<T>`, which declares `ngForOf`'s
type to be `NgIterable<T>`, which does not have a `length` property (due to
its incorporation of the `Iterable` type).

This commit stops the template type-checker from inferring template context
types unless strictTemplates is set (and strictInputTypes is not disabled).

Fixes #33527.
@alxhub alxhub force-pushed the alxhub:ngtsc/ttc/context-inference branch from 022c5d7 to 97ff38e Nov 19, 2019
@alxhub

This comment has been minimized.

Copy link
Contributor Author

alxhub commented Nov 19, 2019

Presubmit
No Ivy presubmit needed; only affects template type-checking in fTTC mode.

@ngbot

This comment has been minimized.

Copy link

ngbot bot commented Nov 20, 2019

I see that you just added the PR action: merge label, but the following checks are still failing:
    failure status "ci/circleci: test_aio_local" is failing
    failure status "ci/circleci: test_aio_local_viewengine" is failing
    pending missing required status "ci/circleci: publish_snapshot"

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken master, please try rebasing to master and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

@alxhub alxhub closed this in eb6975a Nov 20, 2019
alxhub added a commit that referenced this pull request Nov 20, 2019
The Ivy template type-checker is capable of inferring the type of a
structural directive (such as NgForOf<T>). Previously, this was done with
fullTemplateTypeCheck: true, even if strictTemplates was false. View Engine
previously did not do this inference, and so this causes breakages if the
type of the template context is not what the user expected.

In particular, consider the template:

```html
<div *ngFor="let user of users as all">
  {{user.index}} out of {{all.length}}
</div>
```

As long as `users` is an array, this seems reasonable, because it appears
that `all` is an alias for the `users` array. However, this is misleading.

In reality, `NgForOf` is rendered with a template context that contains
both a `$implicit` value (for the loop variable `user`) as well as a
`ngForOf` value, which is the actual value assigned to `all`. The type of
`NgForOf`'s template context is `NgForContext<T>`, which declares `ngForOf`'s
type to be `NgIterable<T>`, which does not have a `length` property (due to
its incorporation of the `Iterable` type).

This commit stops the template type-checker from inferring template context
types unless strictTemplates is set (and strictInputTypes is not disabled).

Fixes #33527.

PR Close #33537
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.