Skip to content

Prompt to run "pub upgrade" upon opening a package if the SDK versions has changed #4261

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

Closed
DanTup opened this issue Nov 10, 2022 · 7 comments
Labels
in commands Relates to commands (usually invoked from the command Palette) is enhancement
Milestone

Comments

@DanTup
Copy link
Member

DanTup commented Nov 10, 2022

When the user upgrades their SDK (a major or minor change, but not patch) there may be new versions of packages that were not available before (or that are required for some classes of changes). We should detect this change when we're checking if they need to run "pub get" (we can get the last version used to fetch packages from .dart_tool/package_config.json: ("generatorVersion": "2.19.0-374.0.dev")) and offer to run pub upgrade (in preference to pub get).

@devoncarew should we do this in both directions, or only one? It probably doesn't affect many users, but I realise that I often switch between SDK versions (for testing) and if I click pub upgrade on this prompt and then switch back to an older SDK, there's a possibility that I now have packages that are not valid. Would pub upgrade even fix this (I don't thinkpub downgrade is what's wanted here, because I want to go back to "the highest that is valid given this new (lower) SDK"?)

@DanTup DanTup added is enhancement in commands Relates to commands (usually invoked from the command Palette) labels Nov 10, 2022
@DanTup DanTup added this to the v3.54.0 milestone Nov 10, 2022
@devoncarew
Copy link
Contributor

Interesting. I think the sdk downgrade case is pretty rare. I could see people running into it if they (intentionally) have multiple SDKs installed locally.

I would say we could either:

  1. not handle it, or
  2. handle it but switch to using pub get?

We'd mostly be concerned with package versions that solved into the newer sdk but that wouldn't solve into the older sdk.

@DanTup
Copy link
Member Author

DanTup commented Nov 10, 2022

handle it but switch to using pub get?

Ah, I hadn't considered that that would fix it, that makes sense. So where today it's:

  • if package_config is missing or timestamped older than pubspec
    • prompt to pub get

I think we probably want:

  • if package_config is missing
    • prompt to pub get
  • else if sdk major/minor version has increased since current package_config
    • prompt to pub upgrade
  • else if package_config is timestamped older than pubspec or sdk major/minor version has decreased
    • prompt to pub get

?

@devoncarew
Copy link
Contributor

sgtm!

@DanTup
Copy link
Member Author

DanTup commented Nov 14, 2022

@devoncarew here's how this current looks (this prompt is just a special case of the pub-get check when you open a project or change your workspace folders):

Screenshot 2022-11-14 at 15 53 50

Let me know if you have any suggestions for the text. It may also be useful to have a "More Info" button here that explains why this this is recommended. I can add something to the Dart-Code website, although perhaps it's better having something on the Dart website (or Wiki?) since this may be applicable to other editors (and CLI users)?

(I'm slightly worried that "Upgrade packages" might make it seem like this is going to change constraints in pubspec or similar, and people may skip clicking it unsure what it does)

@DanTup DanTup closed this as completed in cc509eb Nov 14, 2022
@devoncarew
Copy link
Contributor

devoncarew commented Nov 14, 2022

The prompt looks great - thanks!

I'm slightly worried that "Upgrade packages" might make it seem like this is going to change constraints in pubspec or similar, and people may skip clicking it unsure what it does

It matches the 'get' packages prompt however? Instead of Upgrade packages we could say run dart pub upgrade? It's more technical but there'd be less uncertainty about what the tool is going to do. I'm not 100% sure this is an issue, but the command name would have slightly more clarity.

@DanTup
Copy link
Member Author

DanTup commented Nov 14, 2022

Yeah, the other one said "Get packages". I've changed them to run 'pub get' and run 'pub upgrade' (I left off dart to be shorter and avoid confusion with flutter and this prompt may apply to multiple projects in the workspace), looks like this now (I changed "upgrade" to "updated" in the text as I think it reads a little better too, but happy to tweak):

Screenshot 2022-11-14 at 17 13 04

@devoncarew
Copy link
Contributor

Looks great - very clear about what it'll do (and makes sense to not specify the 'dart' or 'flutter' prefix).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in commands Relates to commands (usually invoked from the command Palette) is enhancement
Projects
None yet
Development

No branches or pull requests

2 participants