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): generate a better error for template var writes #34339

Closed
wants to merge 1 commit into from

Conversation

@alxhub
Copy link
Contributor

alxhub commented Dec 11, 2019

In Ivy it's illegal for a template to write to a template variable. So the
template:

<ng-template let-somevar>
  <button (click)="somevar = 3">Set var to 3</button>
</ng-template>

is erroneous and previously would fail to compile with an assertion error
from the TemplateDefinitionBuilder. This error wasn't particularly user-
friendly, though, as it lacked the context of which template or where the
error occurred.

In this commit, a new check in template type-checking is added which detects
such erroneous writes and produces a true diagnostic with the appropriate
context information.

@alxhub alxhub requested a review from angular/fw-compiler as a code owner Dec 11, 2019
@googlebot googlebot added the cla: yes label Dec 11, 2019
@googlebot googlebot added the cla: yes label Dec 11, 2019
@alxhub alxhub force-pushed the alxhub:ngtsc/template-var-assignment branch from bb269b1 to 8ea6d45 Dec 11, 2019
@JoostK
JoostK approved these changes Dec 11, 2019
Copy link
Member

JoostK left a comment

Looks good, with one comment

@@ -972,6 +981,16 @@ class TcbExpressionTranslator {
// returned here to let it fall through resolution so it will be caught when the
// `ImplicitReceiver` is resolved in the branch below.
return this.resolveTarget(ast);
} else if (ast instanceof PropertyWrite && ast.receiver instanceof ImplicitReceiver) {

This comment has been minimized.

Copy link
@JoostK

JoostK Dec 11, 2019

Member

Consider adding a test in type_check_block_spec.ts for this case.

This comment has been minimized.

Copy link
@alxhub

alxhub Dec 12, 2019

Author Contributor

Done.

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

kara left a comment

LGTM, but can you add Closes #33674 to the bottom of the commit?

In Ivy it's illegal for a template to write to a template variable. So the
template:

```html
<ng-template let-somevar>
  <button (click)="somevar = 3">Set var to 3</button>
</ng-template>
```

is erroneous and previously would fail to compile with an assertion error
from the `TemplateDefinitionBuilder`. This error wasn't particularly user-
friendly, though, as it lacked the context of which template or where the
error occurred.

In this commit, a new check in template type-checking is added which detects
such erroneous writes and produces a true diagnostic with the appropriate
context information.

Closes #33674
@alxhub alxhub force-pushed the alxhub:ngtsc/template-var-assignment branch from 8ea6d45 to 8486471 Dec 12, 2019
@alxhub

This comment has been minimized.

Copy link
Contributor Author

alxhub commented Dec 12, 2019

@kara kara closed this in 6ba5fdc Dec 12, 2019
kara added a commit that referenced this pull request Dec 12, 2019
In Ivy it's illegal for a template to write to a template variable. So the
template:

```html
<ng-template let-somevar>
  <button (click)="somevar = 3">Set var to 3</button>
</ng-template>
```

is erroneous and previously would fail to compile with an assertion error
from the `TemplateDefinitionBuilder`. This error wasn't particularly user-
friendly, though, as it lacked the context of which template or where the
error occurred.

In this commit, a new check in template type-checking is added which detects
such erroneous writes and produces a true diagnostic with the appropriate
context information.

Closes #33674

PR Close #34339
@angular-automatic-lock-bot

This comment has been minimized.

Copy link

angular-automatic-lock-bot bot commented Jan 12, 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 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.