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

Template Local Variables #15280

Open
kylecordes opened this issue Mar 18, 2017 · 172 comments
Open

Template Local Variables #15280

kylecordes opened this issue Mar 18, 2017 · 172 comments
Assignees
Labels
area: core Issues related to the framework runtime feature: under consideration Feature request for which voting has completed and the request is now under consideration feature Issue that requests a new feature freq3: high hotlist: google
Milestone

Comments

@kylecordes
Copy link

[X] feature request

Current behavior

The new syntax is a great improvement!

<div *ngIf="userStream|async as user">...</div>

... And was discussed at length in #15020.

Expected behavior

The ability to capture the async pipe output and make it available as a template variable, is much appreciated and very useful. Unfortunately, this capability is currently bound closely to *ngIf; it is only available is a feature of *ngIf. (I am aware that there is something similar for *ngFor, unrelated to this feature request).

Here's what I request: that this capability be yanked apart from the conditional (*ngIf), and made available elsewhere.

I don't know what syntax would make the most sense. Perhaps:

<div *ngLet="userStream|async as user">...</div>

Or alternatively; it would be excellent to have a syntax to directly use any observable anywhere in a template, with the reactivity handled by Angular, and without worrying about potentially multiple async pipe instances needlessly subscribing many times. For example, imagine the following syntax:

<p>Hello, {{ userStream^.name }}</p>

What is the motivation / use case for changing the behavior?

I have a use case in front of me, the details which don't matter, where:

  • I have a handful of observable streams with data to use in the template.
  • Some of them occasionally have falsely values; but I still want to use them even if that is the case.
  • Currently I'm doing some hackery to deal with the falsely problem, and nesting a big chunk of my template in a handful of nested *ngIfs which exists solely for the purpose of giving access to this ability to capture the async values to template variables.
@DzmitryShylovich
Copy link
Contributor

<div *ngLet="userStream|async as user">...</div>

you can create such directive yourself

@kylecordes
Copy link
Author

@DzmitryShylovich yes, I could of course. Just a copy of ng_if with the key bit of conditional code removed. Still, I have this here is a feature request, because I think it makes much sense as a feature of Angular for everyone to use. I'd be happy to implement it, of course.

(Incidentally, I put together something roughly analogous to the ; let user or as user functionality, 487 days ago, link below, inside of an early A2 firebase adapter library. The lesson here: building my own little thing in my own sandbox, doesn't really help anyone else.)

https://github.com/OasisDigital/angular2-firebase/blob/cff0e5d78d2022c4fbc3b145c4bf2a9543e20275/lib/ngWhen.ts

@mlc-mlapis
Copy link
Contributor

@DzmitryShylovich You mean like your one, right? http://plnkr.co/edit/Sunu2LPQVAaWZvZRcWXb?p=preview

@DzmitryShylovich
Copy link
Contributor

@mlc-mlapis yeap

@IgorMinar IgorMinar added area: core Issues related to the framework runtime feature Issue that requests a new feature labels Mar 20, 2017
@ddavid67
Copy link

ddavid67 commented Mar 23, 2017

I second this.
I want to toggle a named router outlet :

<a *ngIf="asideShow$ | async as link" [routerLink]="[{outlets:{aside: link}}]">toggle</a>

The local link variable is either null or a string for the required named outlet to display.
If the asideShow observable emits a null value, the <a> element will dissapear...

How can I achieve this without ngIf ?

@colltoaction
Copy link

I've found another place where this would be useful and it might help turning your opinion. I have list of favorites, and a message when there's no favorite. Since the variable is not null, but an empty array, *ngIf evaluates to true and I have to workaround it.

<ng-container *ngIf="favoritesObservable | async as favorites">
    <my-list *ngIf="favorites.length > 0; else noFavoritesMessage"
             [elements]="favorites">
    </my-list>
    <ng-template #noFavoritesMessage>
        Your favorites list is still empty. To add the first one you can ...
    </ng-template>
</ng-container>

Having to have a wrapping ng-container is not ideal, and I'd argue *ngLet would be a good alternative. I'd also be happy with evaluating an empty list to false, but I imagine that's a huge breaking change.

@DzmitryShylovich
Copy link
Contributor

@tinchou you can map empty array to null

favoritesObservable.map(arr => arr.length ? arr : null)

@Modellium
Copy link

It is clear that having multiple subscription of the same observable is not the way to go.

There is absolutely no reason that angular should not have a way to create a local variable, might it be ngLet or something else.

Creating loca variables in ngIf is just a hack to achieve that functionnality. And this hack reaches its limit when you need to use a falsy value from your local variable.

@pkt-zer0
Copy link

While local variales in templates could still make sense, avoiding multiple subscriptions could also be done with the share operator on Observables.

@mischkl
Copy link

mischkl commented Oct 2, 2017

Also discussed in some depth in #2451

@crysislinux
Copy link

we are using this in production

import {
  Directive,
  Input,
  OnDestroy,
  OnChanges,
  EmbeddedViewRef,
  ViewContainerRef,
  TemplateRef,
  SimpleChanges,
  ChangeDetectorRef,
} from '@angular/core';
import { Observable } from 'rxjs/Observable';
import { Subscription } from 'rxjs/Subscription';
import 'rxjs/observable/from';

@Directive({
  selector: '[btContext][btContextOn]',
})
export class ContextDirective implements OnChanges, OnDestroy {
  @Input() btContextOn: Observable<any> | Promise<any>;

  private viewRef: EmbeddedViewRef<any>;
  private sub: Subscription;

  constructor(private template: TemplateRef<any>, private vcr: ViewContainerRef, private cdr: ChangeDetectorRef) { }

  ngOnChanges(changes: SimpleChanges) {
    if (!this.viewRef) {
      this.viewRef = this.vcr.createEmbeddedView(this.template);
    }

    const ch = changes['btContextOn'];
    if (ch && ch.previousValue !== ch.currentValue) {
      this.viewRef.context.$implicit = null;

      if (this.sub) {
        this.sub.unsubscribe();
        this.sub = null;
      }

      if (ch.currentValue) {
        const obs = Observable.from(ch.currentValue);
        this.sub = obs.subscribe(state => {
          this.viewRef.context.$implicit = state;
          this.cdr.markForCheck();
        });
      }
    }
  }

  ngOnDestroy() {
    if (this.sub) {
      this.sub.unsubscribe();
    }
  }
}

the usage is

<div *btContext="let something on something$">
  <!-- you can use {{ something?.somefield }} here -->
</div>

@xmlking
Copy link

xmlking commented Mar 4, 2018

is there a better syntax for this template?

<h1 *ngIf="isLoggedIn$ | async; else anonymous">
  <div  *ngIf="profile$ | async as profile;">   Hallo, {{profile.name}}</div>
</h1>
<ng-template #anonymous>
  <h1>Hallo</h1>
</ng-template>

@sandangel
Copy link

sandangel commented Mar 4, 2018

@xmlking may be you need *ngLet
@ngrx-utils
https://github.com/ngrx-utils/ngrx-utils

@jotatoledo
Copy link
Contributor

@xmlking thats kinda off topic though, you are asking for support...

@nnennajohn
Copy link

nnennajohn commented Apr 25, 2018

Seems this is an old thread and not being implemented. But I thought I'd add one more use case.

<ng-container *ngIf="(applicationMethods$ | async) as methods; else noAppMethods">
        {{ methods[recItem.products.applicationMethodType.applicationMethodId]?.title }}
</ng-container>
 <ng-template #noAppMethods>
        Some random alternate logic...   
 </ng-template>

Given the code above, we do a lot of data mapping on the client side. applicationMethods$ is just a map amongst several maps saved in the store. The problem here is that after using an async pipe here, it seems anywhere else I call it in the same template does not work. That is, perhaps it takes the subscription and completes or something funky like that. :)

Technically, I'd like to use the same map elsewhere in the template. How do I accomplish this without going to the component file. An NgLet for the map would have been ideal, and maybe would alllow me multiple subs of it.

@jotatoledo
Copy link
Contributor

@nnennajohn thats a different issue. If you look the src code of the async pipe, you would notice that it doesnt do something similar to what you describe.

I can think of 2 (common) options:

  • Wrap all (template) nodes that depend on the value emited by applicationMetdhos$ inside of a ng-container
  • Make applicationMetdhos$ capable of replaying its last emitted value

sanbornsen added a commit to sanbornsen/angular that referenced this issue Aug 7, 2018
- This helps to store async value locally in the template
- This fixes angular#15280
sanbornsen added a commit to sanbornsen/angular that referenced this issue Aug 14, 2018
- This helps to store async value locally in the template
- This fixes angular#15280
sanbornsen added a commit to sanbornsen/angular that referenced this issue Aug 14, 2018
    - This helps to store async value locally in the template
    - This fixes angular#15280
sanbornsen added a commit to sanbornsen/angular that referenced this issue Aug 14, 2018
        - This helps to store async value locally in the template
        - This fixes angular#15280
sanbornsen added a commit to sanbornsen/angular that referenced this issue Aug 14, 2018
        - This helps to store async value locally in the template
        - This fixes angular#15280
sanbornsen added a commit to sanbornsen/angular that referenced this issue Aug 14, 2018
            - This helps to store async value locally in the template
            - This fixes angular#15280
sanbornsen added a commit to sanbornsen/angular that referenced this issue Aug 14, 2018
                - This helps to store async value locally in the template
                - This fixes angular#15280
@otodockal
Copy link
Contributor

@mauriziocescon I would be surprised if template variables are allowed outside of if/for blocks.

@TomTomB
Copy link

TomTomB commented May 8, 2024

I actually quite like the approach Svelte takes here. In Svelte, all variable declarations must directly follow after the control flow block. Putting them somewhere random would result in a compile error if I recall correctly. But of cause this would take away the freedom to eg. group variables & their respective html markup logically like so:

@let userFormCtrls = form.controls.user.controls;

@let addressFormCtrls = userFormCtrls.controls.address.controls;
<label>
  Street:
  <input [formControl]="addressFormCtrls.street" >
</label>

@let otherFormCtrls = userFormCtrls.controls.other.controls;
<label>
  Other:
  <input [formControl]="otherFormCtrls.something" >
</label>

I'm leaning towards the latter option imho. Seems a lot cleaner than just stacking a lot of let variables right inside my if condition. But in the end there could always be some kind of lint rule to enforce a certain style.

@alxhub
Copy link
Member

alxhub commented May 9, 2024

Assuming I'm not missing something obvious (then my bad), there's no constraint between variables definition and html structure, right? Meaning, this (or more complex variants) would be fine

<div>
   <div>
      @let something = ...
      ...
   </div>
</div>
<div>
   @if (something) {...}
</div>

Correct, that'd be legal. The same as:

<div>
  <input #something>
</div>
{{ something.value }}

Obviously variables should be declared as early as possible, and I'm sure someone will write a lint check to enforce that. But there's nothing technically wrong with the first example, and sometimes a variable needs to go in a strange place because it needs to be evaluated at a specific time. This is one of the arguments against @let as a block - it forces you to put the evaluation above any DOM structures where the variable is used. Sometimes that's just not possible. This template cannot be written with a scoped @let block:

<div>
  <input [ngModel]="data" #m="ngModel">
  @let value = process(m.value);
  <p>{{value}}</p>
</div>
{{ value }}

It would look like:

@let (value = process(m.value)) {
  <div>
    <input [ngModel]="data" #m="ngModel">
    <p>{{value}}</p>
  </div>
  {{ value }}
}

Here value gets assigned too early - before the [ngModel] binding which updates m.value. This is a contrived example, but it shows the issue with restricting variables to a block of the template.

@antischematic
Copy link

Refs have dynamic scope so shouldn't this also be legal?

<app-autocomplete [for]="m"></app-autocomplete>

@let value = process(m.value);
<div>
  <input [ngModel]="data" #m="ngModel">
  <p>{{value}}</p>
</div>
{{ value }}

Then this should also be possible.

<app-autocomplete [for]="m"></app-autocomplete>

@let (value = process(m.value)) {
  <div>
    <input [ngModel]="data" #m="ngModel">
    <p>{{value}}</p> 
  </div>
  {{ value }}
}
{{ value }} <!-- error -->

@LcsGa
Copy link

LcsGa commented May 9, 2024

In the end I would lean towards using const simply to disallow something like the following from happening.

@let isVisible = isStuffVisible();
<button (click)="isVisible = !isVisible">Toggle</button>

This wouldn't be permitted regardless of the syntax - Angular will tell you that template variables are read-only.

Whereas your previous answer made it clearer why you proposed @let, this answer here is the reason why I feel like @const is better suited.
The value will be reassigned, but internally only. We won't be able to do it ourselves, which is why it seems more appropriate.

@alxhub
Copy link
Member

alxhub commented May 9, 2024

@antischematic legal yes, but not correct. The example is a bit contrived, but it shows a situation where timing is important.

[ngModel]="data" is evaluated with the <input>, which updates the NgModel directive's internal state, which changes m.value. So if we move the @let before the binding, we'll compute process(m.value) on the old value of m.value, before it updates.

This is why this template throws ExpressionChanged:

{{ m.value }}
<input [ngModel]="data" #m="ngModel">

But this one does not:

<input [ngModel]="data" #m="ngModel">
{{ m.value }}

@danielglejzner
Copy link

danielglejzner commented May 17, 2024

Would that be considered a proper usage?
Screenshot 2024-05-17 at 13 27 41

If so I don't really like the freedom part.

If used like this, change seems nice. 👇
Screenshot 2024-05-17 at 15 05 51

@crisbeto
Copy link
Member

Both are valid usages so it's up to you to choose how you write your code.

@thw0rted
Copy link

I just had an evil idea:

@for(tagDescription of [user.posts[3].tags[2].description]; track $index) {
  {{tagDescription}}
}

At least I guess we don't need a scoped @let? (Honestly, this sucks, please don't do this.)

@danielglejzner
Copy link

Both are valid usages so it's up to you to choose how you write your code.

Fair enough - more unreadable code to shovel out for me :D

But I get it - objectively it's preference.

@solos
Copy link

solos commented May 17, 2024

This feature is unnecessary. The variables should not be defined in template. The variables should be changed in typescript. The template layer should be simple and just used to render views. Please do not add more to template.

@fMoro1999
Copy link

This feature is unnecessary. The variables should not be defined in template. The variables should be changed in typescript. The template layer should be simple and just used to render views. Please do not add more to template.

I definitely do not agree with this. The new let syntax is just a tool. I am also a big fan of clean templates, but how do you deal with situations in which you need to re-use thevalue of a resolved observable in multiple parts without subscribing to it multiple times?

Typically in previous versions you could

  1. Implement a directive to keep stored into a template variable the resolved value from the observable. Why should not this be "natively" supported by the framework? I am happy with this new feature
  2. Implement a view model constant into the .ts which combines multiple reactive sources through combineLatest (or with computed in a signal approach). Generally simple, but sometimes you can bump into non trivial situations
  3. ...

@solos
Copy link

solos commented May 17, 2024

This feature is unnecessary. The variables should not be defined in template. The variables should be changed in typescript. The template layer should be simple and just used to render views. Please do not add more to template.

I definitely do not agree with this. The new let syntax is just a tool. I am also a big fan of clean templates, but how do you deal with situations in which you need to re-use thevalue of a resolved observable in multiple parts without subscribing to it multiple times?

Typically in previous versions you could

  1. Implement a directive to keep stored into a template variable the resolved value from the observable. Why should not this be "natively" supported by the framework? I am happy with this new feature
  2. Implement a view model constant into the .ts which combines multiple reactive sources through combineLatest (or with computed in a signal approach). Generally simple, but sometimes you can bump into non trivial situations
  3. ...

I will use the second one and i think others use this way before. I do not know why they need this feature. Maybe you are right.

@thw0rted
Copy link

I think there is still a use case for this. Consider a list whose list-items have an observable property:

@for(item of items; track item.id) {
  <mat-list-item [class.looksDisabled]='!(item.enabled$ | async)'>
    {{ item.name }} {{ (item.enabled$ | async) ? 'x' : 'o' }}
  </mat-list-item>
}

It's contrived but obviously a common use case. One extra subscription might not be a big deal but sometimes it's a lot more.

Now, I might solve this by adding an extra conditional element to the markup, to create a temporary variable in the for-loop scope using ngIf. You could also make a whole component just for these list items, but it's a ton of extra maintenance overhead for what amounts to a couple lines of super simple template logic. It would be much better to create a locally-scoped variable inside the for-loop directly in the template, without requiring superfluous markup.

@Harpush
Copy link

Harpush commented May 17, 2024

I think the most important thing is when will it be evaluated. If it will work like signals so if we do: @let a = b() + c() it will only run when b or c changes - then that's awesome as it allows @for item based computed.

@crisbeto
Copy link
Member

First of all, this is a completely optional feature so you can decide not to use it in your project. The code for it will be tree-shaken if you don't use it. As for advantages over existing solutions, these are some off the top of my head:

  • It doesn't need to instantiate an embedded view and a directive to declare a variable. This means that we generate less JS during compilation and consume less memory at runtime.
  • Better type checking support, because we know exactly how to infer and narrow the type of @let.
  • It allows you to declare multiple variables per embedded view, whereas existing directives only allow one.

@rip222
Copy link

rip222 commented May 18, 2024

First of all, this is a completely optional feature so you can decide not to use it in your project. The code for it will be tree-shaken if you don't use it. As for advantages over existing solutions, these are some off the top of my head:

  • It doesn't need to instantiate an embedded view and a directive to declare a variable. This means that we generate less JS during compilation and consume less memory at runtime.
  • Better type checking support, because we know exactly how to infer and narrow the type of @let.
  • It allows you to declare multiple variables per embedded view, whereas existing directives only allow one.

Now these are some compelling arguments!

@danielglejzner
Copy link

danielglejzner commented May 18, 2024

I have thought about it more, researched how Svelte does it.

Makes perfect sense to introduce constraints .

I love that this feature solves the pains we had.

However allowing to fantasy go wild with declaring let at the top is not compelling for me.

As mentioned "code however you like it's optional"

I'm also not sold on this reasoning.

Knowing pollution of template with overusing let will happen - to just accept it as a "choice".

Especially when this concept is already being used in the wild with the constraints to exactly avoid this scenario.

I would reconsider.

A lot of people that interacted with my thoughts on various platforms and some experts I know in real life do see the benefits that are clear.

But we are afraid about lack of the constraints.

PS: I have seen arguments from Alex as well - it's definitely a challenge for scoping it. But allowing to use it so freely is also not the best choice in my opinion.

First of all, this is a completely optional feature so you can decide not to use it in your project. The code for it will be tree-shaken if you don't use it. As for advantages over existing solutions, these are some off the top of my head:

  • It doesn't need to instantiate an embedded view and a directive to declare a variable. This means that we generate less JS during compilation and consume less memory at runtime.

  • Better type checking support, because we know exactly how to infer and narrow the type of @let.

  • It allows you to declare multiple variables per embedded view, whereas existing directives only allow one.

@crisbeto
Copy link
Member

crisbeto commented May 19, 2024

@danielglejzner can you elaborate on the kinds of constraints you're thinking about? Usually when we introduce constraints at the framework level, it's to avoid cases that break the mental model, not to arbitrarily catch code that we deem to be "inappropriate".

As for overusing template variables: I don't think that's a reason not to implement something. The goal is to provide developers the tools to build web apps, we aren't there to police their coding style. Every single part of the framework can cause issues if it's "overused". Furthermore, there's a clear need for something like this, because we've seen many cases over the years where developers have had to implement directives like <div *myLet="expression as myVar"></div> just so they can reuse an expression. This comes with all sorts of performance overhead (see #15280 (comment)) and a worse developer experience.

@danielglejzner
Copy link

danielglejzner commented May 21, 2024

@danielglejzner can you elaborate on the kinds of constraints you're thinking about? Usually when we introduce constraints at the framework level, it's to avoid cases that break the mental model, not to arbitrarily catch code that we deem to be "inappropriate".

As for overusing template variables: I don't think that's a reason not to implement something. The goal is to provide developers the tools to build web apps, we aren't there to police their coding style. Every single part of the framework can cause issues if it's "overused". Furthermore, there's a clear need for something like this, because we've seen many cases over the years where developers have had to implement directives like <div *myLet="expression as myVar"></div> just so they can reuse an expression. This comes with all sorts of performance overhead (see #15280 (comment)) and a worse developer experience.

I don't deny any of your points. If people want to break it they will.
Only constraints I have been thinking about are already mentioned in this discussion

It's hard to make a compelling case to constrain using @let in the light of what you have written.

I have been thinking that trackBy could be a good example of such constraints already introduced.
However that seems to fit in to "avoid cases that break the mental model".

If we go without constraints we need to have clear and well visible guidelines.

While specifying massive amount of variables is not going to directly affect performance ( I think ) it will affect readability so the least we can do is have a clear guidelines that already appear in many articles written on the subject.

@alxhub
Copy link
Member

alxhub commented May 21, 2024

There are only two flavors of constraint I can imagine which are reasonable to apply:

  • limit @let to the top of a scope
  • make @let itself a block that declares a scope

There is no difference functionally between these options, and this kind of constraint would not prevent the case of "many variables at the top of the template". It would only add friction to using @let.

I do think having many variables at the top is not necessarily a code smell. Expression aliases are not "logic" - you can't do anything with them that you couldn't do in a binding. In that sense, the ability to give names to your binding expressions is incredibly helpful for making templates more readable.

Other types of restrictions like "you can't make a @let variable for something only used once" or "only put @let variables in certain places" seem better enforced by linters rather than designed in restrictions.

@danielglejzner
Copy link

There are only two flavors of constraint I can imagine which are reasonable to apply:

  • limit @let to the top of a scope

  • make @let itself a block that declares a scope

There is no difference functionally between these options, and this kind of constraint would not prevent the case of "many variables at the top of the template". It would only add friction to using @let.

I do think having many variables at the top is not necessarily a code smell. Expression aliases are not "logic" - you can't do anything with them that you couldn't do in a binding. In that sense, the ability to give names to your binding expressions is incredibly helpful for making templates more readable.

Other types of restrictions like "you can't make a @let variable for something only used once" or "only put @let variables in certain places" seem better enforced by linters rather than designed in restrictions.

I have no more arguments :) thanks for the discussion.

@yjaaidi
Copy link
Contributor

yjaaidi commented May 24, 2024

I have two questions about @let and signals:

@yjaaidi
Copy link
Contributor

yjaaidi commented May 24, 2024

Did someone let https://github.com/let know that they will get spammed a lot? 🙃

@JoostK
Copy link
Member

JoostK commented May 24, 2024

We're still aiming to support narrowing, regardless of @let.

By using backticks around @let you avoid mentioning folks :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: core Issues related to the framework runtime feature: under consideration Feature request for which voting has completed and the request is now under consideration feature Issue that requests a new feature freq3: high hotlist: google
Projects
No open projects
Feature Requests
Proposed Projects