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

Replace custom Platform & PlatformChecker with package platform #2646

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

Replace custom Platform & PlatformChecker with package platform #2646

vaind opened this issue Jan 30, 2025 · 7 comments

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

Update: we've done some improvements/refactors in PlatformChecker but this issue is blocked until dart officially adds support for web as well, see dart-lang/core#862

@github-project-automation github-project-automation bot moved this to Needs Discussion in Mobile SDKs Jan 30, 2025
@buenaflor buenaflor added this to the 9.0.0 milestone Jan 30, 2025
@buenaflor buenaflor moved this from Needs Discussion to Todo in Mobile SDKs Jan 30, 2025
@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 SDKs 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 SDKs Feb 25, 2025
@buenaflor buenaflor moved this from Needs Review to Blocked in Mobile SDKs Mar 11, 2025
@buenaflor buenaflor changed the title [v9]: Replace custom Platform & PlatformChecker with package platform Replace custom Platform & PlatformChecker with package platform Mar 11, 2025
@buenaflor buenaflor removed the 9.0.0 label Mar 11, 2025
@buenaflor buenaflor removed this from the 9.0.0 milestone Mar 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Blocked
Development

No branches or pull requests

4 participants