Ngrx18 migration incomplete - migration for breaking changes are not included

colinscz commented 4 months ago

Current Behavior

Currently some of the ngrx dependencies are automatically updated. As introduced in this PR: https://github.com/nrwl/nx/pull/26549/files

But there are some breaking changes and new packages introduced in certain scenarios that are not yet covered by the migration. This results in the build failing because for example this import right here: import { concatLatestFrom } from '@ngrx/effects';

is not found anymore and newly reside in the other package: import { concatLatestFrom } from '@ngrx/operators';

More info here: https://ngrx.io/guide/migration/v18

Could you please add migrations for those scenarios?

I checked the open PRs and issues but I haven't found any planned change for this.

Expected Behavior

Breaking changes from Ngrx migration including changes for some code moved from @ngrx/effects to @ngrx/operators are included in the migrations.

GitHub Repo

No response

Steps to Reproduce

Update from a Nx Angular workspace containing more than just the ngrx store dependency (in my case I was on Nx 17.3.2)
Try to run the application after this upgrade and after all necessary migrations have been applied

Nx Report

Nx report from before the nx migrate latest command was executed since after that I already have issues with the npm install:

   Node   : 18.20.2
   OS     : win32-x64
   npm    : 10.5.0

   nx (global)        : 19.3.1
   nx                 : 17.3.2
   @nx/js             : 17.3.2
   @nx/jest           : 17.3.2
   @nx/linter         : 17.3.2
   @nx/eslint         : 17.3.2
   @nx/workspace      : 17.3.2
   @nx/angular        : 17.3.2
   @nx/cypress        : 17.3.2
   @nx/devkit         : 17.3.2
   @nx/eslint-plugin  : 17.3.2
   @nrwl/tao          : 17.3.2
   @nx/web            : 17.3.2
   @nx/webpack        : 17.3.2
   typescript         : 5.3.3
   ---------------------------------------
   Community plugins:
   @ngrx/component      : 17.0.1
   @ngrx/data           : 17.0.1
   @ngrx/effects        : 17.0.1
   @ngrx/entity         : 17.0.1
   @ngrx/router-store   : 17.0.1
   @ngrx/store          : 17.0.1
   @ngrx/store-devtools : 17.0.1
   eslint-plugin-ngrx   : 2.1.4
   nx-stylelint         : 17.1.4

Failure Logs

After the migrations to Nx 19.3.1 were run in the project

$ npm install
npm ERR! code ERESOLVE
npm ERR! ERESOLVE unable to resolve dependency tree
npm ERR!
npm ERR! While resolving: corapi-ui@0.0.0
npm ERR! Found: @angular/common@18.0.4
npm ERR! node_modules/@angular/common
npm ERR!   @angular/common@"18.0.4" from the root project
npm ERR!
npm ERR! Could not resolve dependency:
npm ERR! peer @angular/common@"^17.0.0" from @ngrx/component@17.0.1
npm ERR! node_modules/@ngrx/component
npm ERR!   @ngrx/component@"17.0.1" from the root project
npm ERR!
npm ERR! Fix the upstream dependency conflict, or retry
npm ERR! this command with --force or --legacy-peer-deps
npm ERR! to accept an incorrect (and potentially broken) dependency resolution.
npm ERR!
npm ERR!

// even if I fix this one manually there are more dependencies not up to date that were as far as I can remember previously 
// included in updates:

(imported as 'concatLatestFrom') was not found in '@ngrx/effects' ([39m[22m[1m[31mpossible exports: Actions, EFFECTS_ERROR_HANDLER, EffectSources, EffectsFeatureModule, EffectsModule, EffectsRootModule, EffectsRunner, ROOT_EFFECTS_INIT, USER_PROVIDED_EFFECTS, act, createEffect, defaultEffectsErrorHandler, getEffectsMetadata, mergeEffects, ofType, provideEffects, rootEffectsInit)

[91merror[0m[90m TS2305: [0mModule '"@ngrx/effects"' has no exported member 'concatLatestFrom'.[39m[22m
[1m[31m[39m[22m
[1m[31m[7m5[0m import {Actions, concatLatestFrom, createEffect, ofType} from '@ngrx/effects';[39m[22m

Package Manager Version

No response

Operating System

[ ] macOS
[ ] Linux
[X] Windows
[ ] Other (Please specify)

Additional Information

Unfortunately I cannot provide an exact repo and nx-examples does not use ngrx-store and related packages to that extend.

But happy to help in case I can assist with any additional information.

leosvelperez commented 3 months ago

Thanks for reporting this!

Unfortunately, I can't reproduce the issue. Based on the provided information I tried:

created a fresh Nx 17.3.2 (the version you referred you were on) workspace
created an app and a lib
set up NgRx in both projects with the effect files importing concatLatestFrom from @ngrx/effects
ran nx migrate latest
ran nx migrate --run-migrations

After that, the effect files were correctly updated. The concatLatestFrom operator was changed to be imported from @ngrx/operators, and that package was installed in the workspace.

Please provide a reproduction so we can troubleshoot the issue.

colinscz commented 3 months ago

@leosvelperez thank you for trying this out. I am looking to do the same as there seems to be an issue with an older migration not being correctly applied. So before going to 17.3.2 I was on 16.4.0. I am now trying locally to go from 17.3.2 first to 18.3.5 before going to the latest version. If it's possible for you to share your tryout project that would be great.

I saw there were some old migrations added and ported back to include the @ngrx/operators package here: https://github.com/nrwl/nx/pull/21417 I am using ngrx but the operator package was never added in the past migrations. I usually just bump to the latest new released major version.

I don't know if it should have been automatically added by the ngrx 17 to ngrx 18 migration if it wasn't there before. At least that was my expectation. 🤔

So what I did for now I installed the @ngrx/operators package manually, bumping Nx to 18.3.5 and afterwards will try to go through with the update to 19.5.1.

I post again once I am done with that experiment.

colinscz commented 3 months ago

@leosvelperez just tried it out. Looks like the migration command (nx migrate latest) from 18.3.5 to 19.5.1 does not bump the @ngrx/operators package (if it is already listed as a dependency) to the same version but it does it correctly for all other @ngrx packages. 🤔 grafik

what I did next I bumped it manually to match the other @ngrx packages, executed the npm install and ran the migrations according to the Nx instructions:

 Next steps:

- Make sure package.json changes make sense and then run 'npm install',
- Run 'npx nx migrate --run-migrations'
- To learn more go to https://nx.dev/features/automate-updating-dependencies
- You may run 'npm run nx -- connect-to-nx-cloud' to get faster builds, GitHub integration, and more. Check out https://nx.app

But it looks like the issue is the same. The migration fails once it gets to the migration step for replacing the imports for @ngrx/effects to @ngrx/operators according to the output.

    [@ngrx/effects] Updated concatLatestFrom to import from '@ngrx/operators'
    [@ngrx/effects] Updated concatLatestFrom to import from '@ngrx/operators'
    [@ngrx/effects] Updated concatLatestFrom to import from '@ngrx/operators'

 NX   Failed to run ngrx-effects-migration-18-beta from @ngrx/effects. This workspace is NOT up to date!

 NX   Index 690 outside of range [0, 486].

I'll try to create a repro with the nx-examples repository in case your tryout project is not directly available. :)

More detailed logs, took some time because I had to replace the confidential stuff.

    [@ngrx/effects] Updated concatLatestFrom to import from '@ngrx/operators'
    [@ngrx/effects] Updated concatLatestFrom to import from '@ngrx/operators'
    [@ngrx/effects] Updated concatLatestFrom to import from '@ngrx/operators'

 NX   Failed to run ngrx-effects-migration-18-beta from @ngrx/effects. This workspace is NOT up to date!

 NX   Index 690 outside of range [0, 486].

Error: Index 690 outside of range [0, 486].
    at UpdateRecorderBase._assertIndex (C:\Develop\parent-project\child-project\nx-workspace\node_modules\@angular-devkit\schematics\src\tree\recorder.js:65:19)
    at UpdateRecorderBase.remove (C:\Develop\parent-project\child-project\nx-workspace\node_modules\@angular-devkit\schematics\src\tree\recorder.js:80:14)
    at createChangeRecorder (C:\Develop\parent-project\child-project\nx-workspace\node_modules\@ngrx\effects\schematics-core\utility\change.js:135:26)
    at commitChanges (C:\Develop\parent-project\child-project\nx-workspace\node_modules\@ngrx\effects\schematics-core\utility\change.js:157:20)
    at C:\Develop\parent-project\child-project\nx-workspace\node_modules\@ngrx\effects\migrations\18_0_0-beta\index.js:89:49
    at visitTSSourceFiles (C:\Develop\parent-project\child-project\nx-workspace\node_modules\@ngrx\effects\schematics-core\utility\visitors.js:66:22)
    at C:\Develop\parent-project\child-project\nx-workspace\node_modules\@ngrx\effects\migrations\18_0_0-beta\index.js:37:50
    at callRuleAsync (C:\Develop\parent-project\child-project\nx-workspace\node_modules\@angular-devkit\schematics\src\rules\call.js:77:24)
    at C:\Develop\parent-project\child-project\nx-workspace\node_modules\@angular-devkit\schematics\src\rules\call.js:73:40
    at Observable._subscribe (C:\Develop\parent-project\child-project\nx-workspace\node_modules\rxjs\dist\cjs\internal\observable\defer.js:8:31)
Command failed: npx nx _migrate --run-migrations --verbose

 NX   Command failed: npx nx migrate --run-migrations --verbose

Error: Command failed: npx nx migrate --run-migrations --verbose
    at checkExecSyncError (node:child_process:890:11)
    at execSync (node:child_process:962:15)
    at runNxSync (C:\Develop\APPDAT~1\21\tmp-39832-xycx77Sd445b\node_modules\nx\src\utils\child-process.js:27:34)
    at runMigrations (C:\Develop\APPDAT~1\21\tmp-39832-xycx77Sd445b\node_modules\nx\src\command-line\migrate\migrate.js:964:39)
    at C:\Develop\APPDAT~1\21\tmp-39832-xycx77Sd445b\node_modules\nx\src\command-line\migrate\migrate.js:1033:19
    at async handleErrors (C:\Develop\APPDAT~1\21\tmp-39832-xycx77Sd445b\node_modules\nx\src\utils\params.js:22:24)
Command failed: C:\develop\APPDAT~1\21\tmp-39832-xycx77Sd445b\node_modules\.bin\nx _migrate --run-migrations --verbose

colinscz commented 3 months ago

@leosvelperez seems like I found the issue: https://github.com/ngrx/platform/issues/4397 Since Nx is pointing to the version with the migration bug it causes issues during the upgrade. So I assume bumping the version to 18.0.1 fixes this.

I tried it out and damn it works. After bumping all @ngrx packages to v18.0.1 I have no issues running the migrations. 🥳 grafik

This probably means one or more bugfix releases for Nx so the broken Ngrx setup in v18.0.0 is not referenced anymore. Don't shoot the messenger 😄 😉

leosvelperez commented 3 months ago

@colinscz thanks for the detective work here! I'll add a migration to bump the version to 18.0.1 or later.

For some reason, the NgRx team doesn't include the @ngrx/operators in the @ngrx/store package group. We update all NgRx packages through that package group, which is supposed to list all the packages that update together. Given the package is not listed there, I'll add it to our migrations to ensure it's updated.

colinscz commented 3 months ago

@leosvelperez welcome man, thanks for the quick fix and happy to help :)

That is indeed weird. Not sure what the reason is. 🤷🏽‍♂️

github-actions[bot] commented 2 months ago

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.

nrwl / nx