-
Notifications
You must be signed in to change notification settings - Fork 752
isolates.md no flutter web support #7044
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
base: main
Are you sure you want to change the base?
Conversation
Flutter web no isolates
Summary of ChangesHello @randomizedcoder, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the documentation for Isolates by adding a crucial note regarding their support in Flutter web. The change clarifies that multiple Isolates are not available in Flutter web environments, providing developers with accurate information and a reference to related concurrency documentation, thereby preventing potential confusion and issues. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request adds a note to the isolates.md page to clarify that isolates are not supported on Flutter for web. My review includes suggestions to improve the wording for clarity and correctness, and to align it with the documentation style guide and conventions used in the file.
| :::flutter-note | ||
| If you're using Flutter web, multiple Isolates are not supported. | ||
| See also: [Concurrency on the web](https://dart.dev/language/concurrency#concurrency-on-the-web) | ||
| ::: |
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.
Thanks for adding this note! To improve clarity, correctness, and consistency with the rest of the file and the style guide, I have a few suggestions:
- For consistency with other notes in this file (e.g., line 59), please use five colons (
:::::) to open and close the note block instead of three (:::). - The Google Developer Documentation Style Guide recommends avoiding contractions.1 "you're" should be "you are".
- The phrase "multiple Isolates" could be misleading. On the web, isolates are not supported at all, not just multiple ones. It would be clearer to say "isolates are not supported".
- "Isolates" should be lowercase when referring to the concept, as is done elsewhere in the documentation.
- The link to the "Concurrency on the web" section should be a relative path (
/language/concurrency#...) for consistency with other internal links in this file. - Sentences should end with a period.
Here is a suggestion that applies all these points:
| :::flutter-note | |
| If you're using Flutter web, multiple Isolates are not supported. | |
| See also: [Concurrency on the web](https://dart.dev/language/concurrency#concurrency-on-the-web) | |
| ::: | |
| :::::flutter-note | |
| If you are using Flutter web, isolates are not supported. | |
| See also: [Concurrency on the web](/language/concurrency#concurrency-on-the-web). | |
| ::::: |
Style Guide References
Footnotes
-
The Google Developer Documentation Style Guide recommends avoiding contractions like "you're". Use "you are" instead for a more formal tone and to avoid potential misinterpretation. ↩
antfitch
left a comment
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.
We need to avoid adding information about Flutter in the Dart documentation when at all possible, so this is something I'd like to avoid adding to the Dart docs.
Instead, make sure that this is covered in the Flutter documentation here: https://docs.flutter.dev/perf/isolates. If you think something needs to be updated in those docs, feel free to create a PR in https://github.com/flutter/website/pulls.
|
@antfitch Maybe you know @tirth-patel-nc ? Maybe you guys want to play rock paper scissors? ;) |
|
@randomizedcoder links you posted belonged to this repo hence I commented that. https://docs.flutter.dev/perf/isolates was not there in your original report. |
Hahaha, too funny! We'll get this figured out. @sfshaza2 we have an issue and I'd love to get your advice. We have Flutter-specific information about isolates that needs to be added somewhere. I'm thinking it would make sense to add this in the Flutter documentation? We've been trying to avoid putting Flutter things in the Dart docs since Dart goes beyond Flutter? |
|
Yes, I see two places where we can add it: 1) In the web FAQ and 2) In the Concurrency and isolates page (https://docs.flutter.dev/perf/isolates). But then maybe a note on the Dart site to reference one or both of those pages to learn about Flutter and isolates. |
Flutter web no isolates
Just a small notes into the Isolates page about lack of support in Flutter Web.
flutter/flutter#179083