Remove package root

[mit-mit]

Removes the Platform.packageRoot, which was already marked deprecated (dart-lang/sdk#41197), and which doesn't work in Dart 2.

This is currently a blocker for dart-lang/sdk#47769

[mit-mit]

One question: do we think this should be a new version 4.0.0? Normally I'd say yes as this is technically a breaking change. But on the other hand the API was already broken...

[mit-mit]

cc @gspencergoog @lrhn for review

[gspencergoog]

This change looks good to me. Whether it counts as a breaking change probably depends on what the deprecation policy says. I know that Flutter doesn't make a new version for removing previously deprecated items, which our deprecation policy says are only removed after being deprecated for a year. I'm not sure what the corresponding policies are for Dart.

[mit-mit]

@kevmoo @natebosch thoughts on the version?

[kevmoo]

Ugh. Strictly speaking this should be a new major version (breaking change) since we're removing an API.

BUT this API hasn't worked forever...so I could see allowing an exception.

[mit-mit]

The flutter framework/package pins this. So if we do roll a major version, that may upset the ecosystem, and I suspect has a much larger probability of being a major distraction than actually removing this API that doesn't work (it was literally hard-coded to always return null) ?

[kevmoo]

It returns null? It doesn't throw? Hrm.

[natebosch]

This is currently a blocker for dart-lang/sdk#47769

How does this block a change in the SDK?

[mit-mit]

How does this block a change in the SDK?

Apparently a number of things in the SDK use it:

stderr:
third_party/pkg/platform/lib/src/interface/local_platform.dart:46:19: Error: Member not found: 'packageRoot'.
      io.Platform.packageRoot; // ignore: deprecated_member_use
                  ^^^^^^^^^^^

Details in https://dart-review.googlesource.com/c/sdk/+/221340

[mit-mit]

It returns null? It doesn't throw? Hrm.

Yup: https://dart-review.googlesource.com/c/sdk/+/221340/8/sdk/lib/io/platform.dart

[natebosch]

What I mean is, we could remove the call to io.Platform.packageRoot without changing the static API here.

[mit-mit]

What I mean is, we could remove the call to io.Platform.packageRoot without changing the static API here.

Hmm, it actually looks like @gspencergoog already did that recently (but that this version hasn't been published):
https://github.com/google/platform.dart/pull/35/files

Still, I don't know what the value is of retaining an API that is clearly broken and has no functional purpose. It just seems very, very confusing. I'm still very much in favor of getting it removed.

[mit-mit]

I'm pretty confident this doesn't break anything. We have not supported package directories for years: dart-lang/pub#1960

I suggest we a) move forward with removing this API, and b) that we keep the version increment minor. Any objections @natebosch ?

[natebosch]

Still, I don't know what the value is of retaining an API that is clearly broken and has no functional purpose. It just seems very, very confusing. I'm still very much in favor of getting it removed.

yeah I think it's fine to consider removing it, just wanted to make sure we weren't rushing it for a constraint that didn't exist.

I suggest we a) move forward with removing this API, and b) that we keep the version increment minor. Any objections

Not enough to block it, but I'm also not super eager to be the one on the hook to push it out and watch for breakages 😉