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

Question: Does transaction combine actions to multiple stores? #54

Closed
DmitryEfimenko opened this issue Aug 20, 2018 · 11 comments
Closed
Labels
help wanted Extra attention is needed

Comments

@DmitryEfimenko
Copy link
Contributor

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[ ] Performance issue
[ ] Feature request
[X] Documentation issue or request
[ ] Support request
[ ] Other... Please describe:

Question

This issue is just a question / potential request for clarifications in the docs.
The documentation provides examples where the transaction is applied to the methods in the same store. I poked around the code for @transaction, but could not quite understand how it works. The question is whether it can be applied to actions in different stores. For example:

@transaction
actionAffectingTwoStores() {
  this.storeOne.add({ name: 'asd' });
  this.storeTwo.add({ name: 'qwe' });
}

Thanks for the awesome library.

@NetanelBasal
Copy link
Collaborator

NetanelBasal commented Aug 20, 2018

@transaction only optimized updates on the same store. So for example, if you call the following code:

@transaction
method() {
  this.storeOne.setActive(1);
  this.storeOne.add({ name: 'qwe' });
}

Subscribers to the store will get one notification instead twice (one for each call update).

If it answers your question, please close the issue. Thanks.

@NetanelBasal NetanelBasal added the help wanted Extra attention is needed label Aug 20, 2018
@david-bulte
Copy link

Hi @NetanelBasal. Thanks for the great framework! Do you consider adding this in a future release?

The use case I'm thinking of is when you have 2 related entities, say Movie and Actor, where Movie has a field actorIds. There is a selector that returns the Actors of the "active" Movie. This goes well when you first add the Actors entities and then sets the active Movie. When you do it the other way around though things fall apart.

So there is a workaround, but it would be handy to do these changes in an atomic way.

@NetanelBasal
Copy link
Collaborator

Thanks, can you add a concrete example (stackblitz) so we can look into this?

@david-bulte
Copy link

david-bulte commented Mar 29, 2019

Here's an example: https://stackblitz.com/edit/angular-rqbnpv.

Have a look at ActorQuery's selectActorNames() (see below) which combines the Actor and Movies store. Depending on whether we first add the Actors and call the MovieStore.setActive() method afterwards or vice versa, the selectActorNames() method succeeds or fails. This is because when we first call the setActive() method, the selector is immediately triggered, and at that time we don't find the corresponding Actors.

Ideally the selectors should only be triggered after each 'tick'/event? I dunno. Or otherwise use the transaction mechanism for this?

actor.query.ts

  selectActorNames(): Observable<Actor[]> {
    return combineLatest(this.movieQuery.selectActive(), this.selectAll({asObject: true})).pipe(
      filter(([movie, actors]) => !!movie && !!actors),
      map(([movie, actors]: [Movie, any]) => {
       return movie.actorIds.map(actorId => actors[actorId].name)
      })
    );
  }

movie.service.ts

  load(id: string) {
    this.mockHttp(id).subscribe(({movie, actors}) => {      
        this.movieStore.add(movie);

        // this is ok
        // this.actorStore.add(actors);
        // this.movieStore.setActive(movie.id);

        // this is not
        this.movieStore.setActive(movie.id);
        this.actorStore.add(actors);
    })
  }

@NetanelBasal
Copy link
Collaborator

As I mentioned in my article, you should use the auditTime operator:

  selectActorNames(): Observable<Actor[]> {
    return combineLatest(this.movieQuery.selectActive(), this.selectAll({asObject: true})).pipe(
      auditTime(0),
      filter(([movie, actors]) => !!movie && !!actors),
      map(([movie, actors]: [Movie, any]) => {
       return movie.actorIds.map(actorId => actors[actorId].name)
      })
    );
  }

@NetanelBasal
Copy link
Collaborator

@david-bulte I'm working a prototype that improves the transaction API. First, a new operator:

  getMovies() {
    return timer(1000).pipe(
      mapTo(movies),
      withTransaction(response => {
        this.actorsStore.set(response.entities.actors);
        this.genresStore.set(response.entities.genres);
        const movies = {
          entities: response.entities.movies,
          ids: response.result
        };
        this.moviesStore.set(movies);
      })
    );
  }

The second is a built-in solution for the case you mentioned, something like:

  selectActorNames(): Observable<Actor[]> {
    return combineLatest(this.movieQuery.selectActive(), this.selectAll({asObject: true})).pipe(
      batch(),<====
      filter(([movie, actors]) => !!movie && !!actors),
      map(([movie, actors]: [Movie, any]) => {
       return movie.actorIds.map(actorId => actors[actorId].name)
      })
    );
  }

@NetanelBasal
Copy link
Collaborator

@david-bulte check the new functionality here:

https://stackblitz.com/edit/angular-brebji?file=src%2Fapp%2Factor.query.ts

It's still experimental so let me know if there any issues.

@david-bulte
Copy link

david-bulte commented Apr 1, 2019

Thanks. This is working fine.

Some more thoughts, some may be relevant, others probably out of scope:

  • What's not (yet?) working are transactions spanning multiple async operations:
  load(id: string) {
    this.mockHttp(id).pipe(
      withTransaction(({ movie, actors }) => {
        this.movieStore.add(movie);
        this.movieStore.setActive(movie.id);
        // this.actorStore.add(actors);
        this.addActors(actors);
      }),
    ).subscribe()
  }

  addActors(actors) {
    timer(500).pipe(
// ---> I also tried with nested transactions <---
//      withTransaction(() => {
//        this.actorStore.add(actors);
//      }),
    ).subscribe(() => {
        this.actorStore.add(actors);
    })
  }

  • I like the waitForTransaction() selector, it clearly communicates what is going on. On the other hand, one could argue that the selectors shouldn't be triggered as long as the transaction hasn't been committed. Selectors shouldn't know whether something was added in a transaction or not (separation of concerns). Would filtering on a transaction state help? Something along these lines?
  const tx$ = new BehaviorSubject(false);

 _select<R>(project) {
    return combineLatest(this.store.asObservable(), tx$).pipe(
      filter(([state, tx]) => !!tx),
      map(([state]) => state)
      map(project),
      distinctUntilChanged()
    );
  }

@NetanelBasal
Copy link
Collaborator

NetanelBasal commented Apr 1, 2019

What's not (yet?) working are transactions spanning multiple async operations:

Transactions are only for synchronous operations; we can't know that all the async operations are done. (unless we use zone js and we don't want to)

Selectors shouldn't know whether something was added in a transaction or not (separation of concerns).

The waitForTransaction is relevant only for a combineLatest/merge functionality. Adding it per selector won't help.

@NetanelBasal
Copy link
Collaborator

If you have a nice suggestion for a solution to your first issue, share with us.

@hgoebl
Copy link

hgoebl commented Apr 27, 2020

I would say transactions are not necessary over async operations. This can be achieved by waiting for both operations to complete and then manipulate the store.

But manipulating multiple stores so observables only fire once is very important in order to avoid ripple-effects. Otherwise, every selector spanning multiple stores has to use the auditTime operator. Not sure, but I can imagine that this also might add problems (values from single store access are rendered and next tick the things from multiple stores force a re-rendering.

I don't have a stackblitz and just came over this question because we're currently choosing a state mgmt solution (competitors are NGXS and NgRx).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants