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

feedback (code structure): response to reddit post about enhancing this project #3

Open
michael-small opened this issue Sep 23, 2024 · 2 comments

Comments

@michael-small
Copy link

michael-small commented Sep 23, 2024

Hey, I am the person that responded to your reddit post.

TL;DR

  • This is a whole lot, but I like explaining things. Feel free to ask followups, but I am hoping to frontload most things, as well as provide key terms you can look up in your own time.
  • This issue focuses on one particular aspect of how to structure your code, how state could be handled in a more reactive and declarative way, but it is a rich topic.
  • I provide three Stackblitz examples for alternatives to the app.component.ts and how the files are fetched. The benefits of this refactor are spread out across the projects that I made which recreated the basic flow. Notes are in code comments in the main component file and a service defined above it, as well as some HTML unordered lists on each Stackblitz that you can read as you test out the demo.
    1. First example project sets the state of the original files in a fairly normal way to see in projects, but values which then depend on it are much more reactive.
    2. Second example project is fully declarative by doing some extra RXJS stuff - likely overkill, but your scenario here is also a bit more complex than many components out there which just load something unconditionally on page load. I talk about how easy that is in a couple spots across these examples.
    3. Third example using Angular signals from Angular 16+, with a bit of RXJS still for handling the HTTP - arguably the easiest, but could be mixed with some things of the other two approaches.

How I structured my examples of the code, and the overall benefits

I noticed that there is one method getFiles() that handles resetting and then setting assorted state, as well as the respective HTTP parameters. I think that with a few tweaks, that method and all of its responsibilities could be simplified or even removed.

Before I summarize the three approaches, some overall similarities

  • The HTTP param setting logic was moved into the same service that has the HTTP call. I mocked this via an rxjs delay(1000) aka one second, but you could just put the param logic in the getFormatedFiles of your DataService
    • Anything like the HTTP params and other HTTP stuff like headers tend to be put into the same methods in a service with the HTTP method itself, rather than done in a component.
  • The isLoading logic was moved into the service as well. This may be more of a personal preference, but in most projects I have been on or followed when it came to handling HTTP in services or stores that call them, the loading status tends to be put in services. I think it would be fine in the component as well though.
    • If you did include the loading in your service, I would do the loading of true before the call and then in the end use the RXJS operator finalize to set the loading back to false. RXJS has plenty of specific operators, but finalize is really slick for scenarios like this since it handles when the observable terminates from both being completed or from an error. Pretty common to see it in the wild, but mostly for this scenario.
  • The pattern of the loading state being declared twice, once in a private settable state and then once as a readonly public variable that is a mirror of the settable one, is called the "Service with a Subject" or "Service with a Signal" approach.
    • This is a fairly common way to handle state in Angular projects that do not use a state library. You can find plenty on that if you look around, but the nice part is that nothing outside of the service can mutate it yet you can still read it elsewhere. Good examples you see of this approach that also handle HTTP probably will also handle the loading state and do the finalize convention.
  • Many variables such as srcPaths and dstPaths were made to be "reactive" and "declarative". The originalFiles is also both of those things after the first example. You can find plenty of things if you search around with those terms, but the big benefits are
    • One class variable declaration handles the whole definition of its state. It is not re-assigned elsewhere.
    • Variables like srcPaths and dstPaths can react to other changes and also have their whole state handled. This is done either by using an RXJS stream to map the values in relation to originalFiles$, or in example number three with the relatively new Angular signals API.
    • Example number three is arguably the easiest and most future friendly in Angular, as once you handle setting some asynchronous action's signal, then you can reactively declare other values as just computed values from some signal it depends on, rather than the terminology of RXJS with its pipes and some other fancy operators.

Overall summary of the three approaches

  1. Manual subscribe event such as a button click.
    • Probably the most normal thing you would see, but still more reactive than plenty of projects.
    • You would just want to be sure to unsubscribe after the call, otherwise there may be memory leaks. I commented out an operator called takeUntilDestroyed that is introduced in Angular 16 that is quite nice for scenarios like this.
    • The setting of the value through a function which subscribes is referred to as imperative, and the values that depend on the value which is then set are considered reactive/declarative. If what you take away from all these fancy suggestions is that you can make some values fully declarative and react to other values even if those reacted-to values are manually (imperatively) set, then you would be excelling beyond a ton of code I have seen in the wild. In particular, the next point:
    • Lets say you had a component that just loads some data when it is initialized, rather than on a button press. You could merely declare files$ = this.serviceName.getFiles(), and once you need that in the template just use files$ | async or in the component files$.pipe(...) to declare other values or files$.subscribe(...). No need to declare files in a function, or assign it in a constructor or ngOnInit.
  2. Subject event such as a button click (fully reactive with RXJS)
    • This one makes the originalFiles itself fully declarative, but does add some more RXJS understanding overhead.
    • A Subject that stands in for the button click event happening. Then when that occurs, the switchMap operator "switches" into the HTTP observable and "maps" its value to the files observable.
    • I don't tend to use subjects as often, but I wanted to show off how you could be very reactive.
    • A much more common scenario that you could apply some of this advice to much more easily: if you were in a scenario where you didn't need a button click or other event to load the files or something else, aka you just want to set a value on load, then it could simply be files$ = this.serviceName.getFiles(), or in example 3, $files = toSignal(this.serviceName.getFiles(). People normally do something like this in a method that is in a constructor or ngOnInit, but they are getting none of the reactive/declarative benefits
  3. toSignal event such as a button click
    • This uses Angular signals which were introduced in Angular 16, and the new control flow (@if instead of *ngIf and whatnot) introduced in v17. The current version of Angular is 18.2, and you could probably upgrade to it at your project's scale if you think it is worth it.
    • Signals are in many, many scenarios much easier to deal with than observables, since signals excel in synchronous values, and RXJS excels in async things like HTTP or events like button clicks.
      • Angular has functions like toSignal to allow events like the getFilesEvent$ and the inner mapping of the HTTP call to be ultimately be handled in a synchronous manner. The src/dst paths don't need to be piped, don't need the async pipe, and otherwise are treated synchronously as something reacting to the changing of the original files. Also, the shareReplay(1) is not needed since once the inner observable computes, the signal value for the files is purely synchronous and anything that depends on it isn't retriggering an observable stream.
      • You are quite likely to be able to do something like $files = toSignal(this.serviceName.getFiles() in most other scenarios. Or if you want to be like scenario one where a button click manually subscribes, do that scenario but rather than .next you .set a signal.
      • RXJS and Signals work well together, but signals may eventually make RXJS in Angular optional. In my opinion, use them to their strong spots and handle them with the toSignal or toObservable as needed.
      • The other values just used for two way binding do not technically need to be signals at the moment, but at some future point, values that change in a component should be made into signals so that Angular can optimally handle change detection.
    • The new control flow replaces *ngIf/*ngSwitch/*ngFor with @if/@switch/@for, and has other benefits such as
      • Notice how in the first two stackblitz examples, I had to use an *<div ngIf="...; else loading">...</div> <ng-template #loading>...</ng-template> to be able to do a mere else. With the new control flow, there is built in @else and @else if (condition). There is similar wins in the other two types as well.
      • The new for loop renders big arrays of things more optimally
@AIxHunter
Copy link
Owner

Hello @michael-small,
That's an immense work, thanks a lot, I will check it asap, and try to apply it to the code.
Btw, don't hesitate to add PR if you want to contribute to the code base.

@michael-small
Copy link
Author

michael-small commented Jan 12, 2025

@AIxHunter starting in Angular 19, there is a much better (but highly experimental) way to load things on demand: resource and rxResource. Same idea, but the later can use RXJS and the former uses promises

https://push-based.io/article/everything-you-need-to-know-about-the-resource-api

I took this following code block from that article, for visual reference

@Component()
export class MyComponent {
  limit = signal(10);

  todosResource = rxResource({
    request: this.limit,
    loader: (limit) => {
      return this.http.get<Todo[]>(
        `https://jsonplaceholder.typicode.com/todos?_limit=${limit}`
      );
    },
  });
}

The resource will call that API with the value of the limit signal (the callback from the loader is technically invoked, so there isn't a need for () to get it as a signal value). And when this.limit changes, todosResource will call the API again. So in this example, that todo list will call out for whatever amount of todos from that API as the limit variable. That value could be changed however and then fire off another call.

TL;DR - a resource can do things like an HTTP get on initial load of a component and then when a signal it depends on changes.

Some more important details about what resources add in value over just a toSignal or just observables

  • Get the value of a resource like myResource.value()
    • You can manually .set(...) a value to override the value it automatically calls out for
    • Can also manually tell the resource to .reload()
  • Resources have different state like myResource.error() state or myResource.loading() state, of if they were idling
    • Has helpers like myResource.hasValue()
  • Can refer to previous values of itself ⭐

Where this ties in

Because these resources can access their previous state to see when they were idling, and because they can manually be called to load, you can make this function

function rxOnDemandResource<T>(
  observableFn: Observable<T>
): ResourceRef<T | undefined> {
  return rxResource({
    loader: (param) => {
      return param.previous.status === ResourceStatus.Idle
        ? of(undefined)
        : observableFn;
    },
  });
}

and use it like this

  response = rxOnDemandResource(
    this.myService.callToAPI(
      this.someSignalA(),
      this.SomeSignalB()
    )
  );

And then get the .value() or .isLoading() or .error() and so forth, all by clicking a button that calls response.reload()

<label for="a">A</label>
<input [(ngModel)]="someSignalA" name="a" />

<label for="b">B</label>
<input [(ngModel)]="someSignalB" name="b" />

<button (click)="response.reload()" type="button">Load</button>

<div>
  <b>"response" resource's states</b>
  <p>isLoading(): {{ response.isLoading() }}</p>
  <p>value(): {{ response.value() }}</p>
  <p>error(): {{ response.error() }}</p>
</div>

or in the TS

someComputed = computed(() => {
    return this.reponse.value() + 1
})

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants