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

fix(@angular/cli): logic which determines which temp version of the CLI is to be download during ng update #22200

Merged
merged 1 commit into from
Nov 23, 2021
Merged

Conversation

alan-agius4
Copy link
Collaborator

Previously, when using an older version of the Angular CLI, during ng update, we download the temporary latest version to run the update. The ensured that when running that the runner used to run the update contains the latest bug fixes and improvements.

This however, can be problematic in some cases. Such as when there are API breaking changes, when running a relatively old schematic with the latest CLI can cause runtime issues, especially since those schematics were never meant to be executed on a CLI X major versions in the future.

With this change, we improve the logic to determine which version of the Angular CLI should be used to run the update.

Below is a summarization of this.

  • When using the --next command line argument, the @next version of the CLI will be used to run the update.
  • When updating an @angular/ or @nguniversal/ package, the target version will be used to run the update. Example: ng update @angular/core@12, the update will run on most recent patch version of @angular/cli of that major version @12.2.6.
  • When updating an @angular/ or @nguniversal/ and no target version is specified. Example: ng update @angular/core the update will run on most latest version of the @angular/cli.
  • When updating a third-party package, the most recent patch version of the installed @angular/cli will be used to run the update. Example if 13.0.0 is installed and 13.1.1 is available on NPM, the latter will be used.

@alan-agius4 alan-agius4 added the target: patch This PR is targeted for the next patch release label Nov 19, 2021
@google-cla google-cla bot added the cla: yes label Nov 19, 2021
@alan-agius4 alan-agius4 added the action: review The PR is still awaiting reviews from at least one requested reviewer label Nov 19, 2021
Copy link
Collaborator

@dgp1130 dgp1130 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When updating an @angular/ or @nguniversal/ and no target version is specified. Example: ng update @angular/core the update will run on most latest version of the @angular/cli.

Does ng update @angular/core go to latest or the next major? If I'm on v10 and run this, it should go to v11 right, since we only allow one major at a time? In that scenario, wouldn't we want to upgrade with the latest patch of the destination major (in this case, @angular/cli@11)?

packages/angular/cli/commands/update-impl.ts Outdated Show resolved Hide resolved
@alan-agius4
Copy link
Collaborator Author

Does ng update @angular/core go to latest or the next major? If I'm on v10 and run this, it should go to v11 right, since we only allow one major at a time? In that scenario, wouldn't we want to upgrade with the latest patch of the destination major (in this case, @angular/cli@11)?

When no version is specified the update will use the @latest tag. We can improve this but, it might be confusing that updating an angular package will update to the next major version while other packages will update to latest.

…LI is to be download during `ng update`

Previously, when using an older version of the Angular CLI, during `ng update`, we download the temporary `latest` version to run the update. The ensured that when running that the runner used to run the update contains the latest bug fixes and improvements.

This however, can be problematic in some cases. Such as when there are API breaking changes, when running a relatively old schematic with the latest CLI can cause runtime issues, especially since those schematics were never meant to be executed on a CLI X major versions in the future.

With this change, we improve the logic to determine which version of the Angular CLI should be used to run the update.

Below is a summarization of this.

- When using the `--next` command line argument, the `@next` version of the CLI will be used to run the update.
- When updating an `@angular/` or `@nguniversal/` package, the target version will be used to run the update. Example: `ng update @angular/core@12`,  the update will run on most recent patch version of `@angular/cli` of that major version `@12.2.6`.
- When updating an `@angular/` or `@nguniversal/` and no target version is specified. Example: `ng update @angular/core` the update will run on most latest version of the `@angular/cli`.
- When updating a third-party package, the most recent patch version of the installed `@angular/cli` will be used to run the update. Example if `13.0.0` is installed and `13.1.1` is available on NPM, the latter will be used.
@alan-agius4
Copy link
Collaborator Author

Note: once merged, I will create PRs to backport this change to all LTS versions of the CLI.

@dgp1130
Copy link
Collaborator

dgp1130 commented Nov 23, 2021

When no version is specified the update will use the @latest tag. We can improve this but, it might be confusing that updating an angular package will update to the next major version while other packages will update to latest.

Hmm, I'm a bit torn here. npm install @angular/core means @angular/core@latest, so I wouldn't want to diverge from that. It would also be weird for @angular/* packages to be special-cased like you mentioned. On the other hand running with the latest CLI means that it needs to support migrating from more than the previous version, which is what we're hoping to avoid.

I just tried this with today's logic to make sure I understood and running ng update @angular/core @angular/cli on a v10 project fails because it tries to update to v13, which isn't allowed (though the error message doesn't actually say that and we should probably be more clear that it tried to update to @latest).

$ ng update @angular/core @angular/cli
Using package manager: 'npm'
Collecting installed dependencies...
Found 29 dependencies.
Fetching dependency metadata from registry...
Updating multiple major versions of '@angular/core' at once is not supported. Please migrate each major version individually.
Run 'ng update @angular/core@11' in your workspace directory to update to latest '11.x' version of '@angular/core'.

For more information about the update process, see https://update.angular.io/?v=10.0-11.0

So I guess the decision of which CLI runner to use doesn't really matter since the update will fail anyways if @latest is not the next major version. Based on that, I'm ok with using the latest CLI runner as you currently have here, but this is maybe something we should look into when we eventually revamp ng update. I would love to see running ng update just go to the next major with no fuss, and this is something we'd have to tackle more comprehensively to make that happen safely.

@alan-agius4 alan-agius4 added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Nov 23, 2021
@alan-agius4 alan-agius4 merged commit 1e9e890 into angular:master Nov 23, 2021
@alan-agius4 alan-agius4 deleted the ng-update-runner-downloader branch November 23, 2021 06:44
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Dec 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants