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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ngFor should optionally show a template when there are no items in the collection #14479

Closed
adharris opened this issue Feb 14, 2017 · 27 comments
Closed
Labels
area: common Issues related to APIs in the @angular/common package feature Issue that requests a new feature P4 A relatively minor issue that is not relevant to core functions state: has PR
Milestone

Comments

@adharris
Copy link

I'm submitting a ... (check one with "x")

[ ] bug report => search github for a similar issue or PR before submitting
[x] feature request
[ ] support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question

Now that you can give ngIf a template to display as an else condition (#13297), it would be nice if ngFor got the same treatment for when there are no items in the collection.

Current behavior
Often, my collections come from an observable, so in order to display a "No results" message i do:

<div *ngFor='let item of results$ | async'></div>
<div *ngIf='(results$ | async)?.length === 0'>No results</div>

Or, in 4.0.0:

<div *ngIf='results$ | async; let items'>
  <div *ngFor='let item of items'></div>
  <div *ngIf='items.length === 0'>No Results</div>
</div>

Expected behavior

It would be nice to be able to do something like this:

<div *ngFor='let item of results$ | async; empty noResults'></div>
<template #noResults>No results</template>

Which would be similar to how ngIf handles else cases.

  • Angular version: 4.0.X
@tbosch tbosch added area: common Issues related to APIs in the @angular/common package feature Issue that requests a new feature labels Apr 10, 2017
@arlowhite
Copy link

arlowhite commented Apr 23, 2017

Maybe keep else as the keyword instead of creating a new empty keyword.

For example, Python re-uses the else keyword:

for item in items:
    # do something with item
else:
   # no items

EDIT: Please read comment and links from epsy below, which points out that this syntax has confused Python programmers and doesn't do what I assumed. I do think empty is more clear.

@maxime1992
Copy link

Any news about this feature ? :D

@jefbarn
Copy link

jefbarn commented Jul 20, 2017

There is a subtle difference here with async items between 'no results' and 'results still loading'. Any solution would need to target both (or optionally default to one template).

@epsy
Copy link

epsy commented Sep 8, 2017

Worth pointing out is that the "else" keyword in Python's "for-else" construct is regarded as bad naming [1][2]. Unlike what the comment above suggests, this else block will run whenever the for loop completes without being interrupted (by using break, return, or if an exception was thrown), independently of whether the loop had any iterations or not. It usually gets confused for the feature being requested here (i.e. runs when the iteratee is empty), but Python's mistake adds enough confusion to the 'else' name that it might be worth avoiding.

[1] https://mail.python.org/pipermail/python-ideas/2012-June/015350.html
[1] http://python-notes.curiousefficiency.org/en/latest/python_concepts/break_else.html

@ngbot ngbot bot added this to the Backlog milestone Jan 23, 2018
trotyl added a commit to trotyl/angular that referenced this issue Feb 17, 2018
trotyl added a commit to trotyl/angular that referenced this issue Feb 17, 2018
trotyl added a commit to trotyl/angular that referenced this issue Feb 17, 2018
trotyl added a commit to trotyl/angular that referenced this issue Feb 17, 2018
@xclusive36
Copy link

xclusive36 commented Apr 15, 2018

<ion-item *ngIf=items?.length === 0'>No Results

This works if you place it inside of the < /ion-list > but outside of *ngFor

Example:
< ion-list >
< ion-item-sliding *ngFor="let item of items" >
< ion-item *ngIf="item.display" >
{{ item.title }}
< /ion-item >
< /ion-item-sliding >
< ion-item *ngIf='items?.length === 0' >No Results< /ion-item >
< /ion-list >

This is obviously a stripped down version of what I have but it works as needed.

@thw0rted
Copy link

@xclusive36 the reason you got a downvote, I think, is that it's easy to make a "no items displayed" conditional for something as simple as a single empty list, but doesn't address the issue of a computed value for ngFor, like when the async pipe is used. Rather than having two subscriptions to the list observable, you would have one (maintained internally by the template service) and it would feed both the list and the "no results" item at the same time. It's more efficient than maintaining two subscriptions and makes for simpler code than maintaining your own shared subscription in your component.

@Airblader
Copy link
Contributor

Airblader commented Jun 26, 2018

I recently wrote an article on Medium which enhances ngFor with another feature, see the article here. The same strategy can be applied here to implement this behavior yourself.

I'll attach a quick attempt at this here, but note that it's not thoroughly tested and might not be ideal. It goes without saying that support in core would be preferable, of course. You can see it in action here.

Edit: This does actually not work quite correctly yet, I just noticed. I don't have time right now to look into it any further, perhaps someone would want to use it as a basis or inspiration, though.

@Directive({
  selector: "[ngForIfEmpty]",
})
export class NgForIfEmpty<T> {

  @Input()
  public set ngForIfEmpty(templateRef: TemplateRef<any>) {
    this.templateRef = templateRef;
    this.viewRef = null;
    this.updateView();
  }

  private templateRef: TemplateRef<any>;
  private viewRef: EmbeddedViewRef<any>;
  private _isEmpty: boolean = false;

  constructor(@Host() private ngForOf: NgForOf<T>,
              private viewContainer: ViewContainerRef) {
    const _ngDoCheck = ngForOf.ngDoCheck.bind(ngForOf);
    ngForOf.ngDoCheck = () => {
      _ngDoCheck();

      this.isEmpty = !ngForOf.ngForOf || this.isIterableEmpty(ngForOf.ngForOf);
    };
  }

  private set isEmpty(value: boolean) {
    if (value !== this._isEmpty) {
      this._isEmpty = value;
      this.updateView();
    }
  }
  private get isEmpty() {
    return this._isEmpty;
  }

  private updateView() {
    if (this.isEmpty) {
      if (!this.viewRef) {
        this.viewContainer.clear();
        if (this.templateRef) {
          this.viewRef = this.viewContainer.createEmbeddedView(this.templateRef);
        }
      }
    } else {
      //this.viewContainer.clear();
      this.viewRef = null;
    }
  }

  private isIterableEmpty(iterable: NgIterable<T>): boolean {
    for (let item of iterable) {
      return false;
    }

    return true;
  }

}

@umutc
Copy link

umutc commented Nov 14, 2018

@Airblader

    private viewContainer: ViewContainerRef) {
    const _ngDoCheck = ngForOf.ngDoCheck.bind(ngForOf);
    ngForOf.ngDoCheck = () => {
      _ngDoCheck();
      /* HERE IS THE PROBLEM */
      /* ngForOf.ngForOf is always return undefined */
      this.isEmpty = ngForOf['_ngForOf'].length === 0;
    };
  }```

@karocksjoelee
Copy link

anybody else have a more elegant want to solve this ? I found using *ngIf else to check the length and display the 'no result' status cannot be re-used. I was think using custom directive to solve it ?

@Mintenker
Copy link

I believe this issue is still relevant. Any updates? While not exactly critical, it would be nice to have this.

@ghost
Copy link

ghost commented Jul 11, 2019

This would be particularly useful in cases where an Observable is the source, and an additional subscription to it is required within the component to determine whether or not it's empty. In my current project, the source is either an Observable or null, and it comes from an external library beyond my control.

I haven't investigated thoroughly yet, but this seems like it would take a lot less overhead (and less chance of error) if it were implemented as part of Angular. I have a situation where I absolutely must distinguish between loading and empty, and the code to determine this seems far more involved than I would have thought.

@thw0rted
Copy link

I've started using a pattern where I "fix" an Observable that doesn't play well with the async pipe:

class MyComponent {
  /** @internal */ public readonly fixed$: Observable<Array<Thing>>;

  constructor (service: MyService) {
    this.fixed$ = service.maybeThings$.pipe(map(arr => arr || []));
  }
}

Now, if your library emits null (or undefined) instead of an array, you always get something. So now you can write

<ng-container *ngIf="fixed$ | async; let things">
  <div *ngIf="things.length > 0; else noThings">...</div>
  <ng-template #noThings>...</ng-template>
</ng-container>

Re-reading your post, you say the source "is either an Observable or null" -- do you mean you have maybeThings$: Observable<Thing[]> | null? That's terrible, and there's not going to be a good solution no matter what. You have to poll to see if it changes to an Observable, then subscribe to it. It's a bad pattern and the library interface should be changed (and its author publicly mocked). If you're stuck with it, write a wrapper that handles the polling internally and "translates" to a single (actual) Observable.

@alexgurrola
Copy link

Bump.

@petebacondarwin petebacondarwin added the P4 A relatively minor issue that is not relevant to core functions label Nov 9, 2020
mhevery pushed a commit to trotyl/angular that referenced this issue Dec 4, 2020
Add support for `default` template to be displayed when `*ngFor` has no
content.

closes angular#14479
mhevery pushed a commit to trotyl/angular that referenced this issue Dec 4, 2020
Add support for `default` template to be displayed when `*ngFor` has no
content.

closes angular#14479
mhevery pushed a commit to trotyl/angular that referenced this issue Dec 4, 2020
Add support for `default` template to be displayed when `*ngFor` has no
content.

closes angular#14479
mhevery pushed a commit to trotyl/angular that referenced this issue Dec 4, 2020
Add support for `default` template to be displayed when `*ngFor` has no
content.

closes angular#14479
@jelbourn
Copy link
Member

jelbourn commented Dec 8, 2020

Hey all, despite some early approvals from team members, after further discussion we've decided not to add this feature. The main rationale here is that this can already be accomplished by using ngIf after the ngFor without any additional APIs. The proposed API would add ~500 bytes to the framework for all users. While this is small, many small features like this add up to kilobytes over time. Expanding the API surface also increases the amount of content for the docs, meaning new users have more in front of them when trying to learn concepts.

I know that there are a number of people who would find this feature useful, but ultimately we think it's to the benefit of the larger audience not add this one.

@jelbourn jelbourn closed this as completed Dec 8, 2020
@Airblader
Copy link
Contributor

Fair enough, but FWIW, if we're going to count bytes, we should also count the additional bytes per conditional required in an average project to be accurate on whether adding this is a net increase of bundle size or not. Every project I worked on has at least a few dozen instances of this.

@kbrilla
Copy link

kbrilla commented Dec 8, 2020 via email

@thw0rted
Copy link

thw0rted commented Dec 9, 2020

Jeremy, how did the team address the concerns I raised... good Lord, two and a half years ago? Specifically, it's very common to have an expression in the ngFor directive that isn't a simple property access -- a pipe, a function call, whatever. In that case the existing API does generally make it possible to accomplish this but it can have a non-trivial negative impact on performance and/or memory.

@jelbourn
Copy link
Member

@thw0rted the thinking is that are generally more rare than the simple case. Adding the proposed API would definitely be useful for those more complex scenarios, but at the trade-off a tiny bit more complexity and size. And while it's inarguably a tiny addition here, lots of tiny additions like this accumulate over time. One of the more consistent themes in the feedback we get on Angular is that it's too complex and that the learning curve is too steep. Every small API addition (like this one) contributes to that, even if they are minor in isolation. Another point of feedback has been around payload size. Again 500 bytes seems inconsequential here, over time it builds up to extra kilobytes of features that are sparsely used. While we're obviously working on that in other areas as well, we have to be super mindful of the cost of every addition. I wish there was a way to satisfy every use-case, but we ultimately have to make a call on the tradeoffs, leaving someone unhappy.

@jelbourn
Copy link
Member

@criskrzysiu closing the issue signals that we don't plan to merge the PR, but folks are still welcome to follow-up on discussion afterwards. Closed issues/PRs are automatically locked only after 30 days of inactivity, which is mainly to prevent people from drive-by commenting on issues that were closed years ago with not-super-related questions (this happens a lot).

As for rationale, I tried to summarize where we ended up, but going into more depth, it was a combination of factors:

  • The extra payload size is small here, but these small additions add up over time
  • Any expansion of the API surface increases the complexity of the framework, which is something we're actively trying to work on (i.e. make things simpler)
  • Comparing this to ngIf: native if statements have an else clause as a widely recognized construct, but for loops don't have something similar
  • The proposed API requires using an ng-template element and a template variable, which we see as being comparatively more complex than ngIf
  • We've discussed on-and-off the idea of someday moving from ngIf and ngFor directives to something built into the templating language in order to reduce the cost of ngIf and ngFor as directives (basically optimizing them in the framework directly). Adding more APIs to ngFor makes a path for such a potential change less clear.

The team genuinely takes feedback seriously, but ultimately have to make a decision about which path has the most benefit/cost to the largest number of applications. We can definitely be wrong, but we're doing our best in trying to synthesize everything we hear from different parts of the community.

@Airblader
Copy link
Contributor

Airblader commented Dec 10, 2020

One of the more consistent themes in the feedback we get on Angular is that it's too complex

From my experience onboarding people this is true, and having to jump through hoops to make simple things work is part of that problem.

More API surface is not equivalent to a higher complexity per se. To name a different example, Angular not having an equivalent of functional components in React, ie components that don't require all of this overhead of a class and decorators and declaring them, causes significant complexity to achieve something as simple as sharing a tooltip across places in an application.

i.e. make things simpler

The roadmap contains no such item. Would you mind sharing the things the Angular team is working on / planning on doing to achieve this? Does this consequently (to the above reasoning) mean there are plans to deprecate certain APIs?

I really think Angular would benefit from communicating such internal efforts with the open source community better. Unfortunately it seems a lot is happening behind closed doors.

native if statements have an else clause as a widely recognized construct, but for loops don't have something similar

Fair point.

The proposed API requires using an ng-template element and a template variable, which we see as being comparatively more complex than ngIf

To me this just reads like "ng-template is good enough for user code, but not good enough for official APIs", which is a disappointing message to bring across.

How would people ever learn to use some of the more advanced Angular features if the framework itself doesn't want to use them?

We've discussed on-and-off the idea of someday [...] Adding more APIs to ngFor makes a path for such a potential change less clear.

Not moving forward because of a hypothetical future that may or may not exist in some form is end-stage design paralysis. It's just not a good reason IMO, and a blanket argument that could be used to deny any feature request.

in order to reduce the cost of ngIf and ngFor as directives (basically optimizing them in the framework directly)

All the more reason such a framework level feature supported actual, average, every day use cases like showing an empty state for empty collections.

@jelbourn
Copy link
Member

jelbourn commented Dec 10, 2020

The roadmap contains no such item. Would you mind sharing the things the Angular team is working on / planning on doing to achieve this? Does this consequently (to the above reasoning) mean there are plans to deprecate certain APIs?

It's not exactly a roadmap item, more of a point of consideration that goes into deciding what to work on / how to spend our time. This was a part of the rationale behind ivy- simplifying the generated code, getting rid of concepts like component factories / ComponentFactoryResolver, class bindings that behave more predictably, etc. For actual projects, we are trying to be pretty transparent by doing RFCs (like the recent #38366 for ng-linker). Some projects do inherently come from our support obligations within Google, though, so we try to at least account for those in the roadmap.

To me this just reads like "ng-template is good enough for user code, but not good enough for official APIs", which is a disappointing message to bring across.

That's not quite the intent here. It's more that using the API has has more going if you were to try to explain it to someone new. E.g, with this simplified example:

<ul>
  <li *ngFor="let user of users">{{user.name}}</li>
</ul>
<div *ngIf="!users.length" class="empty-message">
  No users found
</div>

vs.

<ul>
  <li *ngFor="let user of users; whenEmpty: emptyMessage">{{user.name}}</li>
</ul>
<ng-template #emptyMessage>
  <div class="empty-message">
    No users found
  </div>
</ng-template>

The latter example just has more stuff to explain (the ng-template and the template var). It's not insurmountable, but still a trade-off. For any APIs where an ng-template makes sense, there's no reservation in using them (e.g. we use them quite extensively in the cdk and material).

Not moving forward because of a hypothetical future that may or may not exist in some form is end-stage design paralysis. It's just not a good reason IMO, and a blanket argument that could be used to deny any feature request.

Yeah, this wasn't a particularly impactful point in deciding anything, and we would have decided the same thing without it. I just wanted to mention it to highlight that even small changes can have unexpected long-term implications. And, honestly, our goal of providing smooth upgrade paths for all apps does make us pretty conservative about changes to the framework because we bear the cost of bringing users along with those changes. Those cost weigh heavily on the work we prioritize, which means we tend to do more work on stability and backwards compatibility than new features.

@Airblader
Copy link
Contributor

Fair enough. Thanks for your responses, I appreciate that!

@petebacondarwin
Copy link
Member

@Airblader et al - FWIW it is possible to create a fairly small directive of your own that will achieve this result - although I haven't checked the corner cases. I might be missing something:

https://stackblitz.com/edit/angular-ivy-grtarz?file=src%2Fapp%2Fapp.component.html

The nice thing about this is that the application developer doesn't need to use any particularly new syntax, just add the WhenEmptyDirective to their project (perhaps as an import) and the bind to the extra [whenEmpty] input where desired.

@Airblader
Copy link
Contributor

Yeah, I'm aware I can create my own directives. :-)

My issue with this is, however, that one of Angular's selling points, to me anyway, is that compared to competitors like React it's more of an out of the box solution. That is the trade-off for it not being as lightweight as them, and it's something that can reasonably be argued when talking to people. However, the entire argument goes down the drain when very basic useful features are not added. My pet peeve for this is #15280, and this one is just closely related to it for me. I've had some newjoiners on my team coming from React lately, and I like showing them the benefits Angular brings to the table, but I have to admit that they also have been making very good points about things in Angular that are just frustratingly immature for the framework at this point, be it ngLet, type-safety issues in templates, DX for discoverability of directives and content projection, …

@petebacondarwin
Copy link
Member

petebacondarwin commented Dec 18, 2020

Lots of the Angular built-in pieces expect to be extended: consider router guards, http interceptors, form validators, etc.

My example shows that you even extend ngFor to provide a new feature without it having to be in the core. Using this does not require the application developer to stop using ngFor (and use a custom directive like appFor), they can just continue to use the built-in and extend it for the cases where it should work differently.

The question of what should be "out of the box" is very subjective. It is always a trade off. And in this case it seems to me that this is an extension scenario rather than a core feature of Angular.

Regarding your pet peeve, it looks like it has dropped off the radar since #15280 (comment). Perhaps something to bring back to the table in the New Year @IgorMinar ?

@Airblader
Copy link
Contributor

The question of what should be "out of the box" is very subjective. It is always a trade off.

It is, and this issue has 166 upvotes at the time of writing. So clearly this is not an edge-case scenario, but something a lot of people run into and expect the framework to handle.

Lots of the Angular built-in pieces expect to be extended: consider router guards, http interceptors, form validators, etc. […] My example shows that you even extend ngFor to provide a new feature

Guards and interceptors are (purely) APIs specifically built around extending them. When trying to extend ngIf or ngFor, some things are possible, but you also very quickly hit roadblocks, because they are not really written with the intention of extending them. In general directives are hard to "extend" at the moment, for example I tried to extend *ngFor's trackBy with a solution for another popular feature request (#12969), see my blog post, but it turns out that this doesn't actually work all too well in practice. Other examples include #8785 (at least this is now on the roadmap after four years) or #27672 (as an example of extendability of Forms). I can go on with other examples of where extending Angular is problematic.

Anyway, I don't really want to argue this decision. :-) I get the motivation of why it was rejected (and appreciate you guys taking the time to respond). I also don't want to look like the guy who's hating on Angular. I enjoy Angular and working with it quite a lot, and I try to show others all the good things about Angular, but I also had to onboard a bunch of people coming from other frameworks and they often raise good points where Angular just feels "feature-spotty".

@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jan 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: common Issues related to APIs in the @angular/common package feature Issue that requests a new feature P4 A relatively minor issue that is not relevant to core functions state: has PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.