-
Notifications
You must be signed in to change notification settings - Fork 89
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
Unify package's Browser and IO exports, fix invalid SDK error #34
Conversation
I too have noticed that this library system has matured a little more, not needing seperate files anymore. I like this approach a lot better, since it also was a hassle at time of development. Thanks for looking into it!! Regarding the breaking changes - I'd rather have a smoother upgrade path than just chucking the "two old files". Can you please check if it's possible to annotate the |
@rinukkusu That was a good call! I was able to deprecate the original libraries like you mentioned. If you try to import the old libraries now, VS Code strikes them out and gives you an informative warning 😃 When importing the old libraries, the package's models and methods were still recognized, so I think the whole "library exporting another library" thing worked. |
We'll also need to update the README with these changes once the new version is ready to be released. |
Thank you for making this package a better experience. I'll do the housekeeping later 😄 |
|
That's awesome!!! I really appreciate the quick turnaround time. Out of curiosity, did you test these changes against native Dart IO and Browser apps before releasing? I probably should've mentioned that I've only tested this branch against my Flutter app 😬. |
https://pub.dev/packages/spotify#-analysis-tab- looks much nicer now 😄. |
If you scroll down to the
Maintenance issues and suggestions
section of this package's Analysis tab on pub.dev, you can see that there's aNo valid SDK
error. This is a big hit to the package's overall quality score and it causes the package's platform to show as[UNIDENTIFIED]
on pub.dev.In order for a package to be correctly identified as a Dart library, it needs to have a
lib/package_name.dart
file (lib/spotify.dart
in this case). This package is running into theNo valid SDK
error because it doesn't have that file, but instead has two top-level files:lib/spotify_io.dart
andlib/spotify_browser.dart
.This PR removes the
spotify_io.dart
andspotify_browser.dart
files and replaces them with a newspotify.dart
. There are two reasons that it's now possible to unify them into a single file:spotify
library to import different files based on whether it detects the presence ofdart.library.io
ordart.library.browser
http.Client()
is smart and creates either anIOClient
or aBrowserClient
based on which Dart libraries are available (from what I can tell, explicitly creating different types of clients per platform was one of the primary reasons these files were split up previously)According to a local run of pana, the tool used to generate package quality scores on pub.dev, these changes improve the package's
Maintenance
score from 70 to 100.This will be a breaking change.
If we'd like to make it a non-breaking change, we could keep the two old files around and simply have them export
spotify.dart
. This would be confusing to newcomers though, as they'd see three possible files to import, which would all do the same thing.