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

Running migration for Angular 11 > 12 twice disables AOT compilation #20979

Closed
1 of 15 tasks
simeyla opened this issue May 28, 2021 · 2 comments · Fixed by #20980
Closed
1 of 15 tasks

Running migration for Angular 11 > 12 twice disables AOT compilation #20979

simeyla opened this issue May 28, 2021 · 2 comments · Fixed by #20980
Labels
area: @schematics/angular freq1: low Only reported by a handful of users who observe it rarely severity2: inconvenient type: bug/fix
Milestone

Comments

@simeyla
Copy link

simeyla commented May 28, 2021

🐞 Bug report

Command (mark with an x)

ng update @angular/cli --migrate-only --from 11 --to 12

  • new
  • build
  • serve
  • test
  • e2e
  • generate
  • add
  • update
  • lint
  • extract-i18n
  • run
  • config
  • help
  • version
  • doc

Is this a regression?

Default for Angular 11 was different

Description

The migration for Angular 11 -> 12 checks the values of fields such as aot, vendorChunk, optimization and:

  • Removes the value if it is equal to the Angular 12 default (because it's redundant)
  • If the value is not defined it will add it and set it equal to the Angular 11 default, because it assumes the config is working Angular 11 config

This makes perfect sense if the migration is guaranteed to run only once, but important fields such as aot can get set to false if the migration is run twice. There is no kind of safety check or warning that this has happened.

🔬 Minimal Reproduction

  • Set aot='true' in an Angular 11 config.
  • Run ng update @angular/cli --migrate-only --from 11 --to 12
  • This will remove the aot attribute
  • Run ng update @angular/cli --migrate-only --from 11 --to 12 again
  • This will add back aot attribute as false

Most people are likely to come across this issue if they manually performed ng update (which in my experience can take quite a few attempts) and also run the migration command. There's no warning that migrations have already been run or should only be run once.

There is nothing to protect a user from running the migration twice which leads to extremely confusing results.

Often when doing a large update it is recommended to take a 'fresh' angular.json file from a new project and compare it manually with your existing project. If you then run the migration after doing this (which is actually how I discovered it) then you'll end up with aot=false when it really ought to be true.

Suggested behavior

I can see how this issue would be closed as 'by design' and I would agree on a technicality with that decision, however this is going to cause a lot of confusion for many people and there really ought to be some kind of safety protection against this. Whether it creates a hidden file, or something else I'm not sure.

It ultimately is quite bizarre that if you didn't previously have a value set for aot (assuming the default of false) that it will now enforce that 'old' default when what you actually just asked for was a migration!

🌍 Your Environment


Angular CLI: 12.0.2
Node: 12.16.0
Package Manager: npm 6.10.3
OS: win32 x64

Angular: 12.0.2

Anything else relevant?

I am finding Angular 12 extremely slow compared to Angular 11 for compilation time. I've already seen discussions about projects getting in a 'corrupt' state and people finding that running the migrations command is fixing them. Presumably it is because the user only succeeded in disabling aot and thereby sped up their build.

See also:
#20792 (comment)

Recommendation by Angular team member @alan-agius4 to run the migration command due to a slow build:
#20713 (comment)

@alan-agius4
Copy link
Collaborator

As you mentioned this is expected.

In the case mentioned the recommendation was to run ng-update not to address a slow build, but rather address the issue were the user updated Angular CLI manually. None-the-less I did update my comment to reflect that ng-update shouldn't be run multiple times.

One of the migrations is non idempotent and therefore cannot be run multiple times. That said, I'll try to think of a way to make this idempotent.

@alan-agius4 alan-agius4 added area: @schematics/angular freq1: low Only reported by a handful of users who observe it rarely severity2: inconvenient labels May 28, 2021
@ngbot ngbot bot modified the milestone: needsTriage May 28, 2021
@ngbot ngbot bot modified the milestones: needsTriage, Backlog May 28, 2021
filipesilva pushed a commit that referenced this issue May 31, 2021
…idempotent

With this change we ensure that `update-angular-config-v12` migration is idempotent

Closes #20979
filipesilva pushed a commit that referenced this issue May 31, 2021
…idempotent

With this change we ensure that `update-angular-config-v12` migration is idempotent

Closes #20979

(cherry picked from commit afc9d10)
@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 Jul 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: @schematics/angular freq1: low Only reported by a handful of users who observe it rarely severity2: inconvenient type: bug/fix
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants