Migrations are failing, when @angular-devkit/core is not on root level in the node_modules

[Lipata]

Description

It is possible that the @angular-devkit/core is not at the root level in the node_modules. Migrations are developed so that they require that package on the root level, however, we should revisit that.

• igniteui-angular version: 17.0.x
• browser: N/A

Steps to reproduce

1. Have a project with @angular/core & @angular/cli version 16.2
2. ng update @angular-eslint/schematics @angular/cli @angular/core @infragistics/igniteui-angular
   or ng update @angular-eslint/schematics @angular/cli @angular/core @infragistics/igniteui-angular @igniteui-cli

Result

Migrations for igniteui-angular or igniteui-cli fails

Expected result

Migrations are executed

[Lipata]

We will do the following:

• dig deeper to understand the reason behind the missing @angular-devkit/core package on the root level.
• in any case we should:
  • see if we can get rid of the @angular-devkit/core dependency in the migrations
  • or see if we can load @angular-devkit/core dynamically, like we are doing with the language service

[kdinev]

@Lipata We can't rely on a package we depend on to just be present. We have to make sure we load the package regardless of whether the project already has it or not.

[jackofdiamond5]

@kdinev It seems to me like during an ng update to 17 the package @nguniversal/builders gets replaced with the package @angular-devkit/build-angular so it is removed from the package.json file.

Screenshot during ng update:

However, it is still present in the lock file which causes a peer dependency conflict, so the update schematic enforces an npm install which messes up the dependency tree and the resulting lock file does not have an @angular-devkit/core package at the root level, instead, it is only present for packages that explicitly list it as a dependency. This in turn causes the error in our migrations since we do not list it as a dependency even though we rely on it which, as you said, shouldn't be the case.

I managed to reproduce this behavior on a clean Angular project (without using ng update) using only dependencies and dev-dependencies from the bellum-gens repo. One solution for this problem is for the project to do a clean install or to remove the lock file before updating.

As for igniteui-angular we may be able to work around this by getting rid of all the code that we use from the @angular-devkit/core package or by listing the package as a dependency, or by installing it runtime during an update. These are far from ideal though as a much better solution (as proposed by @damyanpetev) could be to separate the migrations logic to a different package which has the core package listed as a dependency and install that during an update. This way we will not put additional load on igniteui-angular with an additional package and will be safe in case of any other shenanigans happening during an ng update.

[kdinev]

@jackofdiamond5 Deleting the package-lock worked, I confirm. Should I close the issue, if this is the solution?

[jackofdiamond5]

@kdinev This is more of an example of how to get around the problem, we still need to make sure our migrations don't fail in this particular case. After discussing this further with @damyanpetev we agreed to try and dynamically include the core package right before the migrations' execution. This should fix the problem as we will include the package exactly when it is needed which will be right after ng update messes up the lock file but before we execute any migrations. It is still not ideal but for the moment we don't have an easy way provided by Angular to work around this, moreover in the future they might face the same issue with their material package as they have a similar situation there with a package that's being used in-code but not listed as a dependency. So, let's keep the issue open for now and I will PR some changes for our migrations. :)

Ref - angular/angular-cli#26167

[kdinev]

@jackofdiamond5 From what you're describing it seems to me that this is a one-off issue with the migrations and it's caused by the particular Angular update, because they are renaming packages themselves. If that's the case, then it's enough to put guidance in the update guide on how to get the migrations executed correctly.

[damyanpetev]

Even more details and explanations:
I agree that this is likely a rare occurrence/exception to the usual working order, namely the removal of the @nguniversal/* packages as pointed out above by @jackofdiamond5.
Here's exactly what (I think) is happening with full repro here: https://github.com/damyanpetev/bellum-gens/tree/update-package-issues

Pretty much all installs with recent version of npm (>v7) are performed with --force (see https://github.com/angular/angular-cli/blob/10dbdaccb0af04bd88e0ecd2dda54e7910968865/packages/angular/cli/src/commands/update/cli.ts#L1047-L1068) which is why we've been able to update packages in separation for a while I guess (certainly how I've always done it) except some depts that do fail the the ng CLI checks.

As those were suppressed by the linked fix above for the universal package, note how that migration is the last that to update the packages (the ✔ Packages successfully installed. indicator from the schematics NodePackageInstallTask):

bellum-gens> ng update @angular/cli @angular-eslint/schematics
The installed Angular CLI version is outdated.
Installing a temporary Angular CLI versioned 17.0.0 to perform the update.
✔ Packages successfully installed.
Using package manager: npm
Collecting installed dependencies...
Found 57 dependencies.
Fetching dependency metadata from registry...
    Updating package.json with dependency @angular-devkit/build-angular @ "17.0.0" (was "16.2.2")...
    Updating package.json with dependency @angular-eslint/builder @ "17.0.1" (was "16.1.2")...
    Updating package.json with dependency @angular-eslint/eslint-plugin @ "17.0.1" (was "16.1.2")...
    Updating package.json with dependency @angular-eslint/eslint-plugin-template @ "17.0.1" (was "16.1.2")...
    Updating package.json with dependency @angular-eslint/schematics @ "17.0.1" (was "16.1.2")...
    Updating package.json with dependency @angular-eslint/template-parser @ "17.0.1" (was "16.1.2")...
    Updating package.json with dependency @angular/cli @ "17.0.0" (was "16.2.2")...
    Updating package.json with dependency @angular/compiler-cli @ "17.0.2" (was "16.2.5")...
    Updating package.json with dependency @angular/language-service @ "17.0.2" (was "16.2.5")...
    Updating package.json with dependency @angular/localize @ "17.0.2" (was "16.2.5")...
    Updating package.json with dependency ng-packagr @ "17.0.0" (was "16.2.3")...
    Updating package.json with dependency typescript @ "5.2.2" (was "5.1.6")...
    Updating package.json with dependency @angular/animations @ "17.0.2" (was "16.2.5")...
    Updating package.json with dependency @angular/common @ "17.0.2" (was "16.2.5")...
    Updating package.json with dependency @angular/compiler @ "17.0.2" (was "16.2.5")...
    Updating package.json with dependency @angular/core @ "17.0.2" (was "16.2.5")...
    Updating package.json with dependency @angular/forms @ "17.0.2" (was "16.2.5")...
    Updating package.json with dependency @angular/platform-browser @ "17.0.2" (was "16.2.5")...
    Updating package.json with dependency @angular/platform-browser-dynamic @ "17.0.2" (was "16.2.5")...
    Updating package.json with dependency @angular/platform-server @ "17.0.2" (was "16.2.5")...
    Updating package.json with dependency @angular/router @ "17.0.2" (was "16.2.5")...
    Updating package.json with dependency @angular/service-worker @ "17.0.2" (was "16.2.5")...
    Updating package.json with dependency zone.js @ "0.14.2" (was "0.13.3")...
UPDATE package.json (4324 bytes)
✔ Packages successfully installed.
** Executing migrations of package '@angular-eslint/schematics' **

> Updates @angular-eslint to v17.
UPDATE package.json (4327 bytes)
✔ Packages installed successfully.
  Migration completed (1 file modified).

** Executing migrations of package '@angular/cli' **

> Replace usages of '@nguniversal/builders' with '@angular-devkit/build-angular'.
UPDATE angular.json (19219 bytes)
UPDATE package.json (4288 bytes)
  Migration completed (2 files modified).

> Replace usages of '@nguniversal/' packages with '@angular/ssr'.
RENAME projects/bellumgens/server-bellumgens.ts => projects/bellumgens/server-bellumgens.ts.bak
RENAME projects/ebleague/server-ebleague.ts => projects/ebleague/server-ebleague.ts.bak
CREATE projects/bellumgens/server-bellumgens.ts (2205 bytes)
CREATE projects/ebleague/server-ebleague.ts (2203 bytes)
UPDATE projects/bellumgens/src/app/app.module.ts (3619 bytes)
UPDATE projects/ebleague/src/app/app.module.ts (3185 bytes)
UPDATE package.json (4273 bytes)
✔ Packages installed successfully.
  Migration completed (5 files modified).

> Replace deprecated options in 'angular.json'.
UPDATE angular.json (19203 bytes)
  Migration completed (1 file modified).


> Angular v17 introduces a new control flow syntax that uses the @ and } characters.
  This migration replaces the existing usages with their corresponding HTML entities.
UPDATE projects/ebleague/src/app/tournament-registration/registration-success/registration-success.component.html (1213 bytes)
  Migration completed (1 file modified).

> Updates `TransferState`, `makeStateKey`, `StateKey` imports from `@angular/platform-browser` to `@angular/core`.
  Migration completed (No changes made).

> CompilerOption.useJit and CompilerOption.missingTranslation are unused under Ivy.
  This migration removes their usage
  Migration completed (No changes made).

That makes me think while the other packages are installing (core, cli, etc) the universal builder is still present and its dependency to @angular-devkit/core@16 stands and keeps the old package on root level, thus forcing the newly installed packages to all nest the v17 devkit package as a sub-dependency in the tree. And then after the universal package is removed, whether by design or mistake or possibly due to the --force applied on the installs, npm doesn't attempt to rectify the tree structure, simply deletes the old v16 package from top level and leaves none in it's place, just a bunch of v17 copies nested further down:
https://github.com/damyanpetev/bellum-gens/blob/a205795d72ab08f59798c2dfd4770b9cb70b2c62/package-lock.json

There might be multiple issues here, not sure if npm is meant to do that or perhaps even the order of execution of those migrations could alter the result (i.e. remove the deprecated package first). IMO that last bit might be a thing the ng CLI could improve, because while rare this is not impossible to happen again and I don't think the package tree as left in this state is ideal.

Regardless, for the immediate solution, while nuking node_modules and the package-lock.json and installing again will certainly do the trick, I have an alternative to offer that's a bit less all-encompassing I think:

npm dedupe --legacy-peer-deps

That seems to do the job quite fine damyanpetev/bellum-gens@9287969 - now a single top level @angular-devkit/core and few other minor cleanups, but nothing too major.
PS: @kdinev in your case the explosive option might still be beneficial even separately, at least locally it bumped the lock version to 3 and removed about a 3rd of the file in bulk :)

After which ng update @infragistics/igniteui-angular is also able to run damyanpetev/bellum-gens@cc41515
So that would be my suggestion for the Upgrade Guide - IF your project is using universal SSR, run in two parts (cli, core, etc) -> dedupe in between -> update igniteui-angular.

---

As for the future direction, yes we're relying on packages we don't explicitly declare as dependencies and we might need some safety measures, yet as mentioned other packages are doing much the same, for example the NodePackageInstallTask use here
https://github.com/angular/components/blob/d7b454315ac739eae0318f8de7dce99d6f7996fe/src/material/schematics/ng-update/migrations/legacy-imports-error.ts#L10
There's no explicit devkit dependency either and IMO that entire story is not completely fleshed out.
Even if we try to split most of schematics into a separate package, the migration collection is still bound to the library I believe and we might still need the above install task to neatly plug into the whole process and I'm not sure the result will be any less messy.