angular 9 / Ivy => async 'as' leads to error

[mpalourdio]

🐞 bug report

Affected Package

https://github.com/mpalourdio/ng2

Is this a regression?

Yes, the previous version in which this bug was not present is angular8@latest.

Description

<div id="app" *ngIf="(applicationsList$ | async) as applicationsList">
    <app-search-filter [(applicationsList)]="applicationsList"></app-search-filter>
</div>

yarn start

Error : Property 'applicationsList' does not exist on type 'ChildComponent'. Did you mean 'applicationsList$'?

Disabling Ivy leads to

ERROR in Cannot assign value "$event" to template variable "applicationsList". Template variables are read-only.

🔬 Minimal Reproduction

• clone https://github.com/mpalourdio/ng2
• run ng update @angular/cli @angular/core --next
• yarn start

🔥 Exception or Error

 Property 'applicationsList' does not exist on type 'ChildComponent'. Did you mean 'applicationsList$'?

Disabling Ivy leads to

ERROR in Cannot assign value "$event" to template variable "applicationsList". Template variables are read-only.

🌍 Your Environment

Angular Version:

Angular CLI: 9.0.0-rc.6
Node: 12.13.0
OS: linux x64

Angular: 9.0.0-rc.6
... animations, cli, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router
Ivy Workspace: Yes

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.900.0-rc.6
@angular-devkit/build-angular     0.900.0-rc.6
@angular-devkit/build-optimizer   0.900.0-rc.6
@angular-devkit/build-webpack     0.900.0-rc.6
@angular-devkit/core              9.0.0-rc.6
@angular-devkit/schematics        9.0.0-rc.6
@ngtools/webpack                  9.0.0-rc.6
@schematics/angular               9.0.0-rc.6
@schematics/update                0.900.0-rc.6
rxjs                              6.5.3
typescript                        3.6.4
webpack                           4.41.2

---

EDIT (by gkalpak)

Adding the conclusions from #34405 (comment) (after the discussion below) here for visibility:

The issue is that this breaking change is not (clearly) documented, so let's keep this open to track that 😃

Action items:

• Figure out what the breaking change is exactly (incl. how template variables created via as are different from other template variables).
• Document that.

[alexzuza]

Looks like it was made intentionally according to Ivy Compatibility Guide

It is now an error to assign values to template-only variables like item in ngFor="let item of items" (previously, the compiler would ignore these assignments).

[mpalourdio]

Thanks for the answer. Looks like a rough BC break, as this pattern looks very common to me. Anyway, how are we supposed to refactor this kind of code ?

And as I said, it does not look like Ivy specific, as disabling Ivy pops an error too regarding template variable assignment.

Thanks again for feedback.

[gkalpak]

It is not clear what you are trying to achieve (i.e. what the expected/ideal behavior would be) with [(applicationsList)]="applicationsList".

Just to be clear, the error is not related to async ... as (as mentioned in the title), but to the following bit of code:

<app-search-filter [(applicationsList)]="applicationsList"></app-search-filter>

This is syntactic sugar for a two-way binding. It is equivalent to:

<app-search-filter [applicationsList]="applicationsList" (applicationsListChange)="applicationsList = $event"></app-search-filter>

It's this second part ((applicationsListChange)="applicationsList = $event") that seems to be causing the error. It is not clear what you are trying to achieve by setting the template applicationList variable to the filtered result of itself. Maybe the example you gave is contrived and does not reflect the actual usecase. Could you, please, clarify the usecase?

(Finally, as mentioned in the guide, the compiler would previouly ignore the assignments, so this change will not break something that used to work before. It will only turn silent failing at runtime to hard failing at compile time.)

[JoostK]

The error in Ivy was incorrect, this has been resolved in #34339 (which just missed the rc.6 release).

Other than that: what @gkalpak said :-)

[mpalourdio]

It's this second part ((applicationsListChange)="applicationsList = $event") that seems to be causing the error. It is not clear what you are trying to achieve by setting the template applicationList variable to the filtered result of itself. Maybe the example you gave is contrived and does not reflect the actual usecase. Could you, please, clarify the usecase?

Indeed, this repository is just a sandbox to experiment misc things in a quick and dirty way. The full code is irrelevant (even if working with angular 8) and clumsy.

The main idea is just giving the aliased ('as') observable (an array of strings) from a parent component to a child component, filtering the array in the child component based on a search term the user types in a simple input field, and with the 'banana in the box', displaying the filtered array in the parent component. This is something that works today with ng8, it does not fail silently at runtime as you suppose it does. Again, It's not a real world app, but it just works.

. It will only turn silent failing at runtime to hard failing at compile time.)

It just does not fail at runtime today :)

Here is the full extract

<div id="app" *ngIf="(applicationsList$ | async) as applicationsList">
    <app-search-filter [(applicationsList)]="applicationsList"></app-search-filter>
    <table id="mytable">
        <tr *ngFor="let application of applicationsList">
            <td>
                <fav-star [application]="application">
                    {{application.name}} <span *ngIf="application.isFav">(faved !)</span>
                </fav-star>
            </td>
        </tr>
    </table>
    {{applicationsList | json}}
</div>

And just to be clear, without Ivy, it fails too. I understand that this not acceptable anymore in ng9, Ivy or not, but I think that this kind of double-binding is quite common. (ie, passing a value from parent to child, and displaying in the parent a computed value).

BC breaks are really acceptable, particularly in a 9th version :) I just think this is something that is over used today. But I have no problem in the fact that this not a possibility anymore. But I'm exploring migration path from v8 to v9.

Thanks for your valuable feedback.

[JoostK]

Thanks for clarifying. That sounds weird, though. Could you show how the filtering is done? What is important here is whether that is done by mutating an array or by creating a new one.

[mpalourdio]

This is done here : https://github.com/mpalourdio/ng2/blob/7f3a9b6114c04b37c664fe55a73ee1bd06fa061d/src/app/search-filter/search-filter.component.ts#L33

Feel free to just clone this repo, running it on ng8 (current yarn.lock), and upgrading it to angular@next if needed.

It emits a new array from child to parent (so the child keeps track of the original array) (== array#filter), so there's no mutation of the original reference. The ugly this._appListBuffer = JSON.parse(JSON.stringify(this.applicationsList)); in ngOnInit is just the poor (lazy) man's clone version. Again, it's just a sandbox project xD

[JoostK]

@mpalourdio I am amazed this ever worked!

[mpalourdio]

That's what makes angular amazing :). Apart the very poor code quality, what is fundamentally wrong here?

[gkalpak]

I created this StackBlitz project and it does indeed work with v8 💥
Maybe the variables created via as were treated differently than other template-only variables in v8.

So, this sounds like a breaking change that needs to be documented. @JoostK, do you know what's the case for template variables created via as in v8 vs v9?

Apart the very poor code quality, what is fundamentally wrong here?

@mpalourdio: This seems like a big anti-pattern (and I hope it is not as common as you say 😅).

The main issue I see is that the [(...)] binding indicates a two-way-binding relation. Typically, the expectation is that the value is passed into the child-component (the [...] part of the binding) and the child component reacts to this.

So, my expectation when I first looked at the component (and the reason I immediately thought "this can't be right" 😁) was that applicationList goes into the child-component (due to the [...] part of the binding), then gets filtered inside the child-component and emitted back, the filtered list gets assigned to the template applicationList variables (via the (...) part of the binding), then the filtered applicationList gets back into the child-component (via the [...] part of the binding), where it gets filtered again and emitted back, etc.

The only reason the component works in this case is that it uses the [...] binding to initialize its internal state and does not react to changes in the input value. This is, of course, an implementation detail of the child-component and the consumer (parent component) shouldn't have to know about it. Hence, from the consumer's perspective, using [(...)] looks very unintuitive.

In addition, you open yourself up to all sorts of subtle bugs, such as:

• If applicationList$ emits another array, the search-filter component will still be using the original list, giving incorrect results.

Of course, you can work around them, but the point is that the parent-component behavior may change due to the implementation of a child-component and detailed knowledge of the child-component's implementation is needed in order to make the parent-component/template work as intended.

[mpalourdio]

That's really clear for me! Thanks a lot for these great explanation, and for taking time to investigate. So far, again, that's really a quick and dirty project and I wanted to give feedback on the migration path.

Feel free to close this issue, as it's really not an issue ('cause I am the issue :p).

Thanks again!

[gkalpak]

The issue is that this breaking change is not (clearly) documented, so let's keep this open to track that 😃

Action items:

• Figure out what the breaking change is exactly (incl. how template variables created via as are different from other template variables).
• Document that.

[DomEscobar]

Im curious if this is an anti-pattern, how do you do this instead?

thanks you

here is a stackblitz (look console output)
https://stackblitz.com/edit/angular-ivy-taatzh

[Splaktar]

Im curious if this is an anti-pattern, how do you do this instead?

here is a stackblitz (look console output)
https://stackblitz.com/edit/angular-ivy-taatzh

1. You can make your card a separate, dumb component that has an input that takes deviceGroup.
2. You can resolve the Observable in a component or service and provide that reference to the template instead.
3. With Reactive Forms, you can resolve the Observable in a component or service and use that to set the value of the FormControl

@DomEscobar Basically, you just need to stash that value somewhere other than a template variable.

[jelbourn]

Closing since it seems like this was a change for v9 and we're already pretty far out from that and it seems like this haven't been an issue that has affected a large number of users.

[angular-automatic-lock-bot]

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.}