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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decide how to reconcile `async` pipe with strict template type checking #33704

Open
mmalerba opened this issue Nov 8, 2019 · 4 comments
Assignees
Milestone

Comments

@mmalerba
Copy link

@mmalerba mmalerba commented Nov 8, 2019

馃殌 feature request

Relevant Package

This feature request is for @angular/common

Description

Currently the async pipe is difficult to use with strictTemplates and strictNullChecks enabled. This is because the async pipe emits null if the underlying observable has not emitted yet. Therefore using the async pipe with an input that does not accept null will result in a compilation error.

@devversion has done a more detailed write-up of this issue. I'm including a summary of the possible solutions here, but please refer to the write-up for more details.

Describe the solution you'd like

The ideal solution would be to make async pipe not emit until the underlying observable emits, however this would likely be a fairly breaking change. It would probably need to be done in a phased manner, while introducing something like asyncEager that maintains the old behavior to make migration easier.

Describe alternatives you've considered

  1. An option that would split the responsibility for fixing between framework and libraries is to introduce something like @RequiredInput(fallbackValue) that automatically replaces null with fallbackValue. Libraries could then update their inputs to use @RequiredInput when null is not expected.

  2. Another option that would place the responsibility on libraries is to ask them to just handle null and allow it to be passed in the template even if its not part of the input's official type (using static ngAcceptInputType_*)

  3. Finally, a third alternative is to just pass the responsibility down to the app developer to fill in a default: [someInput]="(value$ | async) || defaultValue"

@lacolaco

This comment has been minimized.

Copy link
Contributor

@lacolaco lacolaco commented Nov 9, 2019

Thank you for clarifying the problem. As a user, even if not Ivy, this has been a long-standing issue and very tough.

I think it isn't the child component's responsibility to be prepared for the possibility of null.
I am afraid that the design will be implicitly coupled with the parent implementation.
The parent component should be responsible for the value flowing through its own data stream, and that the non-nullable stream passes the value without null.

The trouble is that even if it is non-nullable, the value is not always available. It can delay.
Personally, I think that lazy execution approaches like React.lazy and React.Suspense may be helpful. <ng-container *ngIf="value$ | async as value"> is close to that but it rejects all falsy value. sometimes it occurs a bug with 0 or ''.

@Airblader

This comment has been minimized.

Copy link
Contributor

@Airblader Airblader commented Nov 9, 2019

It would probably need to be done in a phased manner, while introducing something like asyncEager that maintains the old behavior to make migration easier.

Wouldn't it have to be the other way around, introduce the new behavior as asyncLazy, to not make a breaking change? Unless you intend to write a migration that changes all occurrences, but you'd also have to replace programmatic usages of the pipe.

Alternatively don't create a whole separate pipe but just make it accept a parameter for the initial value:

*ngIf="(foo$ | async: false) as foo" 

For simple type inputs a migration could be written that adds the respective default value of that type (boolean => false etc.) as the initial value. This is at least very likely to do the correct thing; if it wouldn't, the component would have a broken API because it should actually declare to expect null.

@mmalerba

This comment has been minimized.

Copy link
Author

@mmalerba mmalerba commented Nov 10, 2019

@Airblader yeah I was thinking create a schematic to rename it, since we'd want to keep the async symbol for the non-deprecated version. Adding a fallback parameter to async pipe also seems like a good alternative, but I still think the best solution is not setting the input at all until the observable actually emits something

@ngbot ngbot bot modified the milestone: needsTriage Nov 11, 2019
@IgorMinar

This comment has been minimized.

Copy link
Member

@IgorMinar IgorMinar commented Nov 11, 2019

we discussed this today, and we agreed that to fix this properly, we should look at typing of inputs in general and properly allow inputs to be optional, required, etc.

for v9 the best we can likely do is that for users that want to use strictTemplates and strictNullChecks together, they can opt out of strict null typecheck specifically for input bindings via strictNullInputTypes : https://v9.angular.io/guide/template-typecheck#troubleshooting-template-errors

This should be explicitly documented in the v9 updating guide or template type checking guide.

@IgorMinar IgorMinar assigned alxhub and unassigned IgorMinar Nov 11, 2019
@IgorMinar IgorMinar modified the milestones: needsTriage, v9-blockers Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can鈥檛 perform that action at this time.