ngrx / platform

Reactive State for Angular
https://ngrx.io
Other
7.96k stars 1.95k forks source link

17.2.0 -> 18.0.0 Schematics wreak havoc in files #4397

Closed spock123 closed 2 weeks ago

spock123 commented 3 weeks ago

Which @ngrx/* package(s) are the source of the bug?

schematics

Minimal reproduction of the bug/regression with instructions

We are trying to upgrade a larger application. The application is running Angular 18, NGRX 17.2 After performing a seemingly successful upgrade with ng update @ngrx/store , all imports in all files have errors and double entries.

Example:

A file using NGRX contains these imports (notice there is an import using path alias in the middle)

import {
    patchState,
    signalStore,
    withComputed,
    withHooks,
    withMethods,
    withState,
    getState
} from '@ngrx/signals';

import { ApiService } from '@shared/services/api';  // <-- import application using path alias
import { inject } from '@angular/core';
import { rxMethod } from '@ngrx/signals/rxjs-interop';
import { tapResponse } from '@ngrx/component-store';
import { pipe, switchMap, tap } from 'rxjs';

Now we run the migration:

We analyzed your package.json, there are some packages to update:

Name                                Version                  Command to update
 ---------------------------------------------------------------------------------
@ngrx/store                         17.2.0 -> 18.0.0         ng update @ngrx/store

 ng update @ngrx/store --force

Fetching dependency metadata from registry...
                  Package "ngrx-store-localstorage" has an incompatible peer dependency to "@ngrx/store" (requires "^17.0.0", would install "18.0.0").
    Updating package.json with dependency @ngrx/component-store @ "18.0.0" (was "17.2.0")...
    Updating package.json with dependency @ngrx/effects @ "18.0.0" (was "17.2.0")...
    Updating package.json with dependency @ngrx/router-store @ "18.0.0" (was "17.2.0")...
    Updating package.json with dependency @ngrx/store @ "18.0.0" (was "17.2.0")...
    Updating package.json with dependency @ngrx/store-devtools @ "18.0.0" (was "17.2.0")...
UPDATE package.json (4100 bytes)
✔ Packages successfully installed.
** Executing migrations of package '@ngrx/component-store' **

❯ As of NgRx v18, the `tapResponse` import has been removed from `@ngrx/component-store` in favor of the `@ngrx/operators` package.
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
    [@ngrx/component-store] Updated tapResponse to import from '@ngrx/operators'
UPDATE package.json (4134 bytes)
UPDATE src/app/features/accounting-transfers/accounting-transfers.store.ts (2993 bytes)
UPDATE src/app/features/accounting-transfers/components/edit-accounting-transfer/edit-accounting-transfer.store.ts (2487 bytes)
UPDATE src/app/features/administration/payment-transfers/payment-transfers.store.ts (2810 bytes)
UPDATE src/app/features/analyticsold/card-types/card-types.store.ts (3670 bytes)
UPDATE src/app/features/analyticsold/fresto-revenue/revenue.store.ts (4230 bytes)
UPDATE src/app/features/analyticsold/frontpage/analytics.store.ts (4334 bytes)
UPDATE src/app/features/analyticsold/payment-types/payment-types.store.ts (4754 bytes)
UPDATE src/app/features/analyticsold/slug-revenue/slug-revenue.store.ts (4617 bytes)
UPDATE src/app/features/companies/companies.store.ts (1515 bytes)
UPDATE src/app/features/company/company/company.store.ts (3526 bytes)
UPDATE src/app/features/company/company/components/company-stats/widget-revenue/revenue.store.ts (2069 bytes)
UPDATE src/app/features/company/company/dialogs/accounting-config/accounting-config.store.ts (3033 bytes)
UPDATE src/app/features/company/company/dialogs/cashdrawer/cashdrawer.store.ts (5828 bytes)
UPDATE src/app/features/company/company/dialogs/company-cache/company-cache.store.ts (2992 bytes)
UPDATE src/app/features/company/company/dialogs/company-terminal-credentials/company-terminal-credentials.store.ts (4623 bytes)
UPDATE src/app/features/company/company/dialogs/delete-company/delete-company.store.ts (2809 bytes)
UPDATE src/app/features/company/company/dialogs/edit-business-address/edit-business-address.store.ts (3548 bytes)
UPDATE src/app/features/company/company/dialogs/edit-business-data/edit-business-data.store.ts (3540 bytes)
UPDATE src/app/features/company/company/dialogs/edit-company-features/edit-company-features.store.ts (3635 bytes)
UPDATE src/app/features/company/company/dialogs/edit-company-status/edit-company-status.store.ts (3587 bytes)
UPDATE src/app/features/company/company/dialogs/edit-payment-config/edit-payment-config.store.ts (3878 bytes)
UPDATE src/app/features/company/company/dialogs/edit-vat/vat.store.ts (3935 bytes)
UPDATE src/app/features/company/company/dialogs/export-locations/export-locations.store.ts (4517 bytes)
UPDATE src/app/features/company/company/dialogs/import-bookings/import-bookings.store.ts (8021 bytes)
UPDATE src/app/features/create-company/create-company.store.ts (3046 bytes)
UPDATE src/app/features/logs/logs.store.ts (3382 bytes)
UPDATE src/app/features/parent-company/parent-company.store.ts (8150 bytes)
UPDATE src/app/features/parent-company/components/chains/chains.store.ts (3471 bytes)
UPDATE src/app/features/parent-company/components/chains/dialogs/chain/chain.store.ts (8522 bytes)
UPDATE src/app/features/parent-company/components/chains/dialogs/chain-members/chain-members.store.ts (10803 bytes)
UPDATE src/app/features/parent-company/components/edit-parent-company/edit-parent-company.store.ts (2652 bytes)
UPDATE src/app/features/parent-company/components/managers/managers.store.ts (2998 bytes)
UPDATE src/app/features/parent-company/components/managers/dialogs/create-edit-manager/create-edit-manager.store.ts (7684 bytes)
UPDATE src/app/features/reporting/z-reports.store.ts (2707 bytes)
UPDATE src/app/features/reporting/dialogs/report-details/report-details.store.ts (2521 bytes)
UPDATE src/app/features/terminals/create-edit-terminal/create-edit-terminal.store.ts (4812 bytes)
UPDATE src/app/features/terminals/delete-terminal/delete-terminal.store.ts (3404 bytes)
UPDATE src/app/features/terminals/move-terminal/move-terminal.store.ts (3868 bytes)
UPDATE src/app/shared/stores/config/config.store.ts (4482 bytes)
  Migration completed (40 files modified).

** Executing migrations of package '@ngrx/effects' **

❯ As of NgRx v18, the `concatLatestFrom` import has been removed from `@ngrx/effects` in favor of the `@ngrx/operators` package.
    [@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'
    [@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'
    [@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'
    [@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'
    [@ngrx/effects] Updated concatLatestFrom to import from '@ngrx/operators'
UPDATE package.json (4134 bytes)
UPDATE src/app/features/administration/administration.routes.ts (3058 bytes)
UPDATE src/app/features/company/company.routes.ts (1283 bytes)
UPDATE src/app/features/parent-companies/parent-companies.routes.ts (749 bytes)
UPDATE src/app/features/printers/printers.routes.ts (788 bytes)
UPDATE src/app/features/terminals/terminals.routes.ts (894 bytes)
UPDATE src/app/shared/store/index.ts (1720 bytes)
UPDATE src/app/shared/store/notifications/notifications.effects.ts (874 bytes)
UPDATE src/app/shared/store/parent-company/parent-company.effects.ts (3686 bytes)
UPDATE src/app/shared/store/printers/printers.effects.ts (7525 bytes)
UPDATE src/app/shared/store/router/router.effects.ts (851 bytes)
UPDATE src/app/shared/store/session/session.effects.ts (10125 bytes)
UPDATE src/app/shared/store/settings/settings.effects.ts (7209 bytes)
UPDATE src/app/shared/store/terminals/terminals.effects.ts (7913 bytes)
  Migration completed (14 files modified).

** Executing migrations of package '@ngrx/store' **

❯ As of NgRx v18, the `TypedAction` has been removed in favor of `Action`.
  Migration completed (No changes made).

After migration, the file now contains the following.

import {
    patchState,
    signalStore,
    withCoimport { ComponentStore } from '@ngrx/component-store';import { ComponentStore } from '@ngrx/component-store';import { ComponentStore } from '@ngrx/component-store';import { ComponentStore } from '@ngrx/component-store';import { ComponentStore } from '@ngrx/component-store';import { ComponentStore } from '@ngrx/component-store';import { ComponentStore } from '@ngrx/component-store';import { ComponentStore } from '@ngrx/component-store';import { ComponentStore } from '@ngrx/component-store';import { ComponentStore } from '@ngrx/component-store';import { ComponentStore } from '@ngrx/component-store';import { ComponentStore } from '@ngrx/component-store';import { ComponentStore } from '@ngrx/component-store';import { ComponentStore } from '@ngrx/component-store';import { ComponentStore } from '@ngrx/component-store';import { ComponentStore } from '@ngrx/component-store';import { ComponentStore } from '@ngrx/component-store';import { ComponentStore } from '@ngrx/component-store';import { ComponentStore } from '@ngrx/component-store';import { tapResponse } from '@ngrx/operators';
import { tapResponse } from '@ngrx/operators';
import { tapResponse } from '@ngrx/operators';
import { tapResponse } from '@ngrx/operators';
'@shared/services/api';
import { inject } from '@aimport { ComponentStore } from '@ngrx/component-store';import { tapResponse } from '@ngrx/operators';
import { tapResponse } from '@ngrx/operators';
import { tapResponse } from '@ngrx/operators';
xjs';

This happens to hundreds of files that contain NGRX imports.

Expected behavior

Migration should not mess up imports /s

Versions of NgRx, Angular, Node, affected browser(s) and operating system(s)

Ngrx: 17.2.0 Angular: 18.0.3 OS: MacOS Sonoma 14.2

Other information

No response

I would be willing to submit a PR to fix this issue

rainerhahnekamp commented 3 weeks ago

I can confirm that error; let me check. Maybe I can come with a PR

rainerhahnekamp commented 3 weeks ago

Quick update: The error comes when more than just one file is migrated. Expect fix PR soon.

spock123 commented 3 weeks ago

Thanks @rainerhahnekamp

pfeileon commented 2 weeks ago

In my case the migration fails with this error:

Migration failed: Index 500 outside of range [0, 388].

rainerhahnekamp commented 2 weeks ago

@pfeileon, can you please post the content of the file where that happened (as long as you are allowed to do that)? The PR is still open, so I can add a fix to it, if you have a case, we didn't think of.

pfeileon commented 2 weeks ago

@pfeileon, can you please post the content of the file where that happened (as long as you are allowed to do that)? The PR is still open, so I can add a fix to it, if you have a case, we didn't think of.

Sorry, I don't know which file causes this issue, it just happens after several lines of

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

but no file is actually changed.

angular-errors.log:

[error] Error: Index 500 outside of range [0, 388]. at UpdateRecorderBase._assertIndex (/home/pfeileon/git/my-app/node_modules/@angular-devkit/schematics/src/tree/recorder.js:65:19) at UpdateRecorderBase.remove (/home/pfeileon/git/my-app/node_modules/@angular-devkit/schematics/src/tree/recorder.js:80:14) at createChangeRecorder (/home/pfeileon/git/my-app/node_modules/@ngrx/effects/schematics-core/utility/change.js:138:26) at commitChanges (/home/pfeileon/git/my-app/node_modules/@ngrx/effects/schematics-core/utility/change.js:157:20) at /home/pfeileon/git/my-app/node_modules/@ngrx/effects/migrations/18_0_0-beta/index.js:89:49 at visitTSSourceFiles (/home/pfeileon/git/my-app/node_modules/@ngrx/effects/schematics-core/utility/visitors.js:66:22) at /home/pfeileon/git/my-app/node_modules/@ngrx/effects/migrations/18_0_0-beta/index.js:37:50 at callRuleAsync (/home/pfeileon/git/my-app/node_modules/@angular-devkit/schematics/src/rules/call.js:77:24) at /home/pfeileon/git/my-app/node_modules/@angular-devkit/schematics/src/rules/call.js:72:40 at Observable._subscribe (/home/pfeileon/git/my-app/node_modules/rxjs/dist/cjs/internal/observable/defer.js:8:31)


Btw, the update to v18 also causes issues with linting:

An unhandled exception occurred: Failed to load config "plugin:@ngrx/recommended-requiring-type-checking" to extend from.

But I guess that's a separate issue and might have to do with @angular-eslint/schematics and @typescript-eslint/utils (as there is a peer dependency issue).

rainerhahnekamp commented 2 weeks ago

@pfeileon, that error message comes from a file that is importing concatLatestFrom from @ngrx/effects. I don't know how large your application is, but if it is just a bunch of files importing that operator, you could send them to me via email as well. I can figure out if there is a bug.

My email address is straightforward: rainer.hahnekamp[at]gmail.com

pfeileon commented 2 weeks ago

I guessed as much ;-) but I'm not sure if that's helpful because all of the imports of concatLatestFrom have already correctly referenced @ngrx/operators before the update.

I checked the files and in all of them (5 in all, but the message in the console is printed 29 times) the import looks like this:

import { concatLatestFrom } from '@ngrx/operators';

The lines before and after are syntactically correct as well (only imports some with an empty line after).

The component tests are actually working and the app can be served locally so the issue doesn't seem to be harmful. I'm not sure if it's worth the trouble (I'd need to change the file contents first, check if the same error happens, etc.)...

rainerhahnekamp commented 2 weeks ago

I see. I think that the PR covers your error as well. In your case, you probably had a change somewhere and the migration script tried to apply those changes to all files with an import @ngrx/effects.

pfeileon commented 2 weeks ago

I see. I think that the PR covers your error as well. In your case, you probably had a change somewhere and the migration script tried to apply those changes to all files with an import @ngrx/effects.

The git status was clean if that's what you mean. But after a quick look, the code changes in the linked PR do have some additional checks which might prevent this issue.

nileshbhagwat commented 1 week ago

Hi @rainerhahnekamp,

I’m working on migrating from v17 to v18 and have encountered a blocking issue due to the issue mentioned by @pfeileon. Index 1829 outside of range [0, 1153]

NX Running migrations from 'migrations.json'

<!-- Multiple occurences of below statement -->
[@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 1829 outside of range [0, 1153].

Error: Index 1829 outside of range [0, 1153]. at UpdateRecorderBase._assertIndex (C:\Work\CS-Source\scui\node_modules\@angular-devkit\schematics\src\tree\recorder.js:65:19) at UpdateRecorderBase.remove (C:\Work\CS-Source\scui\node_modules\@angular-devkit\schematics\src\tree\recorder.js:80:14) at createChangeRecorder (C:\Work\CS-Source\scui\node_modules\@ngrx\effects\schematics-core\utility\change.js:138:26) at commitChanges (C:\Work\CS-Source\scui\node_modules\@ngrx\effects\schematics-core\utility\change.js:157:20) at C:\Work\CS-Source\scui\node_modules\@ngrx\effects\migrations\18_0_0-beta\index.js:89:49 at visitTSSourceFiles (C:\Work\CS-Source\scui\node_modules\@ngrx\effects\schematics-core\utility\visitors.js:66:22) at C:\Work\CS-Source\scui\node_modules\@ngrx\effects\migrations\18_0_0-beta\index.js:37:50 at callRuleAsync (C:\Work\CS-Source\scui\node_modules\@angular-devkit\schematics\src\rules\call.js:77:24) at C:\Work\CS-Source\scui\node_modules\@angular-devkit\schematics\src\rules\call.js:72:40 at Observable._subscribe (C:\Work\CS-Source\scui\node_modules\rxjs\dist\cjs\internal\observable\defer.js:8:31) error Command failed with exit code 1.

Could you please let me know when the fix for this will be published?

rainerhahnekamp commented 1 week ago

@brandonroberts I guess that's your call then 😉

brandonroberts commented 1 week ago

Thanks @rainerhahnekamp 🙂. 18.0.1 has been released with the migration fixes

nileshbhagwat commented 6 days ago

Thanks, @brandonroberts and @rainerhahnekamp. The patch works fine.