microsoftgraph / msgraph-sdk-javascript

Microsoft Graph client library for JavaScript
https://graph.microsoft.com
MIT License
747 stars 225 forks source link

Separate library utilizing RxJS #665

Open Jonas-Seiler-Wave opened 2 years ago

Jonas-Seiler-Wave commented 2 years ago

Feature Request

Is your feature request related to a problem? Please describe

When using Angular the prescribed method, which I also prefer to use, to handle HTTP request is using RxJS observables instead of promises. Moreover, in my personal inexperienced opinion, the use of promises is harder to reason about and almost archaic compared to the use of observables, even outside of Angular. As such, I find myself actually avoiding this library.

Describe the solution you'd like

I would like there to be an official JS/TS Graph SDK library that utilizes RxJS observables. As for whether it supersedes this existing library or becomes a new separate one, I have no preference.

Describe alternatives you've considered

I have considered simply conceding to using this library, since there might be very good technical reasons as for why promises are used, or rather observables aren't used, that I may not fully comprehend yet, but then I thought making a suggestion couldn't hurt.

Additional context

I am unsure whether this even is the correct place to suggest such a thing, but it seemed the most appropriate to me. I also couldn't find any similar suggestions, so I hope this isn't like the 1000th time someone annoys you with this.

nikithauc commented 2 years ago

Thank you @Jonas-Seiler-Wave for bringing forward this request! You have provided some valuable feedback here.

This will request will be be looked into.

sebastienlevert commented 2 years ago

Thanks for this request @Jonas-Seiler-Wave! I would love to learn more about what you are expecting.

When you are working on Angular projects, I would assume other lu taries are also providing rxjs-specific implementations. Do you have a few examples?

Also, is it that using the SDK is not possible at all in your situation or it would require some gymnastics to make it work?

Are you aware how others might attack this challenge?

Thanks!

Jonas-Seiler-Wave commented 2 years ago

I have not really thought about it further than that I would like to receive the data from HTTP requests as single-emit observables as is the case with Angular's HttpClient, although there are likely also situations where multi-emit observables make a lot of sense.

My single most used library in my current Angular projects is MSAL, of which the MSAL-Angular implementation uses RxJS natively to expose authentication events, but of course handling events and retrieving data over HTTP are a bit different.

Using the SDK certainly is possible, but it comes at the cost of more convenience than it provides for me. As I've said, I much prefer working with and thinking in terms of observables/streams over promises. Having to handle things like batching, paging and ensuring the requests are syntactically correct, while not ideal, is something I like more than to introduce an additional different mechanism for asynchronism.

As for what others do, I would have to guess, but since the issues I have have more to do with personal preference than with actual technical concerns, I would assume many others have no problem using this library in Angular or rather in conjunction with RxJS. I'm also thinking that with some effort one should be able to build correctly functioning observables from these promises, but I am not confident that I could achieve this without introducing a great potential for hard to locate bugs.

Anyways, thanks for the quick response and taking some time for this!

Jonas-Seiler-Wave commented 2 years ago

It just occurred to me that this may need more elaboration due to me haphazardly lumping some terms together. That is I used observable and stream interchangeably and used promise to refer to the promise itself, the attached callbacks and to the method in which the promise is created in the first place.

So more specifically my problem with the current implementation is that it does not in a simple way allow me to reason about HTTP requests as functions returning very specific streams of discrete data points which I can predefine and execute on demand. This is also why simply creating observables from the returned promises isn't a satisfactory workaround.

sebastienlevert commented 2 years ago

Thanks for the details @Jonas-Seiler-Wave! We really appreciate it! This is not a priority for now compared to some of the currently in progress work, but it's something we'll continue to listen to. Feel free to reply to that thread with new data or scenarios, it's useful. For now, I would argue the best approach will be to wrap the promises within a RxJS Observable (to ensure your code works in a comparable way with Graph) even if this is not fully native.

We will be keeping this issue open as a feature request and if we need more details, we'll reply here! Thanks!

snebjorn commented 2 years ago

The requests from Graph SDK can fairly simple be turned into RxJS using defer

public getMe() {
  const request = this.graphClient
    .api('/me')
    .responseType(ResponseType.JSON);

  return defer(() => request.get());
}

It would be much more valuable if a separate package used HttpClient from @angular/common/http to make requests.

That would use RxJS but also allow us to rely on the MSAL setup done for @azure/msal-angular. Making (graph) Client initialization a lot simpler. There would be no "setup" as we could use the already setup MsalIntercepter. All we'd need to do was add the scopes to our protectedResourceMap.

// MsalInterceptorConfiguration
{
    protectedResourceMap: new Map([
        ["https://graph.microsoft.com/v1.0/*", ["user.read", "profile"]],
    ]),
}

To my knowledge @microsoft/microsoft-graph-client is incompatible with MsalIntercepter from @azure/msal-angular. Today we even have to perform odd tricks (#505) in order to work with @microsoft/microsoft-graph-client inside Angular.

Jonas-Seiler-Wave commented 2 years ago

Good point, @snebjorn, using the Angular HttpClient would be make the library quite a bit more convenient to use in Angular, and I'm guessing it would be simpler to write in the first place as well. Thinking about it again, that would be the preferred solution for my case. There might also be only a limited case for the SDK to facilitate working with observables outside of an Angular environment in the first place, since as far as I'm aware of, no other framework has native RxJS integration. This is something the team will have to decide when they get around to this issue.

I am not sure what you mean by this though:

The requests from Graph SDK can fairly simple be turned into RxJS using defer

Where does the defer function in your code example come from and what does it do? I have so far only encountered defer as an HTML attribute, and that is also the only meaning related to Javascript I could find in a web search. Could you point me in the right direction?

snebjorn commented 2 years ago

Where does the defer function in your code example come from and what does it do? I have so far only encountered defer as an HTML attribute, and that is also the only meaning related to Javascript I could find in a web search. Could you point me in the right direction?

Absolutely, defer is from RxJS. The reason I use defer is because I don't want the Graph SDK reqeust to be executed before I actually subscribe to the Observable. It would be used like so:

const me$ = myGraphService.getMe();
me$.subscribe(x => {
   // do stuff
});
sebastienlevert commented 2 years ago

Thanks for the suggestion! It's a great one and I believe delivering this level of integration with Angular would be valuable. Thanks for raising this one! Once we are done with our v4 of the Graph SDK, it's something I'd love to immerse myself in to understand how we can deliver more value for Angular developers!

Adding @DanWahlin to this conversation for awareness.