Add `DateTime.copyWith` to class instead of extension method

[lrampazzo]

Hello everyone,

Currently DateTime.copyWith is implemented via extension method:

https://api.flutter.dev/flutter/dart-core/DateTimeCopyWith/copyWith.html

Unfortunately this approach may lead to subtle bugs, because any other implementation of the DateTime class can call copyWith and be copied as the core lib implementation.

In our case, we are using timezone package, which provides an implementation of DateTime (TZDateTime). By using copyWith, we lose implicit information about the timezone.

Instead of having an extension method, is it viable to add copyWith as virtual method which can be overridden by subclasses?

[dart-github-bot]

Summary: User wants DateTime.copyWith as a virtual method instead of an extension method. This prevents unintended behavior with custom DateTime subclasses like those from the timezone package.

[lrhn]

Adding an instance method is a breaking change.
It will make every existing implementation of the interface invalid.

That's the reason the method was added as an extension method.

One feature that could allow adding instance methods to interfaces is interface default methods. Dart doesn't have that, or any current plans for ading.

So, it's very unlikely that we'll change the DateTime API to add copyWith until Dart gets interface default methods, if ever. (I hope, but haven't been able sell it yet.)

Or unless we make DateTime a final class, which will also make TZDateTime not compile, and with no workaround other than not being a DateTime. Which would be fine, then you can choose to use TZDateTime instead and carry that around, and convert that to a DateTime only when someone needs one. You can't trust that passing a TZDateTime into third party code will guarantee that you get a TZDateTime out again. Usually you won't. Subclassing value-objects to store more information is a risky idea.

[lrampazzo]

While I do understand the reason behind the decision to use extension methods, I do respectfully disagree with it. This means that we will not have any other method added to existing core library classes as well due to implicit interfaces (unless we get interface default methods)? Isn't this approach limiting for the core library evolution?

About the suggestion to make the DateTime class final, in terms of breaking changes I see it as by far more destructive than by adding copyWith as a method and I cannot see any benefit from this change, other than limiting users.

[lrhn]

This means that we will not have any other method added to existing core library classes as well due to implicit interfaces (unless we get interface default methods)?

Correct.

Isn't this approach limiting for the core library evolution?

Absolutely.

So go vote for interface default methods, please! 😉

Breaking changes are possible, but they are expensive. The cost/benefit tradeoff means that in the vast majority of cases, adding an extension method is just a more viable approach. The only other realistic alternative would be doing nothing.

The hypothetical benefit of making the class final would be that you can do this change freely afterwards and any future additions to the API without breaking anyone again.
Subclasses won't be broken because they won't exist, which is definitely more limiting, but if you expect an evolving API, it may be the right choice.

[lrampazzo]

So go vote for interface default methods, please! 😉

Voted!

The hypothetical benefit of making the class final would be that you can do this change freely afterwards and any future additions to the API without breaking anyone again. Subclasses won't be broken because they won't exist, which is definitely more limiting, but if you expect an evolving API, it may be the right choice.

How about achieving this in a less destructive way by making the class base instead of final (which suits very well this specific case)?

[lrhn]

Making the class base instead of final breaks every existing class which implements the interface, precisely the same classes that would be broken by simply adding copyWith as an instance method.
So it's just as breaking, the only advantage is that it will only break things once.
It does allow others to migrate from implementing to extending, but the class isn't really built for that to begin with.
So less invasive than beginning final, but not less than just doing nothing and adding a method.