Make the recommendation to not use `dart:io`/`dart:html` for HTTP requests stronger #5391

brianquinlan · 2023-12-05T01:00:04Z

Part of dart-lang/sdk#52023

I’ve reviewed the contributor guide and applied the relevant portions to this PR.
This PR doesn’t contain automatically generated corrections or text (Grammarly, LLMs, and similar).
This PR follows the Google Developer Documentation Style Guidelines — for example, it doesn’t use i.e. or e.g., and it avoids I and we (first person).
This PR uses semantic line breaks of 80 characters or fewer.

Contribution guidelines:

See our contributor guide for general expectations for PRs.
Larger or significant changes should be discussed in an issue before creating a PR.
Code changes should generally follow the Dart style guide and use dart format.
Updates to code excerpts indicated by <?code-excerpt need to be updated in their source .dart file as well.

brianquinlan · 2023-12-07T02:37:13Z

src/guides/libraries/_dart-html-tour.md

-  // ···
-}
-```
+You should avoid directly using `dart:html` to make HTTP requests.


Let me know if you think that I should just remove this section completely.

brianquinlan · 2023-12-07T02:37:23Z

src/guides/libraries/_dart-io-tour.md

-  httpClient.close();
-}
-```
+You should avoid directly using `dart:io` to make HTTP requests.


Let me know if you think that I should just remove this section completely.

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 /tutorials/server/fetch-data page as an alternative to learn more though since these changes remove learning material.

I do think making the corresponding note/warning on the API's dartdocs more prominent will have a larger effect than any changes here.

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:
https://dart-review.googlesource.com/c/sdk/+/340283

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.

I'm wondering about the severity of this discouragement. Like, if it's a huge problem that people are using HttpRequest / HttpClient, then I'd say completely remove the sections. If it's not so severe, then a better middle ground (for a release or two, like Parker said), would be to keep the old content, but put your new disclaimer at the top of the section before the old content starts. And also pare down all the recommendation-like parts of the old stuff.

I'd say do this for both pages. So here for example, it'd be:

#### HTTP client You should avoid directly using `dart:io` to make HTTP requests.... (all your new content up to recommending the fetch-data tutorial) If you still need to use `HttpClient`, here's an example: (code block)

If you want to go this route, let me know and I can leave another comment on the other section (dart:html) with a similar mini-rough draft of: starting with your discouragement text + how I'd pare it down severely.

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?

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:

Though no longer recommended / not recommended for the majority of use cases, you can find more information on / an example of `HttpClient`/`HttpRequest` in the [API documentation](link-to-api-docs).

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!

Isn't the purpose the library tour to indoctrinate developers new to Dart? Those developers should start with package:http (or some equivalent package).

I preserved the link to the class and it still has an example there.

Oh, and don't trust my judgement; if often sucks ;-)

Isn't the purpose the library tour to indoctrinate developers new to Dart?

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!

Thanks for the detailed answer. I think that I'm OK with it as-is.

MaryaBelanger

I left some initial comments, but have some questions too. There are a few other mentions of using HttpRequest elsewhere on the library tour page (like in code blocks in the Future section, and the HTTP server section under dart:io). Do those all need to be changed to use package:http, or is it not interchangeable like that?

src/guides/libraries/_dart-html-tour.md

src/guides/libraries/_dart-io-tour.md

src/tutorials/server/fetch-data.md

src/guides/libraries/_dart-html-tour.md

src/guides/libraries/_dart-io-tour.md

src/guides/libraries/_dart-html-tour.md

Co-authored-by: Marya <[email protected]>

brianquinlan · 2023-12-09T02:25:30Z

I left some initial comments, but have some questions too. There are a few other mentions of using HttpRequest elsewhere on the library tour page (like in code blocks in the Future section, and the HTTP server section under dart:io). Do those all need to be changed to use package:http, or is it not interchangeable like that?

I updated the Future example. The HTTPServer example is fine.

brianquinlan · 2023-12-11T22:33:48Z

Are the check failures related to my code? I don't know how to interpret the results.
I don't have permission to merge this.

parlough · 2023-12-11T22:44:58Z

Are the check failures related to my code? I don't know how to interpret the results.

We pull code snippets from source files so they can be analyzed and tested. So the two you edited needed to be updated at the source. I did this for you in 22437f2 :)

Thanks for working on this :D

brianquinlan · 2023-12-11T22:45:30Z

Are the check failures related to my code? I don't know how to interpret the results.

We pull code snippets from source files so they can be analyzed and tested. So the two you edited needed to be updated at the source. I did this for you in 22437f2 :)

Thanks for working on this :D

Thanks for fixing it for me!

brianquinlan · 2023-12-11T23:53:36Z

Could someone press the merge button for me?

parlough

Thank you!!

…uests stronger (dart-lang#5391) Part of dart-lang/sdk#52023 --------- Co-authored-by: Marya <[email protected]> Co-authored-by: Parker Lougheed <[email protected]>

brianquinlan added 4 commits October 30, 2023 10:05

Add a note recommending package:http over dart:io/dart:html

336c402

Update fetch-data.md

b216726

Update fetch-data.md

8036ab1

Update fetch-data.md

a962d8b

brianquinlan changed the title ~~Make the recommendation to not use dart:io/dart:html for HTTP requests strong~~ Make the recommendation to not use dart:io/dart:html for HTTP requests stronger Dec 5, 2023

brianquinlan requested a review from MaryaBelanger December 5, 2023 01:00

parlough assigned MaryaBelanger Dec 7, 2023

parlough added the review.copy Awaiting Copy Review label Dec 7, 2023

Library tour

1b7474a

brianquinlan commented Dec 7, 2023

View reviewed changes

Add link to tutorial

8354730

MaryaBelanger reviewed Dec 7, 2023

View reviewed changes

brianquinlan and others added 9 commits December 7, 2023 14:37

Update src/guides/libraries/_dart-html-tour.md

4ee863a

Co-authored-by: Marya <[email protected]>

Update src/guides/libraries/_dart-io-tour.md

966e79c

Co-authored-by: Marya <[email protected]>

Update src/tutorials/server/fetch-data.md

5f4e670

Co-authored-by: Marya <[email protected]>

Update src/guides/libraries/_dart-io-tour.md

d16ccf4

Co-authored-by: Marya <[email protected]>

Update src/guides/libraries/_dart-html-tour.md

5db23d5

Co-authored-by: Marya <[email protected]>

Update src/guides/libraries/_dart-html-tour.md

6319db5

Co-authored-by: Marya <[email protected]>

Merge branch 'main' into client-updates

7b89f76

Update src/guides/libraries/_dart-io-tour.md

2879d6d

Co-authored-by: Marya <[email protected]>

Remove HttpRequest example

62102b8

brianquinlan added 2 commits December 8, 2023 18:29

Remove package dio reference

bb762ac

Merge branch 'main' into client-updates

171c967

MaryaBelanger approved these changes Dec 11, 2023

View reviewed changes

parlough added 2 commits December 11, 2023 16:42

Update source of excerpt for use of package:http

22437f2

Use site variables

5c2e924

Ooops, fix link

11775f3

parlough approved these changes Dec 11, 2023

View reviewed changes

parlough merged commit 507072c into dart-lang:main Dec 11, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make the recommendation to not use `dart:io`/`dart:html` for HTTP requests stronger #5391

Make the recommendation to not use `dart:io`/`dart:html` for HTTP requests stronger #5391

brianquinlan commented Dec 5, 2023

brianquinlan Dec 7, 2023

brianquinlan Dec 7, 2023

parlough Dec 7, 2023

brianquinlan Dec 7, 2023

MaryaBelanger Dec 7, 2023

brianquinlan Dec 9, 2023

MaryaBelanger Dec 11, 2023

brianquinlan Dec 11, 2023

brianquinlan Dec 11, 2023

MaryaBelanger Dec 11, 2023

brianquinlan Dec 11, 2023

MaryaBelanger left a comment

brianquinlan commented Dec 9, 2023

brianquinlan commented Dec 11, 2023

parlough commented Dec 11, 2023

brianquinlan commented Dec 11, 2023

brianquinlan commented Dec 11, 2023

parlough left a comment

Make the recommendation to not use dart:io/dart:html for HTTP requests stronger #5391

Make the recommendation to not use dart:io/dart:html for HTTP requests stronger #5391

Conversation

brianquinlan commented Dec 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaryaBelanger left a comment

Choose a reason for hiding this comment

brianquinlan commented Dec 9, 2023

brianquinlan commented Dec 11, 2023

parlough commented Dec 11, 2023

brianquinlan commented Dec 11, 2023

brianquinlan commented Dec 11, 2023

parlough left a comment

Choose a reason for hiding this comment

Make the recommendation to not use `dart:io`/`dart:html` for HTTP requests stronger #5391

Make the recommendation to not use `dart:io`/`dart:html` for HTTP requests stronger #5391