Skip to content

feat: replace Platform.isX with defaultTargetPlatform#1446

Merged
Gustl22 merged 2 commits into
mainfrom
gustl22/defaultTargetPlatform
Mar 20, 2023
Merged

feat: replace Platform.isX with defaultTargetPlatform#1446
Gustl22 merged 2 commits into
mainfrom
gustl22/defaultTargetPlatform

Conversation

@Gustl22

@Gustl22 Gustl22 commented Mar 20, 2023

Copy link
Copy Markdown
Collaborator

Description

Flutter recommends using defaultTargetPlatform instead of Platform.isX, where possible. This also enables testing platform specific dart code in unit tests (but without native code), which is way more convenient and faster.

Checklist

  • The title of my PR starts with a [Conventional Commit] prefix (fix:, feat:, docs:, chore: etc).
  • I have read the [Contributor Guide] and followed the process outlined for submitting PRs.
  • I have updated/added tests for ALL new/updated/fixed functionality.
  • I have updated/added relevant documentation and added dartdoc comments with ///, where necessary.
  • I have updated/added relevant examples in [example].

Breaking Change

  • Yes, this is a breaking change.
  • No, this is not a breaking change.

@Gustl22 Gustl22 requested a review from spydon March 20, 2023 16:29
@spydon

spydon commented Mar 20, 2023

Copy link
Copy Markdown
Member

The dart:io.Platform object should only be used directly when it's critical to actually know the current platform, without any overrides possible (for example, when a system API is about to be called).

Isn't that the case here?

@Gustl22

Gustl22 commented Mar 20, 2023

Copy link
Copy Markdown
Collaborator Author

Isn't that the case here?

Where exactly do you mean? Building the AndroidContext or converting to json doesn't call any native platform methods. So it's not critical in my opinion. Further we cannot mock the platform to test these methods without an integration test, which should not be needed in many cases. See this test which has different outcomes for different platforms, but is not needed to be executed on an according device / emulator.

@spydon

spydon commented Mar 20, 2023

Copy link
Copy Markdown
Member

Isn't that the case here?

Where exactly do you mean? Building the AndroidContext or converting to json doesn't call any native platform methods. So it's not critical in my opinion. Further we cannot mock the platform to test these methods without an integration test, which should not be needed in many cases. See this test which has different outcomes for different platforms, but is not needed to be executed on an according device / emulator.

Alright, sounds good then. :)

@Gustl22 Gustl22 merged commit 6cd5656 into main Mar 20, 2023
@Gustl22 Gustl22 deleted the gustl22/defaultTargetPlatform branch March 20, 2023 18:50
@Gustl22 Gustl22 mentioned this pull request Mar 21, 2023
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants