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

Figure out how we will handle the fact that adding an extension method can be breaking #620

Open
natebosch opened this issue May 21, 2021 · 5 comments

Comments

@natebosch
Copy link
Member

dart-archive/collection#196 introduced an extension method which conflicted with the same name defined in another library in at least one internal usage. We can handle the conflict internally, but on pub it may not be as easy.

cc @lrhn

@lrhn
Copy link
Member

lrhn commented Jun 1, 2021

Adding an extension method is like adding a top-level name - it can conflict with other declarations with the same name in the same scope.

We don't generally disallow adding top-level members, and the recommended solution for conflicts, which you discover when you do a pub get or pub upgrade, is to hide the one of the names that you aren't using. Since one of them didn't exist before, you definitely weren't using it, and since there is a conflict, you're actually using the other one.

Doing a pub upgrade is not a safe operation which guarantees a working program afterwards. It never was. A non-breaking change in two packages can break a third package anyway when it tries to combine the two. (If we accept that non-breaking changes exist in Dart at all, which we need to pretend for everybody's sanity's sake).

I think the strategy for conflicting extensions should be the same, which means not doing anything in particular, and patching up conflicts when they occur.

The one additional crinkle with extensions is that hiding the extension will hide all its members.
If you are currently using the foo method from extension E1 and the bar method from extension E2, and then E2 also adds a foo method which conflicts with E1.foo (or even takes precedence over it), then you can't just hide E2. Importing one of them extensions with a prefix does not remove the conflict for implicit invocations. You have to use E1(e).foo everywhere you previously used e.foo, otherwise you can't access E2.bar any more. That's the recommended override notation for handling conflicts which cannot be handled using hide.

The counter-design to that problem is to put each individual extension method in its own extension declaration. See #613.
That's not great, we have grouping for a reason, not just convenience (although losing the convenience and having to make up new extension declaration names is bad in itself).

Another alternative is to allow hiding individual extension members. See dart-lang/language#1627.

For now, you have to either hide the conflicting extension, if you don't use it at all, which can easily happen if you import a large library, or use explicit extension invocation for the conflicted name everywhere.

@natebosch
Copy link
Member Author

Collecting examples of hitting this problem when rolling this package internally:

@jakemac53
Copy link
Contributor

It does seem clear at this point that these conflicts are a much more common occurrence than when adding new top level symbols, and probably deserve some special treatment and/or pattern for how we address them?

@natebosch
Copy link
Member Author

I think we should try publishing and see what kind of noise it makes in the ecosystem, then use what we learn to make decisions about potential conflicts in the future.

@lrhn
Copy link
Member

lrhn commented Sep 9, 2022

The safest approach appears to be to have an extension declaration per extension method, because then you can hide individual extensions without losing any other extension method.

There is no way to avoid conflicts, other than not adding new extensions at all, which is not viable.
The best we can do is to help clients resolve the conflicts.

It's just highly annoying to have to have to split a group of related extensions into separate and individually named extensions, and it's not something I'd expect from other package authors.
Maybe this package is just so widely used that it has to make considerations for other packages.

@mosuem mosuem transferred this issue from dart-archive/collection Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants