-
Notifications
You must be signed in to change notification settings - Fork 363
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
[cronet_http] šļø Use dart-define to determine dependency #1111
Changes from 11 commits
c8b5ed9
65b92c7
43a59ae
e524a7a
208a647
cabf835
5d525f5
df4033f
32e2801
1339348
13849c2
dec296b
4c000c3
e1df37c
d623b3d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,22 +2,20 @@ | |
[![package publisher](https://img.shields.io/pub/publisher/cronet_http.svg)](https://pub.dev/packages/cronet_http/publisher) | ||
|
||
An Android Flutter plugin that provides access to the | ||
[Cronet][] | ||
HTTP client. | ||
[Cronet][] HTTP client. | ||
|
||
Cronet is available as part of | ||
[Google Play Services][]. | ||
Cronet is available as part of [Google Play Services][] | ||
and as [a standalone embedded library][]. | ||
|
||
This package depends on [Google Play Services][] for its [Cronet][] | ||
implementation. | ||
[`package:cronet_http_embedded`](https://pub.dev/packages/cronet_http_embedded) | ||
is functionally identical to this package but embeds [Cronet][] directly | ||
instead of relying on [Google Play Services][]. | ||
This package depends on [Google Play Services][] | ||
for its [Cronet][] implementation. | ||
To use the embedded version of [Cronet][] without [Google Play Services][], | ||
see [Use embedded Cronet](#use-embedded-cronet). | ||
|
||
## Motivation | ||
|
||
Using [Cronet][], rather than the socket-based [dart:io HttpClient][] | ||
implemententation, has several advantages: | ||
Using [Cronet][], rather than the socket-based | ||
[dart:io HttpClient][] implementation, has several advantages: | ||
|
||
1. It automatically supports Android platform features such as HTTP proxies. | ||
2. It supports configurable caching. | ||
|
@@ -40,22 +38,45 @@ void main() async { | |
final Client httpClient; | ||
if (Platform.isAndroid) { | ||
final engine = CronetEngine.build( | ||
cacheMode: CacheMode.memory, | ||
cacheMaxSize: 2 * 1024 * 1024, | ||
userAgent: 'Book Agent'); | ||
cacheMode: CacheMode.memory, | ||
cacheMaxSize: 2 * 1024 * 1024, | ||
userAgent: 'Book Agent', | ||
); | ||
httpClient = CronetClient.fromCronetEngine(engine); | ||
} else { | ||
httpClient = IOClient(HttpClient()..userAgent = 'Book Agent'); | ||
} | ||
|
||
final response = await client.get(Uri.https( | ||
final response = await client.get( | ||
Uri.https( | ||
'www.googleapis.com', | ||
'/books/v1/volumes', | ||
{'q': 'HTTP', 'maxResults': '40', 'printType': 'books'})); | ||
{'q': 'HTTP', 'maxResults': '40', 'printType': 'books'}, | ||
), | ||
); | ||
} | ||
``` | ||
|
||
### Use embedded Cronet | ||
|
||
The embedded [Cronet][] is provided as `org.chromium.net:cronet-embedded` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe something more explicit like: By default, If you want your application to work without [Google Play Services][], you can instead depend on the For example: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The first paragraph duplicates the previous paragraphs. I'll take the second. |
||
which does not require to use the [Google Play Services][]. | ||
The library uses `dart-define` to switch to the embedded implementation | ||
when `cronetHttpNoPlay` is set to `true`. | ||
For example: | ||
|
||
``` | ||
flutter run --dart-define=cronetHttpNoPlay=true | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The convention seems to be to use name_with_underscore. So maybe: cronet_http_no_play=true There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any guide suggesting this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, I just looked at some existing defines e.g. But it is not consistent so feel free to leave it this way. |
||
``` | ||
|
||
To use the embedded version in `flutter test`: | ||
|
||
``` | ||
flutter test --dart-define=cronetHttpNoPlay=true | ||
``` | ||
|
||
[Cronet]: https://developer.android.com/guide/topics/connectivity/cronet/reference/org/chromium/net/package-summary | ||
[dart:io HttpClient]: https://api.dart.dev/stable/dart-io/HttpClient-class.html | ||
[Google Play Services]: https://developers.google.com/android/guides/overview | ||
[a standalone embedded library]: https://mvnrepository.com/artifact/org.chromium.net/cronet-embedded | ||
[dart:io HttpClient]: https://api.dart.dev/stable/dart-io/HttpClient-class.html | ||
[package:http Client]: https://pub.dev/documentation/http/latest/http/Client-class.html |
This file was deleted.
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There seems to be some random formatting changes in this file...isn't this formatting actually incorrect?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Flutter prefers trailing commas, which makes the formatting different.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This just looks misindented though. Can you change it back?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, the extra space. Would it be accepted if I only took it away?