Add web safe indirection to Platform.isPlatform getters #33406
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jonahwilliams
added
the
a: tests
goderbauer
added
the
framework
Contributor
|
It would be nice if these were const values instead of accessors, a la dart-lang/sdk#35705. Then the code could be more completely optimized by the compiler. But at least these are accessors and not functions so that they can be swapped out for const values in the future. |
Contributor
Author
|
We could definitely do this ourselves once a) the const evaluation update lands, allowing us to provide defines to non-AOT builds b) we produce different bundles/snapshots per architecture |
added 3 commits
May 28, 2019 13:58
justinmc
approved these changes
May 31, 2019
Contributor
justinmc
left a comment
justinmc
left a comment
There was a problem hiding this comment.
LGTM 👍
Looks like you'll need to merge with master.
jonahwilliams
pushed a commit
that referenced
this pull request
Jun 3, 2019
…)" This reverts commit d92b3b4.
jonahwilliams
pushed a commit
that referenced
this pull request
Jun 3, 2019
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
flutter tests compiled to JavaScript will not be able to safely invoke
Platform.isWindows/isLinux/isMacOSin skip fields since thisdart:iomethod has no implementation currently in the web SDK. Even if we added an implementation, it would likely not capture the spirit of these particular skips - we don't want to conflate skipping something on flutter tester on Windows with skipping something running in a browser on Windows.To avoid the lint "don't define a class with only static members" I made these top level getters, though I am open to other approaches.
Related Issues
#33349