-
Notifications
You must be signed in to change notification settings - Fork 716
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
Make the recommendation to not use dart:io
/dart:html
for HTTP requests stronger
#5391
Changes from all commits
336c402
b216726
8036ab1
a962d8b
1b7474a
8354730
4ee863a
966e79c
5f4e670
d16ccf4
5db23d5
6319db5
7b89f76
2879d6d
62102b8
bb762ac
171c967
22437f2
5c2e924
11775f3
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 |
---|---|---|
|
@@ -234,27 +234,15 @@ void processRequest(HttpRequest request) { | |
|
||
#### HTTP client | ||
|
||
The [HttpClient][] class | ||
helps you connect to HTTP resources from your Dart command-line or | ||
server-side application. You can set headers, use HTTP methods, and read | ||
and write data. The HttpClient class does not work in browser-based | ||
apps. When programming in the browser, use the | ||
[dart:html HttpRequest class.][HttpRequest] | ||
Here's an example of using HttpClient: | ||
|
||
<?code-excerpt "misc/test/library_tour/io_test.dart (client)" replace="/Future<\w+\W/void/g"?> | ||
```dart | ||
void main() async { | ||
var url = Uri.parse('http://localhost:8888/dart'); | ||
var httpClient = HttpClient(); | ||
var request = await httpClient.getUrl(url); | ||
var response = await request.close(); | ||
var data = await utf8.decoder.bind(response).toList(); | ||
print('Response ${response.statusCode}: $data'); | ||
httpClient.close(); | ||
} | ||
``` | ||
You should avoid directly using `dart:io` to make HTTP requests. | ||
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. Let me know if you think that I should just remove this section completely. 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. I think it's good to keep these sections for two releases or so to point users in the right direction. Curious what Marya thinks though. In the sections you remove, do consider linking to the I do think making the corresponding note/warning on the API's dartdocs more prominent will have a larger effect than any changes here. 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. Great suggestion! I've added a link to the tutorial. There is a code review out for making the corresponding change to the API docs: 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.
I'm wondering about the severity of this discouragement. Like, if it's a huge problem that people are using I'd say do this for both pages. So here for example, it'd be:
If you want to go this route, let me know and I can leave another comment on the other section ( 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. We want to be pretty forceful about our recommendation - we are using the same initial wording in the SDK documentation. But we are preserving the example so users can find it there. Wouldn't it be better to avoid having examples in our library tour that should not be used except in rare circumstances? 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. Thanks for explaining, definitely trust your judgement here! You can make the final call here, but just to be extra explicit for readers, you could explicitly state that there's still an example in the sdk docs (that's the API docs right?) if you really need it (like Parker said, having an example in one place then making it disappear too well all of a sudden isn't ideal). So, after lines 245 in this file, and 333 at the end of the html-tour file, something like:
Approving in the mean time so you can make the final call, but I'll come back and check again if you do change something else and want another look. Thanks for doing 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. Isn't the purpose the library tour to indoctrinate developers new to Dart? Those developers should start with I preserved the link to the class and it still has an example there. 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. Oh, and don't trust my judgement; if often sucks ;-) 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.
Definitely so. What Parker and I are referring to is the kind of nebulous, hard to measure, off-chance that "what if someone, somewhere has a link to this specific section that they hold onto for reference that says "example here for whatever", and the off-chance that they'll be greatly perturbed if all of a sudden that "whatever"s not there any more and it's not immediately obvious where to find it. This definitely isn't a big deal so again, your call :) And your judgement about how important it is that no one remotely even goes poking around the old stuff unless they're absolutely digging for it is based on a lot more context than I have, I'm just citing a general best practice for tech writing. And yes, you do have the link on the API names which I'm ok with! 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. Thanks for the detailed answer. I think that I'm OK with it as-is. |
||
The [HttpClient][] class in `dart:io` is platform-dependent | ||
and tied to a single implementation. | ||
Instead, use a higher-level library like | ||
[`package:http`]({{site.pub-pkg}}/http). | ||
|
||
The [Fetch data from the internet][] tutorial | ||
explains how to make HTTP requests | ||
using `package:http`. | ||
|
||
### More information | ||
|
||
|
@@ -269,6 +257,7 @@ For more information about server-side and command-line app development, see the | |
[library tour]: /guides/libraries/library-tour | ||
[dart:io]: {{site.dart-api}}/{{site.data.pkg-vers.SDK.channel}}/dart-io/dart-io-library.html | ||
[Directory]: {{site.dart-api}}/{{site.data.pkg-vers.SDK.channel}}/dart-io/Directory-class.html | ||
[Fetch data from the internet]: /tutorials/server/fetch-data | ||
[File]: {{site.dart-api}}/{{site.data.pkg-vers.SDK.channel}}/dart-io/File-class.html | ||
[HttpClient]: {{site.dart-api}}/{{site.data.pkg-vers.SDK.channel}}/dart-io/HttpClient-class.html | ||
[HttpRequest]: {{site.dart-api}}/{{site.data.pkg-vers.SDK.channel}}/dart-html/HttpRequest-class.html | ||
|
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.
Let me know if you think that I should just remove this section completely.