Conversation
There was a problem hiding this comment.
Code Review
This pull request provides several documentation updates, primarily focused on cleaning up wording, improving formatting for readability, and updating guidance for web development. Key changes include replacing 'see' with 'visit' or 'check out' for external links, reformatting long lines, and adding information about the Jaspr package for building static websites with Dart. My review found a minor grammatical issue where a semicolon was used incorrectly, and I've suggested a fix. Overall, the changes improve the clarity and accuracy of the documentation.
Note: Security Review has been skipped due to the limited scope of the PR.
|
|
||
| Not currently. | ||
| Not currently. Instead, | ||
| consider using the [`os_detect`][] package. |
There was a problem hiding this comment.
I think support exists for kIsWeb - if this is for platform detection would should perhaps update this question here.
There was a problem hiding this comment.
Interesting, @jonasfj asked for this fix. I'll see what he says when we reviews.
There was a problem hiding this comment.
kIsWeb looks fine to use.
But this question is written because historically lots of code has been using Platform.isAndroid and Platform.isIOS, while these are decidedly not deprecated (one can dream I suppose).
Using Platform.isAndroid will not work when targeting web with.
It's sadly technically allowed to import 'dart:io' when compiling for web. This is and should be strongly discouraged. And if you import 'dart:io' calling any method from it will throw an UnsupportedError. It is possible to work around this with IOOverrides. This is not encouraged.
On pub.dev, we will score packages as not supporting web if they import dart:io (they may import it with conditional imports).
If you're coding a flutter app using kIsWeb, is fine. Maybe, even desirable, as it's right there.
If you are coding a package for use by others, you should probably prefer os_detect (unless you have a flutter dependency, then it probably doesn't matter).
I think it's important to mention os_detect, because:
- Packages that don't have a flutter dependency should not add one to detect platform, instead they should use
os_detect. - Packages with a flutter dependency may prefer
os_detect, and it may be an easier migration path for packages that has been usingPlatform.isAndroidand friends.
I suspect the FAQ is here because dart:io can be imported when targeting web, it just becomes a runtime error to call Platform.isAndroid and friends. This is a bit of trap.
Sorry, for all the rambling. This is complex to put it mildly -- generally, I think we should tell users to avoid Platform.isXYZ, and instead prefer package:os_detect (or package:platform when the next PR lands).
There was a problem hiding this comment.
Got it - appreciate the explanation here, @jonasfj! Since this is in the main FAQ for the web part of our docs, I think we should mention both then. kIsWeb for an app, and os_detect for a package. Does you agree?
|
Visit the preview URL for this PR (updated for commit 193700c): https://flutter-docs-prod--pr13160-os-detect-2ngx3z6o.web.app |
Fixes #13146
Also, fixes formatting and updates guidance around static websites.