-
Notifications
You must be signed in to change notification settings - Fork 82
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
RFC: Minimise external dependencies #88
Comments
I will have a look at this. |
So what I found out so far: I think we can replace dio with http, but I would go a step further and move the http calls to its own client for the web api, so this will be usable in the future if we want to extend the functionality of the sdk to provide web api connections-> using web api calls from android or ios. Regarding this @itsMatoosh are we currently using Web Playback SDK or the web api? Further on minimising package use: I think we can probably remove logger and write our own logging implementation. |
@fotiDim We could definitely remove dio and make use of http instead. @brim-borium In the web implementation we are using both the Web Playback SDK and the Web API. The playback SDK only allows for creating a new player in the browser, but doesn't provide functions such as play(), pause() etc. So the player is created using the Web Playback SDK and controlled using the Web API. Regarding the libraries used in the web implementation, they are all needed. JS makes it possible to call the Web Player SDK js API from dart. Crypto is used for generating a code challenge used for authentication. Synchronized is used to ensure that each call to getAuthenticationToken() waits until the last call to that method is finished, this has to do with how the Web SDK handles reauthentication. |
@itsMatoosh regarding the Web Playback SDK according to this page it does support Regarding |
@fotiDim Yes, these methods are implemented using the Web Playback SDK already, however methods such as play(Track) are not available on the Web Playback SDK and therefore I had to use the Web API for that. The stream approach to make a queue of getAuthenticationToken requests seems interesting. I suppose we could implement it by using a StreamController that accepts some kind of a callback function and then having the getAuthenticationToken method would loop over all the token requests and resolve them one by one. The problem is, I think that might be less efficient than just using synchronized. |
Loosely relevant topic, it seems we have some overlap with the spotify-dart package in the web implementation. For example authentication or some web API methods that we use like the play endpoint. Are there any parts that interest us that we could potentially adopt? |
updates? |
This is a request for comments. It seems to me that this library depends on too many external dependencies. In general there is little wrong with that but it can lead to some issues:
If this was an app I would be totally fine with any amount of dependencies but since this is a library my ideal amount of dependencies would be 0.
I was about to suggest to use the
dart:io
library instead ofDio
but by reading further I realised thatdart:io
does not support Flutter web. The official documentation suggests the http package instead. Is there any benefit of using Dio? I would assume that the majority of apps are using the http package so migrating to thehttp
package would mean one dependency less for most apps. Unfortunately I do not have any data to back up this claim but my gut feeling says thathttp
is the most popular choice.Are there any other packages we can get rid of?
The text was updated successfully, but these errors were encountered: