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

Additional Breaking Changes for Dart 3.0 #49892

Open
3 tasks
TimWhiting opened this issue Sep 2, 2022 · 10 comments
Open
3 tasks

Additional Breaking Changes for Dart 3.0 #49892

TimWhiting opened this issue Sep 2, 2022 · 10 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-core type-enhancement A request for a change that isn't a bug

Comments

@TimWhiting
Copy link

TimWhiting commented Sep 2, 2022

I believe Dart 3.0 has the opportunity to make more breaking changes than just only disallowing non-null-safe code.

For example, I have seen in several issues that the dart language team has said, 'if we had null-safety to start with, we would have designed this api differently'.

Feel free to edit or add additional breaking changes to consider / discuss, but here are a few I can think of.

  • collections with StateError list.first, list.last, list.firstWhere -> change return type to nullable. Migrate existing usages to List.first.. ?? throw ... This has the benefit of making it much more explicit to the user where their code can throw. I know I've been bitten by these methods in the past.
  • MapEntry -> typedef MapEntry<K,V> = ({K key, V value}) using the Records feature which seems to be progressing quickly. This only breaks implementors/extenders of MapEntry I believe unless someone is relying on identity of the MapEntry object. Searching Github for "implements MapEntry" I see 95 usages in Dart code. (https://github.com/search?l=Dart&p=7&q=%22implements+MapEntry%22&type=Code) Most are copies of the LruCache's _Link from package:build or SplayTreeMapNode it looks like, but there are a few which basically just expose another name for the key or value, these can be migrated to extension methods or 'Views'. I'm not sure how to handle the migration of _Link or SplayTreeMapNode admittedly.
  • Add .indexed getter on List (could just be an extension method?) that returns a Record (int index, T value)
@lrhn lrhn transferred this issue from dart-lang/language Sep 2, 2022
@lrhn
Copy link
Member

lrhn commented Sep 2, 2022

Platform library changes are very difficult to pull off. Worst case, we need to migrate the world at the same time that we make the change. APIs do not have language versioning, so we can't change behavior based on the language version of the code calling the method.

Convering MapEntry to a record (or maybe a #struct) is something we have considered. I'd probably prefer the struct (or view on record) so that it has its own type.

The .indexed getter on List would definitely have to be an extension, and then the only thing holding anyone back from doing it today is the lack of a record type to use for the value. (But .asMap().entries is an Iterable<MapEntry<int, T>>, so maybe that's just what you want).

@bernaferrari
Copy link
Contributor

bernaferrari commented Sep 2, 2022

The MapEntry idea is awesome. Whatever you decide, I'm in. But I really liked the typedef idea.

@lrhn
Copy link
Member

lrhn commented Sep 5, 2022

The problem with making MapEntry a typedef of a record type is that it won't have the constructor that people are currently using. You can't do MapEntry(key, value) any more.
A view on a record could probably have a constructor, and the struct design would be the closest thing to the current class, while still not requiring a persistent identity. So, those are more likely to succeed, since the migration will be much smaller.

@lrhn lrhn added area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-core type-enhancement A request for a change that isn't a bug labels Sep 5, 2022
@bernaferrari
Copy link
Contributor

Yeah, makes sense. I'm not familiar with the view, if it works, perfect. If it doesn't, dart fix wouldn't be hard.

@Wdestroier
Copy link

The methods could accept MapEntry<K, V> | (K key, V value) entry with union types and the MapEntry class could be deprecated to be removed one or two major versions later.

@bernaferrari
Copy link
Contributor

As much as I want union types (personal favorite request), I don't think it will arrive before views/other things.

@ykmnkmi
Copy link
Contributor

ykmnkmi commented Dec 12, 2022

Variable number of arguments varargs.
From package:path changelog:

1.8.3: Support up to 16 arguments in join function and up to 15 arguments in absolute function.

@dupuchba
Copy link

dupuchba commented Apr 16, 2023

@TimWhiting
well, the ClojureDart compiler (its standard lib) does use MapEntry and while migrating to Dart 3 we're going to loose this interop btw Dart map-entry and clojure map-entry.
I know Dart 3 != Dart 2 but breaking changes are really not cool, I'd rather work on something else than patch the ClojureDart compiler for these kind of changes.

Plus it's not like I had a choice to not upgrade, Dart 2.X will eventually be dropped like Dart 1.X was.

@TimWhiting
Copy link
Author

These were just proposed changes. I'm not part of the dart team, and the map entry change is unlikely to happen at least for dart 3.

@lrhn
Copy link
Member

lrhn commented Apr 16, 2023

The MapEntry class was made final in the Dart 3 platform libraries, which means it cannot be subclassed by other 3.0 libraries.
So that has happened.

It won't break existing 2.19 code, but it may require some rewriting when upgrading to language version 3.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-core type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

6 participants