bug: rxLet directive no longer renders content when Observable emits undefined

[lincolnthree]

Description

Hey RX'ers. Potential bug with rxLet directive. It seems that something has changed in a recent update and undefined values / emissions now cause the directive to fail to render the children within the element to which it is attached.

Previously, the rxLet directive would render regardless of the value of the emission.
So unless I'm missing something, this seems like a breaking change. An emission of null renders child elements just fine, which is what I would expect from either that or undefined values.

Steps to Reproduce the Issue

export class MyComponent {
   value$ = of(undefined);
}

<div *rxLet="value$ as value">
ABCD
</div>

Essentially. If value$ emits undefined, the content will not be rendered. If value$ emits null, the content will be rendered as expected.

Both null and undefined should cause content to be rendered.

Environment

    "rxjs": "^7.5.7",
    "@rx-angular/cdk": "^1.0.0-rc.2",
    "@rx-angular/state": "^1.7.0",
    "@rx-angular/template": "^1.0.0-rc.3",
    "@angular/animations": "14.2.6",
    "@angular/cdk": "14.2.5",
    "@angular/common": "14.2.6",
    "@angular/core": "14.2.6",
    "@angular/fire": "^7.4.1",
    "@angular/forms": "14.2.6",
    "@angular/platform-browser": "14.2.6",
    "@angular/platform-browser-dynamic": "14.2.6",
    "@angular/pwa": "14.2.6",
    "@angular/router": "14.2.6",
    "@angular/service-worker": "14.2.6",

Related to Other Issues

Tasks to Resolve This

Notes

[lincolnthree]

Update on this. Apparently this only happens in some instances and not others. I am attempting to diagnose why and what could be different. Any advise would be greatly appreciated.

I have verified that the undefined values are indeed being emitted at the time this occurs.

[lincolnthree]

Apparently this occurs when the first emitted value is 'undefined' (followed by no other values.)

If any other value precedes the undefined value, the template is rendered correctly. Still digging.

[lincolnthree]

Ok. I've reached the limit of my understanding / ability to debug this. I suspect this has something to do with initial change detection (inside the rxLet directive's logic to decide when the first value has been emitted) not understanding that undefined is actually a valid emission/event/value.

[hoebbelsB]

hi @lincolnthree sorry for the late response and thanks for the issue. Yes, this is indeed a breaking change and was introduced with the refinement of the reactive context and template trigger api.

An initial undefined value will be seen as suspense state. If a suspense template was provided, it will be rendered. Otherwise rendering will be suppressed.

Switching from a value to an undefined value will also result in the suspense state. In case there is no suspense template, it will only set the context variable of the RxViewContext

[lincolnthree]

Hey @hoebbelsB Thanks for the explanation. Suspense templates were indeed rendered as expected. I assume that means this issue won't be fixed/changed back to original behavior?

We've relied on this handling 'undefined' as 'falsy' in... more places than I care to think about... hundreds of places...and it will be a massive effort to go in and change all of this. Any recommendations for how best to move forward other than update our code in every single instance where an undefined value might be emitted?

On the topic, if this is a new feature, I'm not sure I really agree with this change. The entire point of *rxLet (as I understand it) is to provide an alternative to *ngIf that renders content even if emitted values are falsy. Treating undefined as a special value here breaks that contract, and creates a more complex/unintuitive/nuanced directive for end users/developers.

Why does an undefined value need be treated as different than null in this respect?

If the goal here is to treat falsy values as false / trigger the suspense condition, perhaps a new component should be introduced for this new functionality. It could be called *rxIf.

Thoughts? Thanks again.

[lincolnthree]

Additional information that I think may be relevant to the discussion, and why I was having such a hard time consistently reproducing this...

It seems undefined values are falsy while the source Observable is still connected (Template not rendered). But undefined is "valid/truthy" and causes the template to be rendered if the source Observable completes. Which.... sort of makes sense if you think about the observable no longer being in suspense, state, but still leads to a hugely complex coding scenario and a lot of boilerplate.

For example:

<ng-container *rxLet="value$ as value">
  value: {{value}}
</ng-container>

value$: Observable<any> = from([undefined, undefined]).pipe(concatMap(v =>
      of(v).pipe(delay(1500))
 ))

The template will render after 3 seconds (when the final undefined value is emitted.)

Switching from a value to an undefined value will also result in the suspense state. In case there is no suspense template, it will only set the context variable of the RxViewContext

This does appear to be the case -- If you "complicate" the scenario as seen below, the template will render when the first non-undefined value is emitted, and stay rendered through the emission of final undefined value:

value$: Observable<any> = from([undefined, 'abcd', undefined, 'abcd', undefined]).pipe(concatMap(v =>
      of(v).pipe(delay(1500))
    ))

But why should this be the case? Why should the first emission being undefined matter here?

Further adding a suspense template to the example does indeed cause suspense to be rendered when undefined values are emitted:

<ng-container *rxLet="value$ as value; suspense: suspense">
  value: {{value === undefined ? 'nothing to show' : value}}
</ng-container>

<ng-template #suspense>
  SUSPENSE
</ng-template>

I think this behavior needs to be unified. This is quite confusing/unintuitive for developers. Particularly since null and undefined are handled differently.

In the case where no suspense template has been provided, I think this should work the way it did previously, where undefined values will allow/trigger template rendering whether or not the source observable has completed -- thus retaining feature parity with the documented use-case for rxLet.

If the suspense template is provided, I can see an argument for this rendering suspense if the value is undefined, but I still think it's better served as a separate component because as it is now, now you need both a suspense template, and ALSO logic inside your template to handle the scenario where thet observable completes after emitting a final undefined value.

tl;dr> If I'm not using suspense, all values (truthy or not, even if the first value is undefined) should pass through unhindered to the rendered template.

Thoughts again?

[hoebbelsB]

hey @lincolnthree

Thanks for your valuable input. Let me try to catch up.

Suspense templates were indeed rendered as expected. I assume that means this issue won't be fixed/changed back to original behavior?

We've decided this behavior internally. I guess we will keep it like this for now.

Any recommendations for how best to move forward other than update our code in every single instance where an undefined value might be emitted?

If you use RxState you will anyway never emit undefined values. Undefined values are completely omitted by it, thus going perfectly hand in hand with the template package. But ofc this doesn't help you if your problem is to refactor the whole app. I'm extremely sorry ... 🙈

On the topic, if this is a new feature, I'm not sure I really agree with this change. The entire point of *rxLet (as I understand it) is to provide an alternative to *ngIf that renders content even if emitted values are falsy. Treating undefined as a special value here breaks that contract, and creates a more complex/unintuitive/nuanced directive for end users/developers.

rxLets purpose isn't and never was to replace ngIf. It is here to provide a convenient way of binding Observables to the template + enable local change-detection without the need of NgZone

Why does an undefined value need be treated as different than null in this respect?

because undefined is - as the name suggests - pretty undefined. In most cases it represents the non-existence of a value. We've defined non-existent as suspense. Null in comparison is a value that has to be proactively set by the user to indicate a falsy/nullish value. The discussion around null vs. undefined is quite complex and there is no right or wrong. I hope you get the idea why we've chosen this route.

If the goal here is to treat falsy values as false / trigger the suspense condition, perhaps a new component should be introduced for this new functionality. It could be called *rxIf.

There is already an rxIf directive and a PR stabilizing it's API, you can take a look.

It seems undefined values are falsy while the source Observable is still connected (Template not rendered). But undefined is "valid/truthy" and causes the template to be rendered if the source Observable completes. Which.... sort of makes sense if you think about the observable no longer being in suspense, state, but still leads to a hugely complex coding scenario and a lot of boilerplate.

Thanks for the intense testing, this def. sounds like something we need to take a look at.

But why should this be the case? Why should the first emission being undefined matter here?

Because we cannot distinguish between not-existing and undefined set on purpose. That's why we distinguish between null and undefined in the first place. One of the main ideas behind rxLet is lazy-rendering. It cannot lazy-render if it accounts undefined as a value. The template would always be initialized immediately.

In the case where no suspense template has been provided, I think this should work the way it did previously, where undefined values will allow/trigger template rendering whether or not the source observable has completed -- thus retaining feature parity with the documented use-case for rxLet.

An observable that completed once cannot emit any other value.

[lincolnthree]

Hey @hoebbelsB, thanks for reviewing. I appreciate the detailed response / breakdown.

Some responses, not necessarily in any order:

First, do you agree that *rxLet bound to an Observable that has completed with a value of undefined (and causes the template to render with this undefined value) is inconsistent and problematic within what I think is the new design/scope of *rxLet? If so, how should this be addressed? If not, why and how do you recommend we as users of your library address it in our code?

It feels like we need a fifth conditional template in addition to suspense: template and kin... something like, missing: template, except we already have that in *ngIf="..." and *rxIf="...", ---> "...; else templateName". Here,missing means "render the missing:template if no valid value was received and the observable has completed", this could even be expanded to include other falsy values, but now we’re really getting away from the point.

Which is…. as I understand it,*rxLet is supposed to be different from those branch conditionals and not require a missing or else. But it's now re-introducing the problem it was designed to address in the first place by treating undefined as a special falsy value -- except now we don't even have an else clause to handle it and we're still forced to branch in our template, once the observable completes.

Essentially, we now have a scenario with *rxLet that cannot be handled by any bound template, since suspense no longer knows if we are actually waiting or not when undefined values are received. And if we don't bind suspense nothing shows up at all.

In my opinion, if suspense is not bound/provided, the template should be rendered no matter what. That's what *rxLet was supposed to do: https://www.rx-angular.io/docs/template/api/let-directive#problems-with-async-and-ngif

Once suspense is provided, all bets are off and the component can treat undefined as specially as it wants. But even here, when the observable completes with undefined we still have a problem.

[lincolnthree]

If you use RxState you will anyway never emit undefined values.

I don't necessarily agree. This is 100% possible. Whether or not it should be possible is a different question.

First, is rx-angular/state tangential to the problem here? It is not (as I understand it) a requirement for using rx-angular/template. Correct me if I am wrong?

Second, while an undefined root object may never be omitted, (e.g. undefined from the root store or select()ed property itself), it can certainly still be emitted downstream if users are using their own pipes rather than special RxState selectors, and emitted objects themselves may contain undefined attributes, members, and values.

If that's something that rx-angular has decided should never be done, then I guess this doesn't matter as this paradigm will be required to use rx-angular/template. It makes sense for you to be consistent, I agree. But it's still something that can and will happen.

The issue here is that right now there's essentially no way to handle this consistently with how *rxLet is currently working -- unless I missed something.

Because undefined is - as the name suggests - pretty undefined. In most cases it represents the non-existence of a value.

Except that's not always true, and "most cases" isn't "all cases". Undefined can actually be set as a value! Undefined can also result from requesting data from a member/attribute/element that does not exist. It is a value! Undefined has all the problems of null and more.

[lincolnthree]

The discussion around null vs. undefined is quite complex and there is no right or wrong. I hope you get the idea why we've chosen this route.

I can understand why this route was chosen, and I agree with the fact that this is complex and undefined existing at all is complex and nuanced. However, right now the decision to treat undefined as a special case is effectively forcing an opinion of "right or wrong" downstream, and will essentially force developers who want to use *rxLet to completely adopt the rx-angular/state recommended usage of undefined everywhere in their code. Which will force people to handle this everywhere. Again, if that's your intent, then I guess that's fine, and it's just unfortunate for me :)

That said, the complexity here is indeed quite high. We have values, null values, undefined values, Observables that have emitted values, Observables that have not emitted values, Observables that have emitted undefined, Observables that have completed, Observables that are in error state.... I could go on. The issue here is that this is adding yet another possible decision tree.

So now we also have to deal with the new, special case where... "undefined means not-existing which also means no value emitted which triggers suspense but wait we actually did emit a value and it was undefined so instead of suspense we really need to display no current value template instead of a loading spinner ..." -- I hope you can see my point as well?

This new, opinionated definition of what undefined means is more complex, and perhaps necessarily, reductive.

This is a core issue of all languages that allow null and undefined values, and the only worse thing we can do besides using null or undefined at all is adding additional specification beyond what the language's built-in conditional logic dictates. Either we have a value or we don't, undefined or null or not - it is a value, but you have decided to treat is specially as not a value, which makes it a very special value -- that is different than how JavaScript treats it, and that is, I think, problematic.

But if that's the direction rx-angular wants to go, then my views on this are irrelevant, but I thought I'd post them just in case. Because currently this decision forces everyone use of this paradigm for undefined in ways that affect upstream code and design decisions significantly.

[lincolnthree]

In conslusion:

Thanks again for listening. I hope you will consider letting undefined pass through as a normal value if suspense is not provided. Regardless of that outcome, I look forward to hearing how we can deal with the inconsistency of completed vs. non-completed Observables emitting undefined.

And if this functionality is here to stay, then the docs definitely need to be updated to explain this treatment of undefined, and what happens in all of these scenarios -- currently the two are in disagreement :)

[hoebbelsB]

Hey @lincolnthree, sorry for the late response. Currently not at home. But I've discussed this with @BioPhoton. We will work on a small refactor and try to solve your issue. Coming back next week with more details on this

[lincolnthree]

Hey @hoebbelsB no worries! Thanks for considering and I appreciate the thought. Looking forward to what you guys come up with. Happy to help as I can.

[hoebbelsB]

hey @lincolnthree I sat down and tried to find a proper solution to this issue. But I first want to start by addressing your concerns here.

First, do you agree that *rxLet bound to an Observable that has completed with a value of undefined (and causes the template to render with this undefined value) is inconsistent and problematic within what I think is the new design/scope of *rxLet? If so, how should this be addressed? If not, why and how do you recommend we as users of your library address it in our code?

Yes, I agree, it is inconsistent. I hope that you'll find an answer in the end of my comment 😄

Which is…. as I understand it,*rxLet is supposed to be different from those branch conditionals and not require a missing or else. But it's now re-introducing the problem it was designed to address in the first place by treating undefined as a special falsy value -- except now we don't even have an else clause to handle it and we're still forced to branch in our template, once the observable completes.

Essentially, we now have a scenario with *rxLet that cannot be handled by any bound template, since suspense no longer knows if we are actually waiting or not when undefined values are received. And if we don't bind suspense nothing shows up at all.

In my opinion, if suspense is not bound/provided, the template should be rendered no matter what. That's what *rxLet was supposed to do: https://www.rx-angular.io/docs/template/api/let-directive#problems-with-async-and-ngif

Once suspense is provided, all bets are off and the component can treat undefined as specially as it wants. But even here, when the observable completes with undefined we still have a problem.

I fully agree that the current behavior is quite odd and inconsistent. But imo only the first undefined/suspense handling. I'm not 100% sure what you are saying about the complete state here. You are neither forced to wait for something to complete, nor to have a special branch for it.

I think the most interesting part is about your comments about the undefined handling:

That said, the complexity here is indeed quite high. We have values, null values, undefined values, Observables that have emitted values, Observables that have not emitted values, Observables that have emitted undefined, Observables that have completed, Observables that are in error state.... I could go on. The issue here is that this is adding yet another possible decision tree.

Big thx for listing the different kinds of scenarios, I appreciate that you fully understand the problem 🙏 !

These are exactly the cases we need to take a look at and where we have room for improvements. We just need to find a way to make intuitive and fun to develop with.

The current implementation works like this:

• undefined
• Observable emitting undefined
• Observable emitting nothing (EMPTY, NEVER, ...)

all are treated specially at first run and will lead to no rendered template if no suspense template was provided. I guess here is our main issue right now.

We have different choices to go from there. Still consider undefined (or non-existence of a value) as suspense or only treat non-existing values as suspense. Either way, we can solve your issue with both solutions, we just have to adjust the first run handling.

Option 1: consider undefined + non-existing values as suspense

• undefined
• Observable emitting nothing (EMPTY, NEVER, ...)

result in no rendered template if no suspense was provided.

However, Observable emitting an undefined value still sets the template context to suspense, but results
in a rendered template.

Option 2: consider only non-existing values as suspense

• undefined
• Observable emitting nothing (EMPTY, NEVER, ...)

result in no rendered template if no suspense was provided.

However, an Observable emitting an undefined value sets the template context to next and renders the template without setting the suspense context to true.

I personally think Option 1 is the way to go as it still allows to switch to the suspense state based on the value.

Let me know what you think. I'm already building a PR + unit tests for all the scenarios :)

[hoebbelsB]

Finally, let me answer to another question which is kind of related to this.

So now we also have to deal with the new, special case where... "undefined means not-existing which also means no value emitted which triggers suspense but wait we actually did emit a value and it was undefined so instead of suspense we really need to display no current value template instead of a loading spinner ..." -- I hope you can see my point as well?

You don't have to care about the suspense state if you don't want to. If you don't assign a special suspense template, the let directive will only set a special context variable. You will still be able to branch inside of your template based on the undefined value.

[lincolnthree]

Hey @hoebbelsB thanks for this! I'll try to make this reply less wordy. I'm not always great at laying things out clearly and concisely.

You don't have to care about the suspense state if you don't want to. If you don't assign a special suspense template, the let directive will only set a special context variable. You will still be able to branch inside of your template based on the undefined value.

Can you tell me what this means, in practice?

My understanding is that -- currently -- if I don't assign a suspense template, and the Observable emits (but does not complete with) undefined, nothing in the contained template is rendered, and there is no value to branch on.

[lincolnthree]

A thought. Is there a way we could get on Discord and talk about this for a few minutes? Either I'm not understanding something, or I'm not sure my core is making sense and I want to try to clarify what I'm seeing.

[hoebbelsB]

Hey @hoebbelsB thanks for this! I'll try to make this reply less wordy. I'm not always great at laying things out clearly and concisely.

You don't have to care about the suspense state if you don't want to. If you don't assign a special suspense template, the let directive will only set a special context variable. You will still be able to branch inside of your template based on the undefined value.

Can you tell me what this means, in practice?

My understanding is that -- currently -- if I don't assign a suspense template, and the Observable emits (but does not complete with) undefined, nothing in the contained template is rendered, and there is no value to branch on.

your understanding is correct. I was already talking about the situation when we've fixed the issue ^^

A thought. Is there a way we could get on Discord and talk about this for a few minutes? Either I'm not understanding something, or I'm not sure my core is making sense and I want to try to clarify what I'm seeing.

yes, let's have a chat tomorrow if you like

[hoebbelsB]

@lincolnthree please have a look at the linked PR. Especially take a look at the following spec files, which should explain the behavior :)

• https://github.com/rx-angular/rx-angular/pull/1450/files#diff-33a9e4b917e089ab44c5b316a5ac9a524e65e5edc39fac47f0b0c3731f5d8174
• https://github.com/rx-angular/rx-angular/blob/main/libs/template/let/src/lib/tests/let.directive.template-binding.all.spec.ts

[lincolnthree]

Thanks @hoebbelsB - If I'm grokking things right, this looks awesome :) I am lincolnthree num 1078 on discord if it would be convenient to chat briefly about all this.