Skip to content

[breaking change] Make dart:io Platform abstract #52444

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

Closed
brianquinlan opened this issue May 18, 2023 · 12 comments
Closed

[breaking change] Make dart:io Platform abstract #52444

brianquinlan opened this issue May 18, 2023 · 12 comments
Assignees
Labels
area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). breaking-change-approved breaking-change-request This tracks requests for feedback on breaking changes

Comments

@brianquinlan
Copy link
Contributor

Change Intent

Add the abstract class modifier to the dart:io Platform class.

Justification

Platform only has static properties so there is no reason to make instances of it.

Impact

All code that constructs Platform (i.e. Platform()) will break. There is one known pattern where this is done: package:dcli implements a non-static extension on Platform to allow users to get the line ending for the platform i.e.

Platform().eol;

Mitigation

In Dart 3.1:

  • The constructor for Platform is deprecated.
  • Platform contains a lineTerminator property that obviates the need for the extension in package:dcli.

Before we mark Platform as abstract, I will remove the extension from package:dcli and give users some time to update their dependencies. Making the change in Dart 3.2 would give users an approximately 6 week period to update their dependencies and code.

@brianquinlan brianquinlan added the breaking-change-request This tracks requests for feedback on breaking changes label May 18, 2023
@kevmoo
Copy link
Member

kevmoo commented May 18, 2023

SGTM – although deferring to @mit-mit

@mit-mit
Copy link
Member

mit-mit commented May 22, 2023

I'm a bit confused about the timeline as the Mitigation section mentions both 3.1 and 3.2. Can you clarify @brianquinlan ?

@itsjustkevin
Copy link
Contributor

@vsmenon @Hixie and @grouma another breaking change for your review.

@grouma
Copy link
Member

grouma commented May 22, 2023

As far as I can tell this doesn't impact ACX or our customers. Can we run a global presubmit to validate?

@mit-mit mit-mit added the area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). label Jun 7, 2023
@brianquinlan
Copy link
Contributor Author

I'm a bit confused about the timeline as the Mitigation section mentions both 3.1 and 3.2. Can you clarify @brianquinlan ?

I was hoping to deprecate the Platform constructor in Dart 3.1
I was hoping to make Platform abstract in Dart 3.2

But @timsneath suggested that Dart 3.2 is too aggressive so we can push that out to... when? I don't know what our policy is here.

@mit-mit
Copy link
Member

mit-mit commented Jun 8, 2023

I suggest a few more releases in between deprecation and the following change.

@vsmenon
Copy link
Member

vsmenon commented Jun 9, 2023

lgtm - let's go forward with marking deprecated

If we're conservative, we'd give a year. We can decide whether we want to pull that forward.

@itsjustkevin
Copy link
Contributor

@vsmenon I would like to mark this as approved, but am still waiting for feedback from @Hixie and some assurance through a global presubmit for @grouma.

@itsjustkevin
Copy link
Contributor

Pinging @grouma and @Hixie

@Hixie
Copy link
Contributor

Hixie commented Jul 17, 2023

We generally seem to use abstract final class in the Flutter framework for such "namespace" classes.
But either way, fine by me.
I wish we could move Platform out of dart:io and dart:html and into dart:core so that it works the same everywhere, though.

@itsjustkevin
Copy link
Contributor

@brianquinlan I would like to mark this as approved, could you respond the question by @grouma above so we can clear this up?

@brianquinlan
Copy link
Contributor Author

I'm fine waiting a year to actually do the removal. We did the deprecation in Dart 3.1.0 (released August 16, 2024) so I'll make a note to remove this around August 2024

As far as I can tell this doesn't impact ACX or our customers. Can we run a global presubmit to validate?

We can consider that in a year - but hopefully by then people will have reacted to the deprecation ;-)

copybara-service bot pushed a commit that referenced this issue Aug 9, 2024
Bug:#52444
Change-Id: Ia4f98162f2b60d23c27ed9d8f78f70d308785650
CoreLibraryReviewExempt: landing previously-approved change
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/379740
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Brian Quinlan <bquinlan@google.com>
@github-project-automation github-project-automation bot moved this from In review to Complete in Breaking Changes Aug 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). breaking-change-approved breaking-change-request This tracks requests for feedback on breaking changes
Projects
Status: Complete
Development

No branches or pull requests

7 participants