platform icon indicating copy to clipboard operation
platform copied to clipboard

RFC: Use `*ngrxLet` to create a view model object in the template

Open markostanimirovic opened this issue 3 years ago • 8 comments

Information

The *ngrxLet directive could be used to create a view model object in the template as follows:

<ng-container *ngrxLet="{ books: books$, selectedBook: selectedBook$ } as vm">
  <app-book-list [books]="vm.books"></app-book-list>
  <!-- ... -->
</ng-container>

It would be possible to pass a dictionary of observables whose results will be stored in a template variable (vm in this example). ~~If an observable name contains the $ suffix, then the suffix will be removed:~~

  • ~~books$ => vm.books~~
  • ~~books => vm.books~~

EDIT: We decided not to trim the $ suffix from property names to avoid potential naming collisions.

Describe any alternatives/workarounds you're currently using

  1. With *ngIf + async:
<ng-container *ngIf="{ books: books$ | async, selectedBook: selectedBook$ | async } as vm">
  <app-book-list [books]="vm.books!"></app-book-list>
  <!-- ... -->
</ng-container>

The same result can be achieved by using *ngrxLet + ngrxPush.

  1. With combineLatest:
@Component({
  selector: 'app-books',
  template: `
    <ng-container *ngrxLet="vm$ as vm">
      <!-- ... -->
    </ng-container>
  `,
})
export class BooksComponent {
  vm$ = combineLatest({ books: this.books$, selectedBook: this.selectedBook$ });
}

I would be willing to submit a PR to fix this issue

  • [x] Yes
  • [ ] No

markostanimirovic avatar Aug 23 '22 00:08 markostanimirovic

combineLatest is not the same - it will not emit until every source is emitted at least once. It's not an issue when all the sources are from some store, but it might not always be the case.

ngIf map is not quite reliable - in some circumstances, it might ignore the emitted value. I’m from mobile and forgot the exact case - I’ll check later from my laptop. Something related to lifecycle, 100% reproducible.

And the point of my comment is that we absolutely need it because alternatives are not good enough!

e-oz avatar Aug 24 '22 06:08 e-oz

I'd like to see the case when *ngIf map ignores emitted value, thanks @e-oz!

Btw, *ngrxLet with an observable dictionary will have the same behavior as an alternative with combineLatest.

markostanimirovic avatar Aug 24 '22 18:08 markostanimirovic

Btw, *ngrxLet with an observable dictionary will have the same behavior as an alternative with combineLatest.

Is it possible to make it safe, like

vm$ = combineLatest({ 
    books: this.books$.pipe(startWith(undefined)), 
    selectedBook: this.selectedBook$.pipe(startWith(undefined)),
});

?

Then for streams with values, nothing will be changed - they'll emit their values first (BehaviorSubject, or non-empty ReplaySubject). And values for other streams would be presented as "undefined" (and it's true). This way we can know for sure that vm$ will not be stuck.

e-oz avatar Aug 24 '22 19:08 e-oz

Is it possible to make it safe, like

vm$ = combineLatest({ 
    books: this.books$.pipe(startWith(undefined)), 
    selectedBook: this.selectedBook$.pipe(startWith(undefined)),
});

?

@e-oz In this case, all view model properties need to have | undefined in their type (similar to ngIf + async alternative) if we want a type-safe result.

I'd rather avoid this behavior because the idea for "*ngrxLet + observable dictionary" is to be used with "state" observables (observables that emit the first value synchronously) so we don't need to use the non-null assertion operator in the template for each property even if all passed observables emit the first value synchronously.

If the *ngrxLet directive is used with dictionary that contains async observables, it will render the embedded view when all passed observables emit the first value. Otherwise, if you want to always render embedded view synchronously with *ngrxLet, you would be able to use this type-safe alternative by adding startWith(undefined) to all async observables as follows:

@Component({
  selector: 'app-books',
  template: `
    <ng-container *ngrxLet="{ books: books$, selectedBook: selectedBook$ } as vm">
      <!-- 👉 the type of `vm.books` will be `Book[] | undefined` -->
      <!-- 👉 `selectedBook$` emits synchronously, so the type of `vm.selectedBook` will be `Book` -->
    </ng-container>
  `,
})
export class BooksComponent {
  readonly selectedBook$ = new BehaviorSubject<Book>(defaultBook);
  readonly books$ = this.booksService.getBooks().pipe(startWith(undefined));
}

markostanimirovic avatar Aug 29 '22 23:08 markostanimirovic

is to be used with "state" observables (observables that emit the first value synchronously) so we don't need to use the non-null assertion operator in the template for each property

Most of my "state" object fields are T|undefined, because I usually have no initial values for them. But maybe it's just me :)

But you are right - there are workarounds.

e-oz avatar Aug 30 '22 06:08 e-oz

I really like this idea. A few thoughts that come to mind:

  • This wouldn't be limited to NgRx derived state, right? Could you use it for any dictionary of 'state' observables?
  • Are there advantages/drawbacks to this approach compared to component view model selectors? *ngrxLet seems to save some code over explicit view model selectors.

edezekiel avatar Sep 02 '22 21:09 edezekiel

Hi @edezekiel,

I'm glad you like it. :)

  • This wouldn't be limited to NgRx derived state, right? Could you use it for any dictionary of 'state' observables?
  • Are there advantages/drawbacks to this approach compared to component view model selectors? *ngrxLet seems to save some code over explicit view model selectors.
  1. Yes, it could be used with any dictionary of 'state' observables.
  2. I also prefer view model selectors/observables, but that's a preference. Using view model selectors is more efficient compared to creating view models via combineLatest. On the other hand, there will be no performance difference when creating view models via the *ngrxLet directive and the combineLatest operator.

markostanimirovic avatar Sep 05 '22 22:09 markostanimirovic

Hi @edezekiel,

I'm glad you like it. :)

  • This wouldn't be limited to NgRx derived state, right? Could you use it for any dictionary of 'state' observables?
  • Are there advantages/drawbacks to this approach compared to component view model selectors? *ngrxLet seems to save some code over explicit view model selectors.
  1. Yes, it could be used with any dictionary of 'state' observables.
  2. I also prefer view model selectors/observables, but that's a preference. Using view model selectors is more efficient compared to creating view models via combineLatest. On the other hand, there will be no performance difference when creating view models via the *ngrxLet directive and the combineLatest operator.

Thanks for the response! Based on that I would definitely use ngrxLet except in cases where my component has a facade. When there's a facade I need the typed view model exposed from the facade to the container component.

edezekiel avatar Sep 07 '22 00:09 edezekiel