feat(common): ngLet directive #25472
Conversation
`ngLet` expose the result of an expression that is stored as a variable to its child view. Fixes #15280
|
This feature is tightly related to #25251, which is in some sense more general, but maybe a bit less readable. For me (as the author of the other pull request), both solutions are quite fine. The fact that these pull requests emerge indicates that this is a real deficiency that should be addressed somehow, by merging either one (or by proposing a completely different solution). |
| } | ||
|
|
||
| @Input() | ||
| set ngLet(condition: any) { |
There was a problem hiding this comment.
should we name it condition?
we can absolutely use something like *ngLet="data$ | async as data" here.
There was a problem hiding this comment.
Absolutely! It's not expected to be a "condition" at all. Thanks for pointing that out. Maybe value or data is more appropriate?
| set ngLet(condition: any) { | ||
| this._context.$implicit = this._context.ngLet = condition; | ||
| this._viewContainer.clear(); | ||
| this._viewContainer.createEmbeddedView(this._templateRef, this._context); |
There was a problem hiding this comment.
why you put createEmbeddedView here? Is that mean we will createEmbeddedView everytime input change?
I think we just need to createEmbeddedView once in the ngOnInit hook and only changes the value of property in the context object when Input changes.
@Input()
set ngLet(value: any) {
this._context.$implicit = this._context.ngLet = value;
}
constructor(private _vcr: ViewContainerRef, private _templateRef: TemplateRef<NgLetContext>) {}
ngOnInit() {
this._vcr.createEmbeddedView(this._templateRef, this._context);
}There was a problem hiding this comment.
This is an excellent point... I didn't even consider that updating context on input changes would be enough 🤔
This makes a lot of sense, will update 👍🏼
| }); | ||
|
|
||
| it('should support binding to variable using let', async(() => { | ||
| const template = '<span *ngLet="booleanCondition let v">{{v}}</span>'; |
There was a problem hiding this comment.
Correct me if I'm wrong but I think the syntax should be *ngLet="booleanCondition; let v" (notice the ;)
There was a problem hiding this comment.
It's good either way, the semicolon isn't required 🙂
change argument to be more semantic.
namely aliasing and displaying static and observable values.
|
@sandangel Updated tests to better assert the functionality we're expecting from this directive, namely aliasing and displaying the result of our expression. Let me know what you think 😄 |
|
Hi @ibarsi , I think you should have a look at |
| }); | ||
| }); | ||
|
|
||
| it('should support binding to observable using as', async(() => { |
There was a problem hiding this comment.
Dont use EventEmitter for the purpose of an Observable instance. Use a Subject or similar.
|
hi there. thanks for the PR. I've posted on #15280 (comment) about how we should go about implementing this feature. The implementation in this PR can't be accepted as it would (eventually) result in perf issues. Thanks for exploring this problem space. Your work will inform future implementations. |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
ngLetexposes the result of an expression that is stored as a variable to its child view.Fixes #15280
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #15280
What is the new behavior?
ngLetevaluates the expression and, if stored in a variable, provides the result to its child view.Storing falsy result in a variable
ngIfsupports the common use case of storing a value in a variable and exposing it to its child view.This works well for most cases. However, if the value stored is falsy (ie.
false,0,'', etc) the child view will not be rendered. This can be problematic if the stored value is required to be displayed, or provided as an argument to an output or DOM event.ngLetworks around this issue by always rendering the child view, regardless of the result of its expression.Semantic meaning
Even in the cases where
ngIfworks, some semantic meaning is lost. Reading the code doesn't communicate whether the auther intended contitional rendering or simply needed to expose the result of the expression to its child view.ngLetexplicitly communicates semantic meaning, illustrating that the intent of the author's code was to expose the result of the expression, not conditional rendering.Syntax
Does this PR introduce a breaking change?
Other information
N/A