unsuitable001 / dart_cronet_sample

[SELECTED] Sample project for GSoC '21 - Cronet based HTTP package
https://github.com/google/cronet.dart
MIT License
2 stars 1 forks source link

Dart Standalone and Flutter support in a single package #7

Closed unsuitable001 closed 3 years ago

unsuitable001 commented 3 years ago

Currently, in this repo only contains code that are solely dependent on Dart SDK. But, when we add support for mobile devices, we need access to Flutter SDK too.

I've already added Android support in another repo that depends on this repo. But, managing two different repo can be cumbersome for devs and users both.

So, I want to make this package as such that it can act as a Dart Standalone and a Flutter package both. That means, someone can use pub get and flutter pub get both. i.e making the Flutter SDK optional if someone don't want flutter support.

I've looked into webcrypto as it works both as standalone and flutter package. But I have to do flutter pub get. Just pub get won't work. So, though someone is able to use it without actually using Flutter, still they need Flutter SDK setup in their system.

Is there any solution to this? (Like, conditional dependency :thinking: ?)

Ref: https://github.com/unsuitable001/dart_cronet_sample/issues/3

CC @mannprerak2 @dcharkes

mannprerak2 commented 3 years ago

How about mono repos? we can have 2 separate packages in the same repo. Something similar to https://github.com/rrousselGit/river_pod

unsuitable001 commented 3 years ago

That looks interesting to me. I even found some tools regarding dart monorepo & I wasn't aware that even Google is using it.

Just one problem remains. Users have to be aware that there are 2 packages. One with flutter and one without.

If this isn't much of a problem, mono repo will be a better choice than current one 😊

unsuitable001 commented 3 years ago

TL;DR. Can we do pub publish from a monorepo directly? Without any refactoring? What are the best practices? Am I allowed to use non-google packages like multipack or melos?

One more thing. The source code for wrapper is exactly the same for dart & flutter package. So, we can keep one copy in dart package and use symlink from flutter package, right? (git & pub publish both allows that?)

If this isn't allowed, another way can be: The same trick we used when dart package's wrapper source code wasn't available. #4 As the flutter package will be dependent on dart one, we can find the dart package and copy it to our desired location in users' machine.

Edit: And, Flutter package's dependency on Dart package will be specified by relative path dependency. (We can use git/pub dependency too.. but.. to be self contained shouldn't it use relative paths?) Will not it be a problem when doing pub publish?

mannprerak2 commented 3 years ago

Seems like a lot of effort, perhaps we should simply follow package:webcrypto's approach.

dcharkes commented 3 years ago

Yes, please use package:webcrypto's approach for now. I'll try to come up with a general solution for this for all packages that want to be both Flutter and Dart-standalone compatible.

unsuitable001 commented 3 years ago

Ok. Then let's do that.

unsuitable001 commented 3 years ago

I've implemented it like webcrypto here

I've developed a (sort of) hacky way to make a hybrid package that supports both Dart Standalone & Flutter without creating 2 package or monorepo. Check it out at hybrid branch and let me know if we can use this approach.

@dcharkes @mannprerak2

mannprerak2 commented 3 years ago

I think we should be able to remove the flutter pub run cronet_sample <platform> step, atleast for android/iOS, similar to webcrypto.

unsuitable001 commented 3 years ago

I just didn't wanted to commit binary files to source control. Otherwise, we can do that. flutter pub run cronet_sample <platform> only downloads required binaries on demand.

Edit: on my another repo, I committed them to git. I can adapt to both of them as you, mentors and maintainers, suggest.

dcharkes commented 3 years ago

I've developed a (sort of) hacky way to make a hybrid package that supports both Dart Standalone & Flutter without creating 2 package or monorepo. Check it out at hybrid branch and let me know if we can use this approach.

You should be able to remove:

https://github.com/unsuitable001/dart_cronet_sample/blob/8f2c921ccdef83b4db2129e66371b86a804de7f1/lib/cronet_sample.dart#L7-L15

Then you can also remove the import to Flutter.

I just didn't wanted to commit binary files to source control. Otherwise, we can do that. flutter pub run cronet_sample <platform> only downloads required binaries on demand.

Edit: on my another repo, I committed them to git. I can adapt to both of them as you, mentors and maintainers, suggest.

You can use .pubignore to not upload them to git but upload them when you do pub publish. Ignore the files in .gitignore but dont in .pubignore. https://github.com/dart-lang/pub/pull/2787

So, I want to make this package as such that it can act as a Dart Standalone and a Flutter package both. That means, someone can use pub get and flutter pub get both. i.e making the Flutter SDK optional if someone don't want flutter support.

dart pub get and flutter pub get should both work with the package:webcrypto approach. These work both from the commandline for me.

Note that you indeed need to remove the flutter dependency for dart pub get to work in VSCode. (I'm unsure why it works on command line but not in VSCode without removing it.)

dependencies:
#  flutter:
#    sdk: flutter

Error in VSCode if those lines are not removed:

[dart_app] dart pub get
Resolving dependencies...
Because every version of mylib_source from path depends on flutter any from sdk which is forbidden, mylib_source from path is forbidden.
So, because dart_app depends on mylib_source from path, version solving failed.

Flutter users should run `flutter pub get` instead of `pub get`.
exit code 69
unsuitable001 commented 3 years ago

You should be able to remove:

https://github.com/unsuitable001/dart_cronet_sample/blob/8f2c921ccdef83b4db2129e66371b86a804de7f1/lib/cronet_sample.dart#L7-L15

Then you can also remove the import to Flutter.

Just did that now! It's working perfectly! See patch. I think, it is a better approach and makes hybrid branch obsolete.

You can use .pubignore to not upload them to git but upload them when you do pub publish. Ignore the files in .gitignore but dont in .pubignore. dart-lang/pub#2787

Added! I just did that for android only. Should we also do that for desktops also?

dart pub get and flutter pub get should both work with the package:webcrypto approach. These work both from the commandline for me.

Yup! Now it works with this patch.

Note that you indeed need to remove the flutter dependency for dart pub get to work in VSCode. (I'm unsure why it works on command line but not in VSCode without removing it.)

dependencies:
#  flutter:
#    sdk: flutter

Error in VSCode if those lines are not removed:

[dart_app] dart pub get
Resolving dependencies...
Because every version of mylib_source from path depends on flutter any from sdk which is forbidden, mylib_source from path is forbidden.
So, because dart_app depends on mylib_source from path, version solving failed.

Flutter users should run `flutter pub get` instead of `pub get`.
exit code 69

Actually, I faced the same issue. I've also thought, if we're giving flutter specific info in pubspec (i.e. about plugin platforms), how can we not add flutter as a dependency. That's why I haven't tried this patch and created the hacky way on hybrid branch instead. (That's my fault. Should've at least tried once). package:webcrypto itself doesn't work for me without flutter on CLI and VSCode both. (And, it uses Flutter as dependency).

But, feature_flutter branch in this repo works on both VSCode and CLI with and without Flutter :smile: