boskokg
[Feature]: RxDart version should not leak into public facing API
FlutterBluePlus Version
1.35.X
What is your feature request?
Currently using FBP requires rxdart: '>=0.27.0 <0.29.0'
This effectively becomes part of the API of FBP, limiting users a specific range of versions for RxDart in order to use FBP.
This is annoying, and we should find a way to improve this situation.
We need to either:
- remove rxdart as a dependency
- copy the rxdart souce into FBP-linux
- find some other way to encapsulate this
@tnc1997
we barely use this dependency. is there some other way to do what you are using it for?
limiting users a specific range of versions for RxDart
0.27.0 was released on 7 May 2021 and 0.28.0 is the latest version therefore unless someone is using an older version this is no problem.
this is annoying
Out of interest have any developers raised the version constraint as an issue?
copy the rxdart souce into FBP-linux
I would recommend against this personally as it increases maintenance overhead and we would need our own unit tests to validate the logic.
Out of interest have any developers raised the version constraint as an issue?
yes. here are some examples.
https://github.com/espresso3389/pdfrx/issues/211 https://github.com/fluttercommunity/flutter_google_places/issues/153 https://github.com/invertase/dart_custom_lint/issues/256 https://github.com/fluttercommunity/rx_command/issues/56 https://github.com/zino-hofmann/graphql-flutter/issues/1441 https://github.com/supabase/supabase-flutter/issues/997 https://github.com/fluttercommunity/flutter_google_places/issues/26 https://github.com/bosskmk/pluto_grid/issues/252 https://github.com/Baseflow/flutter_cached_network_image/issues/603 https://github.com/florent37/Flutter-AssetsAudioPlayer/issues/456 https://github.com/zino-hofmann/graphql-flutter/issues/943 https://github.com/DarshanGowda0/GeoFlutterFire/issues/102
It should be clear that by including RxDart as a dependency we are opening ourselves up to more issues down the line.
I would recommend against this personally as it increases maintenance overhead and we would need our own unit tests to validate the logic.
how would it increase maintenance? As long as the code works, we don't need to "maintain" it. We could just stay on a known working version forever. Did you have something else in mind?
To be clear, I would most strongly advocate for this option:
remove rxdart as a dependency
A small utility function should be able to replace the need for it, reducing maintenance, and simplifying the public API.
internal code decisions should not be part of the public facing API whenever possible. it's just bad design, and the dozens of github issues about this design choice prove the point imo.
yes. here are some examples.
I meant developers raising issues with flutter_blue_plus specifically.
internal code decisions should not be part of the public facing API whenever possible.
None of the internals of rxdart are exposed via the public facing API.
Im using API in a more general sense. It's public facing during version resolution. And theres no reason to think FBP would be immune to these issues. Why make things harder for our users?
Why do you want to keep RxDart?
We already have a merge streams function in the FBP utils. Do we really need another one?
Why do you want to keep RxDart?
Because many of its classes and functions are used in the Linux implementation and it avoids reinventing the wheel.
hmm.
I only see three uses?
- MergeStream (which we already have defined elsewhere)
- startWith (which we already have defined elsewhere)
- switchMap
is there something else im missing?
And here is swtichMap in vanilla dart. Do we really need a whole big library for this one function?
extension SwitchMapExtension<T> on Stream<T> {
/// For each event from the source stream, cancels any previous inner
/// subscription and listens to the stream returned by [mapper].
/// Completes only after the source has finished *and* the current inner
/// stream (if any) has finished.
Stream<S> switchMap<S>(Stream<S> Function(T value) mapper) {
// Controller for the output
final controller = StreamController<S>();
StreamSubscription<T>? outerSub;
StreamSubscription<S>? innerSub;
var outerDone = false;
// Helper to close once both outer is done and no inner is active
void tryClose() {
if (outerDone && innerSub == null) {
controller.close();
}
}
controller
// Start listening to source when someone listens to the result
..onListen = () {
outerSub = this.listen(
(value) {
// Cancel previous inner
innerSub?.cancel();
// Listen to the new inner stream
innerSub = mapper(value).listen(
controller.add,
onError: controller.addError,
onDone: () {
innerSub = null;
tryClose();
},
);
},
onError: controller.addError,
onDone: () {
outerDone = true;
tryClose();
},
);
}
// Propagate pause/resume to both subscriptions
..onPause = () {
outerSub?.pause();
innerSub?.pause();
}
..onResume = () {
outerSub?.resume();
innerSub?.resume();
}
// Cancel both if the listener cancels
..onCancel = () async {
await outerSub?.cancel();
await innerSub?.cancel();
};
return controller.stream;
}
}
I believe that it is better to reuse common well-tested implementations than reinvent the wheel and bundle your own copy/pasted equivalents, but of course you are welcome to remove the dependency and maintain your own if you would prefer. There are pros and cons to both approaches some of which are discussed in this thread and this thread.
I believe that it is better to reuse common well-tested implementations than reinvent the wheel and bundle your own copy/pasted equivalents
Why? Copy pasting takes two seconds. Adding a dependency limits who can use FBP. Version resolution is not a simple task in a big app. Thats a huge tradeoff to save two seconds.
Personally, I'd rather remove the linux support than leak RxDart (which impacts all platforms even non-linux apps). If RxDart was not popular it might be okay. But RxDart is very popular and can be hard to resolve. This is why there are so many Github issues about it.
Sure, right now RxDart is stable. But it's a ticking time bomb, and even if we update FBP to support the newer RxDart now users will be stuck on the latest FBP version. These are not small tradeoffs.
if we update FBP to support the newer RxDart now users will be stuck on the latest version
It is worth noting that our version constraint means that developers can use any version since 0.27.0 (7 May 2021).
This means that developers are not stuck with the latest version of rxdart and can use any compatible version.
- new RxDart 30.0 comes out
- other package requires 30.0 for whatever reason
- FBP does not support for 30.0
- we update FBP to support it (hopefully quickly! but honestly i like to take vacations sometimes).
- user cannot update until we do, and is now stuck on latest FBP or must downgrade other packages too
making users go through this hassle for a single util function is not worth it. It's crazy to me.
5. user is now stuck on latest FBP
If other packages are being updated to support 30.0 then including FBP in those is not unreasonable in my opinion.
not unreasonable to be forced to upgrade because of a single util function we didnt want to spend 2 seconds copy pasting?
RxDart is very popular
The popularity of RxDart as a well maintained library is one of the reasons to use it instead of copy/pasting its internals.
we didnt want to spend 2 seconds copy pasting
Like I have previously said above you are welcome to remove the dependency and maintain your own implementations.
Just to add to the discussion. Users do worry about dependencies.
Rust’s dependencies are starting to worry me - https://news.ycombinator.com/item?id=43935067
My goal when developing is to make mature packages (I'd rather call them finished). I.e. code that works "forever" and doesn't require frequent upkeep. Like this comment talks about. It's really hard to make a finished package with a lot of dependencies.
you are welcome to remove the dependency and maintain your own implementations
I think this shows a difference in how we look at. I've don't think of self contained util functions as something that needs "maintenance". Sure you can apply that word, and in theory they could need it. But practically speaking, self contained util functions will never need to be touched if you write them in an maintenance free way. (i.e. using basic language features & zero dependencies).