Skip to content
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

[v9]: Replace custom Platform & PlatformChecker with package platform #2646

Open
vaind opened this issue Jan 30, 2025 · 7 comments
Open

[v9]: Replace custom Platform & PlatformChecker with package platform #2646

vaind opened this issue Jan 30, 2025 · 7 comments
Assignees
Milestone

Comments

@vaind
Copy link
Collaborator

vaind commented Jan 30, 2025

Description

Replace:

  • dart/lib/src/platform/platform.dart
  • dart/lib/src/platform_checker.dart

with https://pub.dev/packages/platform

@denrase
Copy link
Collaborator

denrase commented Feb 17, 2025

@vaind Hey! Just started this and i'm seeing that example_web_compile_test.dart fails, as a file in the package uses dart:io

https://github.com/dart-lang/core/blob/main/pkgs/platform/lib/src/interface/local_platform.dart

Not sure how to circumvent this. Any suggestions?

@denrase denrase moved this from Todo to In Progress in Mobile & Cross Platform SDK Feb 17, 2025
@ueman
Copy link
Collaborator

ueman commented Feb 17, 2025

You should be checking for "kIsWeb" (or do the same check in Dart code) before using the platform package.

@buenaflor
Copy link
Contributor

buenaflor commented Feb 17, 2025

You should be checking for "kIsWeb" (or do the same check in Dart code) before using the platform package.

we can definitely work around this but then we'd still have to use the existing impl for web so not sure if it's worth it to use it? wdyt

@vaind
Copy link
Collaborator Author

vaind commented Feb 18, 2025

Any suggestions?

Is there a branch I can have a look at?

@denrase
Copy link
Collaborator

denrase commented Feb 18, 2025

@vaind Pushed the code i had locally. Pretty much replaced our Platform definition in favour of package:platform/platform.dart, but here we have the issue that dart:io is used.

@vaind
Copy link
Collaborator Author

vaind commented Feb 18, 2025

I've missed that package:platform doesn't support web at the moment. I've opened a draft PR to add support but for now, we cannot use it.

At the very least, I've taken a stab at cleaning up PlatformChecker (moving actual platform-specific stuff to the Platform) and added TODOs for potential improvements. @denrase see #2730 what do you think about these changes and the TODOs in PlatformChecker. Also, feel free to pick it up as I won't have the capacity to do so in the next couple of days, other than fixing any CI issues with #2730

@denrase
Copy link
Collaborator

denrase commented Feb 19, 2025

@vaind Thank you for your insights and the PR, i'll take a look at it. 🙇

@denrase denrase moved this from In Progress to Needs Review in Mobile & Cross Platform SDK Feb 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Needs Review
Development

No branches or pull requests

4 participants