Feedback - Githubissues

Had some time to look at the repo, so here are my 2 cents of feedback 😄

state property is redundant. The BehaviorSubject that is used for the store property has a method .getValue() that lets you retrieve the state value outside of observable stream.
When using a BehaviorSubject, it's a good idea to keep it private, and expose an observable counterpart like public data$ = dataSubject.asObservable().

Regarding fetching the game details. In the game-details.service you can remove selectedGameId and make some changes similar to these: game-details.service


[...]
getGameById$(id: string): Observable<Game> {
if (id == null) {
  // something went wrong
  return of(null);
} else if (this.cache[id]) {
  // game details were already requested previous
  return of(this.cache[id]);
} else {
  return this.findGameDetails(id);
}
}

private findGameDetails(id: string): Observable { const gameDetails$ = this.findGameById(id).pipe( catchError((err) => { console.error(err); return of(null); }), retry(2) ); const screenshots$ = this.findScreenshotsById(id).pipe( catchError((err) => { console.error(err); return of(null); }), retry(2) );

const data$ = forkJoin([gameDetails$, screenshots$]).pipe(
  filter(
    (data): data is [Game, { image: string }[]] =>
      data[0] != null && data[1] != null
  ),
  map(([gameDetails, screenshots]) => {
    // enhance game object with screenshots
    gameDetails.screenshots = screenshots;
    this.setState({ selectedGame: gameDetails });
    return gameDetails;
  }),
  tap((game) => this.updateCache(game))
);

this.setState({ loading: true });
return data$.pipe(
  filter(Boolean),
  finalize(() => this.setState({ loading: false }))
);

} [...]

The next thing would be to decide the way of invalidating the cache.

And in the `game-details.component` you would have something like:
```typescript
...
export class GameDetailsComponent {
  game$ = this.route.params.pipe(
    delay(0),
    switchMap(({ id }) => {
      return this.facade.getGameById$(id);
    })
  );
  loading$ = this.facade.loadingGame$;
  constructor(
    public facade: GameDetailsFacade,
    private route: ActivatedRoute
  ) {}
}

Basically, wherever possible, try and use streams that end up in the templates through async pipe, avoiding subscribes in the component. Regarding the facade, you can have one file .facade.ts that would have the selectors and methods to interact with the store (as you have here), or you can have .selector.ts/.query.ts to select data from the store and .service.ts or whatever you like to call it, to update the store. In the end it's all just actions/events up, date/streams down so I don't have much to add here.

I would recommend using an existing state management solution, though :) Some libraries are: https://ngrx.io/ https://opensource.salesforce.com/akita/ https://github.com/ngneat/elf https://github.com/ngneat/query (something like react-query)

This account has a lot of nice packages for angular https://github.com/ngneat

For dynamic forms: https://formly.dev/ worked well last time I tried it.

floroz / videogame-database

Feedback #1

2