Warn on negated async pipes

[dgp1130]

Which @angular/* package(s) are relevant/releated to the feature request?

@angular/compiler-cli

Description

Negating an async pipe in an *ngIf thrashes the layout because the pipe emits null immediately before checking the condition. This can lead cause unnecessary rendering work and trigger component events which cause XHR's or other work to be done that shouldn't be needed.

<div *ngIf="!(myConditional | async)">
  <!-- ... -->
</div>

http://codelyzer.com/rules/template-no-negated-async/

Proposed solution

We should consider an extended diagnostic to detect and warn the user of this anti-pattern to head-off their confusing and suggest a better alternative.

I'm not entirely sure what that alternative should be, but I imagine there's an easily transformable way of doing this?

Alternatives considered

N/A

[angular-robot]

This feature request is now candidate for our backlog! In the next phase, the community has 60 days to upvote. If the request receives more than 20 upvotes, we'll move it to our consideration list.

You can find more details about the feature request process in our documentation.

[amakhrov]

This sounds very valuable for "truely async" observables.
However, it's likely to be less helpful on projects using observable state management (eg NGRX), where the state always has some current, synchronous value.

[kirillgroshkov]

I don't understand how "negated async" vs "non-negated async" causes more "thrashing"?

In case of "negated async" for smth that is not-null it'll change from "shown" to "hidden".

For "non-negated async" it'll change from "hidden" to "shown".

Both ways is a change in DOM.

What makes the first case worse than the second?

E.g I'm thinking in terms of CLS (cumulative layout shift) - it's the same (just different "direction" of the shift).

[dgp1130]

@kirillgroshkov, my understanding (and I'm not an expert in Angular rendering performance) is that going from hidden -> shown isn't much different starting as shown to begin with. You still need to show the content and you aren't paying much of a cost by rendering nothing first (not sure if that waits a frame though).

By contract, going from shown -> hidden does unnecessary rendering work as opposed to just starting hidden. Users have to pay the cost of rendering a piece of DOM that serves no purpose and is immediately removed from the tree. Starting out in the hidden state would skip this work.

I think in terms of CLS they are equivalent (assuming hidden -> shown does wait a frame), but in terms of work done by the browser a negated async would be worse, because you're wasting rendering effort that didn't need to be done.

From a slightly more subjective standpoint, as a user I would be more annoyed by a piece of UI that appears and disappears after a single frame, than I would from a piece of UI that simply took an extra frame to load in. That depends on bit on how your layout is structured, but in general I would find shown -> hidden more disruptive as a user.

[kirillgroshkov]

@dgp1130 , ok, I think I start to agree with your reasoning, thanks for elaborating!

What is the recommended alternative to "negated async pipe" then?

[pascal-puetz]

Just out of interest: isn't this something that ESLint can already do using the "@angular-eslint/eslint-plugin" (Rule: @angular-eslint/template/no-negated-async )?

[dario-piotrowicz]

I'm wondering, does the async pipe need to emit null before checking the condition? 🤔

Could for example a startWith-like argument be added to the pipe instead? so that the developers can put the initial value there which will be used straight away before the condition checking and also the fist emission?

That could also be a handy shorter way for the developer to start with some value instead of having to add a pipe and startWith to the existing observable 🤔

[angular-robot]

Just a heads up that we kicked off a community voting process for your feature request. There are 20 days until the voting process ends.

Find more details about Angular's feature request process in our documentation.

[angular-robot]

Thank you for submitting your feature request! Looks like during the polling process it didn't collect a sufficient number of votes to move to the next stage.

We want to keep Angular rich and ergonomic and at the same time be mindful about its scope and learning journey. If you think your request could live outside Angular's scope, we'd encourage you to collaborate with the community on publishing it as an open source package.

You can find more details about the feature request process in our documentation.

[jessicajaniuk]

@mgechev Do you have an opinion on adding this extended diagnostic?

[mgechev]

We found it valuable, but I'd say we have higher-priority checks we can look into (performance and a11y conformance).