angular: Use of `null` in `async` pipe has become problematic with NG12, TS4 and strict type checks
Which @angular/* package(s) are relevant/releated to the feature request?
Description
I’ve recently been using Angular 12 for the majority of my Angular projects. I was previously used to non-strict mode TypeScript, which is more convenient, but it definitely allows more little issues to slip through. I appreciate strict mode, and generally prefer it, however there are some things about Angular that make strict mode extremely tedious in certain common use cases.
Notably, the async pipe can emit one of three values: undefined, null, or the type of the stream you are piping. The pipe seems to emit null, I think, as an initial value? The problem here is that it basically forces all presentational components to use inputs defined like this:
export class BasicPresenter {
@Input() someObject?: SomeObject | null;
@Input() someArray: SomeObject[] | null = [];
}
I rarely ever use nulls in my own code. My code usually relies on things either being undefined (not present), or defined and present. The rare case where I may use nulls is with some form fields where I explicitly want data written to a datastore on a server somewhere to actually be null, but that usually applied to properties of my entities…the entities themselves are either undefined or defined, never null.
The fact that async returns null forces me to add | null all throughout my codebase, potentially hundreds or thousands of components, which would not otherwise have to deal with null at all. It affects every stream of every kind, so even if you have streams that are basic primitives, especially streams that never have nulls in them, you still have to deal with string | null, or number | null, etc.
It should also be noted that if null is legitimately NOT a value you WANT to be allowed from a stream or to an input, the fact that the async pipe introduces it forces types to be expanded, thus potentially allowing inappropriate usage of your custom components. Further, null is not a value that will cause, say, default parameters to trigger…if you pass an input on a child component through a pipe that has parameters with default values, if the async pipe starts out with null values, and those values are passed to a pipe in the template, the defaults would not be used, null would be used instead. This in a sense makes null an infectious type that has to be dealt with in more and more code. One of the benefits of using strict mode typescript…is to make the need and use of null EXPLICIT. The async pipes expansion of the types of the Observable streams being piped eliminates a lot of this benefit for child components…and often other code they may have to consume.
For an enterprise scale project with potentially thousands of components, each of which may on average have 3, 5, 10 inputs depending on the nature of the project, that could be many thousands more instances where I have to deal with | null in my code. Its extremely tedious.
I’m honestly not really sure why null comes into play with the AsyncPipe. The API documentation doesn’t mention null at all. Looking at the source code, it appears that null is the initial value for some things, when I think undefined would in fact be more appropriate.
Proposed solution
I propose that the async pipe should only deal with the explicit type defined for the Observable that is being piped. If an Observable is strictly of type SomeObject, then the type for values emitted by async should be SomeObject. By using null internally, then an expansion of the type occurs, when it could potentially be very much undesired. Expansion of the type to include null and undefined requires that downstream code deal with those values, which increases the complexity of that code.
Since the non-nullish assertion operator has been largely shunned by the community, asserting that something cannot be null is usually caught by linters and compilation is impossible without either disabling rules, or commenting them out for each use case (again, tedious and really shouldn’t be necessary.) Non-nullish assertion could be used, but then if it IS appropriate that an input could potentially be undefined, non-nullish assertion cannot actually be used, and you would again be stuck adding | null to your code when its inappropriate.
Maintaining the type narrowness of the original observable seems reasonable. Observables of a particular type may never emit anything. If an observable never emits a value, then no attempt should be made to set the given property of the child component. This would avoid the need to say, initially set all inputs to null, or even undefined. There was never a value emitted by the stream, thus there would never be a value input into the child component, and no event (i.e. no ngOnChanges call) for that input.
Alternatives considered
Alternatively, if for some reason an initial value MUST be input into every child component, perhaps the async pipe could maintain the narrowest possible type of SomeObject | undefined. This would eliminate the need to add | null to every input, and instead rely solely on just adding the optional property marker ?, or defaulting the initial value of the property:
@Input() someObject?: SomeObject;
@Input() someArray: SomeObject[] = [];
Which comes out a lot cleaner and less tedious than including null as a valid type value.
About this issue
- Original URL
- State: open
- Created 3 years ago
- Reactions: 81
- Comments: 31 (7 by maintainers)
Commits related to this issue
- fix(web-client,PayFromToComponent): work around Angular async pipe null issue See: <https://github.com/angular/angular/issues/43727> — committed to ntls-io/nautilus-wallet by PiDelport 2 years ago
- fix(web-client,PayFromToComponent): work around Angular async pipe null issue See: <https://github.com/angular/angular/issues/43727> — committed to ntls-io/nautilus-wallet by PiDelport 2 years ago
- fix(web-client,PayFromToComponent): work around Angular async pipe null issue See: <https://github.com/angular/angular/issues/43727> — committed to ntls-io/nautilus-wallet by PiDelport 2 years ago
- refactor(web-client): renovate session state and services (#163) This renovates the session state management and services a fair bit, in preparation for merging the ASA and XRPL work: 1. Separate ... — committed to ntls-io/nautilus-wallet by PiDelport 2 years ago
A short update on this issue, that discusses both the real-life pain-point / problem as well as possible solutions.
Let’s start with the problem first, which is the fact that the type of the async pipe transform is always widened to include
null. Thenullinclusion comes from the fact that an observable might not have any value when subscribing. And “not having a value synchronously” is certainly true for promises (which are supported by theasyncpipe in addition to observables.Given the stated problem, we can approach it from 3 different angles:
nullandundefinedfor “no value before subscription”. Historically the implementation choice was to includenullbut it causes real-life issues with@Inputbindings - as described in this issue we can’t declare inputs as@Input foo?: Tand are forced to do@Input foo?: T|null. This is is sub-optimal and choosingundefinedwould address some of the pain. Sadly, this would be a very breaking change as noted in https://github.com/angular/angular/issues/16982#issuecomment-769471368BehaviorSubject) and indicate it via an additional argument to theasyncpipe (ex.sync: booleandiscussed in https://github.com/angular/angular/pull/47608#issuecomment-1266120178). This stance has 2 issues:asyncpipe - this is what we’ve attempted in #47608 but it has 2 problems as well:<test-cmp [foo]="(obs$ | async) ?? initialExp"></test-cmp>We’ve discussed all those options in the team and don’t feel like any of the approaches is a “winner” here - we either end up with a sub-optimal solution and / or require costly migration.
At this point we do recognize the problem but don’t have a solution that we would be happy with. Introducing a separate pipe for “always have a value” observables (and dropping promises support) might be an option but this would fragment the Angular ecosystem by providing 2 ways of doing the same thing.
It seems like there are a lot of options here that are reasonable:
undefined:[input]="(myObservable$ | async) ?? undefined"[input]="(myObservable$ | async) ?? ''"ngIf:<div *ngIf="(myObservable$ | async) as myObservable"> <cmp [input]="myObservable"></cmp> </div>AsyncPipe- you’re welcome to introduce any custom logic of your ownAll of the above options would prevent the need for
| nullin component inputs.The async pipe must include
null(orundefined) in its type because the stream may not have emitted a value by the time the template executes.I have 160 * 4 = 640 component file with about 2 or 3 property per component. it will be: 640 * 3 = 1920 modification… or n place have async pipe. I dont know which path will help me finish this task faster. I think people can save me by allow this to fix in the next release. today is 2022/01/02 I have 20 days to do this. While waiting this feature I will take some coffee to help me 😭
On Angular 13, this is a pain. I hate to use hacks to prevent this unneeded behavior, for example an array should never be null.
I believe the only must would be
undefined…there is really no reason that null must be included. With undefined, at least then you could eliminate all of the excess| nulltyping all over the place. It becomes quite infectious and can become required on angular pipes, dealt with for forms, etc. etc.Regarding all of your options, every one of those requires “unwrapping” the stream in the parent component, and I much prefer not to do that. It isn’t all that much better than adding
| nulleverywhere…it is excess work that should not be necessary. All that should be necessary is[input]="myObservable | async". Having to wrap things in parens, coalesce or otherwise figure out a way to pass in a default, directly unwrap withas, etc. are all more complex, increase the time to write and test, etc.The idea here is…let’s simplify things, not make them more complex. I think, if it really is a MUST that
AsyncPipereturn some nullish value, thenundefinedis far preferable a it doesn’t expand the type as much, and it can be represented simply with an additional?rather than an additional| null.I’m still not entirely convinced that “has not emitted” actually requires undefined or null. Non-emission means “nothing has happened”, and that input would simply not be set at that time. Only on the first emission would the input necessarily need to be set, in which case…including null or undefined in the type wouldn’t be a hard requirement.
When can the
asyncpipe emitundefined? I think the only time that would happen is if the stream it is subscribing to emitsundefined, right?I don’t think we could change the pipe to always return
Tunless we provided a way to specify an initial value as a parameter of the pipe:Perhaps that would be enough?
This kind of thing is really the worst answer to issues like this: “Its too hard” This is Angular! We can do it!
This one is pretty critical, and introduces some significant problems, which have hit pretty much all of my code bases pretty heavily. I think there are two phases to solving the problem…switching to undefined first, then adding the ability to choose your own default…OPTIONALLY. Further, I think choosing defaults should be something we can do configurationally to switch the global default (i.e. default to null instead of undefined, which could probably be done alongside a switch to using undefined by default out of the box…that way code bases that rely too heavily on null would have only a minor disruption to deal with: Adding the necessary config, probably in a module, maybe via an injection token, which would allow us to override the default throughout the injector hierarchy, even at the component level.) We should also be able to explicitly specify a default for each async use case.
It’s been discussed before: https://github.com/angular/angular/issues/16982#issuecomment-769471368 The resolution was: too hard to migrate (existing internal usages in google) to justify the change.
I’m confused… Only one of these signatures returns type
Tand it requires the use of a subject. Which would require the construction of said subject every single time you wanted to use theasyncpipe, this perpetrating the problem…theasyncpipe is hard to use in strongly typed code.The problem is solved by using method overloads. It’s been solved very elegantly in the
@rx-angular/templatelibrary by Michael Hladkey, with hispushpipe. Thepushpipe has none of the problems that Angular’sasyncpipe has, because it was designed with multiple overloads that support many different return types, includingT | nullas well asT | undefined.The SYSTEMIC problem in the Angular codebase, is that the Angular development team seems to refuse to acknowledge the differences between
nullandundefined, and seem insistent on using only| nullas a possible option. This has become even more problematic in more recent versions of angular than when I brought up this topic years ago. The Angular team needs to properly supportundefinedas it is an idiomatic thing in JavaScript and TypeScript, as it is the only thing that will trigger many of JavaScript’s built in features…such as default parameters and the like.The problem is deep enough that the solution is not to hack up the
asyncpipe with an option for BehaviorSubject. The solution is to create a new pipe, whatever they want to call it, that doesn’t only supportT | nullas the return value in its method signatures, and instead supports a variety of options that are relevant to the input types passed in, such asTfor aBehaviorSubject(or even aReplaySubject), as well as BOTHT | nullANDT | undefined(but as INDEPENDENT method signatures…check out how the rx-angularpushpipe was implemented). This should, in fact, be a sweeping overhaul of Angular, as its support ofundefinedis effectively non-existent, and its infections insistence on adding| nullto everything is deeply and broadly systemic… 😢I think we just simple make a new pipe for people.
"data$ | async-fill"or some keyword to tell data is always ready. no need to cast null. So we reduce time to migrate. p/s: I know I can make a custom pipe but if I not have it includes to angular/common. I have to reimport that custom pipe every where. That not really good.so I see. We can make this more simple:
{{ obs | async : 'initial' }}as solution. initial mean you alway have data, no need to cast null. Then if you want do this global changes. We can play an global config like:=> so we dont have to declare ‘initial’ on every async pipe. I think that is the most possible solution. “A global config for async pipe”.
Agree with
T | undefinedas the return type, I can’t think, and not sure, if there’s any reason behind the decision of returningnull, but it’s definitely worth the consideration.As a side note, “defining an initial value” may be interesting to discuss in another issue.
If we did have that, and async did emit null, even if it was “confidently”, we would actually still have the problem I’m describing in he child components. Instead of say just
@Input() input?: Type;, we would be forced to do this@Input() input?: Type | null;Not because the observable is in fact ever going to emit anull…but becauseAsyncPipeis expanding the type.The goal is to eliminate that expansion of type, so that we don’t end up with nullable types that should not be nullable. (If in fact you DID want a nullable type, then the observable itself should be typod that way:
Observable<Type | null>.) This is a thing that occurs with strict type checking with TypeScript. If you disable strict type checks, then you may not actually experience the problem I’m describing. If you are using strict type checks (default now in Angular), then you actually end up losing the strict null checking because of howAsyncdPipeis designed today.The goal is purposely to eliminate
nullfrom theAsyncPipetypes, and to eliminate that unwanted expansion of the type for component inputs.For anyone like me, that trying to migrate angular into a newer version(v10 to v15), in a huge code base, please stop searching anymore, I believe the solution would be (for now, as per the comments above):
undefinedrather thannull;??or||to provide a default value;I was like ‘oh this is the only framework that provides a detailed upgrade guide, it should be quite easy’ after actually reading the upgrading guide, but now here I am lol.
BehaviorSubjectis a special kind ofObservablethat has a default value.Why can’t we have this
Yeah, we have actually used the push pipe (in our case, we used @BioPhoton’s rx-angular lib and its push pipe and let directive) in a project or two.
I’m actually quite a fan of both…and, I guess, its worth adding to the discussion whether that may be a solution here. Instead of updating async to work a different way, perhaps adding a
pushpipe, and even adding something similar to the*rxLetdirective with its extended capabilities (i.e. handling all three channels of an RxJs observable, supportingsuspense, etc.) might be a worthy alternative approach. This would allowpushto returnundefinedby default without requiring any changes toasync.It would be very interesting to have an
*ngLetor something like that to handle observables in a richer, more comprehensive fashion as well…however there was more to rx-angular than simply a pipe and a directive. It also offered notable performance benefits due to the way it handled the render cycle of observables in templates. It may just be better to use rx-angular if you want anything beyondpush.Agree that the default initial value thing should be a separate feature. I think
T | undefinedis very important and rather foundational, whereas default initial probably needs more thought and discussion and should be decoupled from this feature.I’m not sure if what you wrote here was a typo or not, but I said “confidently omit”, not “confidently emit”.
The whole point of what I’m suggesting would prevent
AsyncPipefrom expanding the type (when appropriate).I think what we need is something like rx-js traits (developed by rxjs core team members). If we had a guarantee that an observable would be synchronous (which, in rxjs-traits would correspond to the “traits” type having
async: false, I believe),AsyncPipecould confidently omitnullfrom the return type.The Typescript signature would be something along the lines of this:
transform<TValue, TTraits>(obs: Observable<TValue, TTraits>): TTraits extends { async: false } ? TValue : (TValue | null)