`dart pub add` should sort dependencies

[dcharkes]

The first thing that happens after dart pub add abc is a lint that I should sort my dependencies.

[sigurdm]

I wonder if we should always sort them, or just if they were sorted already...

[DanTup]

@FMorschel fyi - I think you asked about this somewhere, but I don't recall where.

I wonder if we should always sort them, or just if they were sorted already...

I think if the list is already sorted the new package should go in the right place. If the list is not sorted, I don't think the existing entries should change though (probably just append?). If you don't care about order, you shouldn't get a more complex diff because you added a new one.

[FMorschel]

I asked about it here dart-lang/sdk#55985 (comment).

But I'd also like some more thoughts over at:

• sort_pub_dependencies should consider non-pub as different sdk#59714

If this is something you are thinking of addressing.

[sigurdm]

Yeah - if we decide to sort them, we also have to decide "how" to sort them...

I can see arguments both for ordering alphabetically by dependency name or first ordering by source.

[DanTup]

However it's sorted, I think it should match the sort_pub_dependencies lint (whether that means updating that lint, or copying what it already does).

[sigurdm]

Yeah - that's a good point.
I think sort_pub_dependencies is alphabetical https://dart.dev/tools/linter-rules/sort_pub_dependencies

Looking at yaml_edit it seems it already should preserve keys in sorted order:
https://github.com/dart-archive/yaml_edit/blob/eb2078f78b397a28410728bb6fc1104d68f5f6ea/lib/src/utils.dart#L163

@dcharkes can you provide a reproduction?

If I have

name: a
environment:
  sdk: ^3.9.0
dependencies:
  collection:
  retry:

And run dart pub add quiver it gets added after collection and before retry as expected.

[dcharkes]

Interesting, I cannot reproduce it anymore. I'll reopen this issue if I am ever able to repro it again.

(I'm very sure I had an issue, because I saw the lint. 🤷‍♂️ )

[DanTup]

can you provide a reproduction?

Create a pubspec with no dependencies, then do:

dart pub add path
dart pub add async

You end up with:

dependencies:
  path: ^1.9.1
  async: ^2.13.0

This logic looks wrong to me, it always appends if there is one existing item, rather than assuming it should be alphabetical?

https://github.com/dart-lang/tools/blob/fe1a6e087406b3881f492cea4548159571d2ef4b/pkgs/yaml_edit/lib/src/utils.dart#L151-L154

  // We can't deduce ordering if list is empty, so then we just we just append
  if (keys.length <= 1) {
    return map.length;
  }

[sigurdm]

Nice find!

Yeah - that looks like a bug in package yaml_edit!

It would also be nice if it provided more control on where to insert new keys in maps. The alphabetical logic is nice here, but not always.

[sigurdm]

dart-lang/tools#2259 should fix this

[sigurdm]

Closing in favor of dart-lang/tools#2258