-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
17.2.0 -> 18.0.0 Schematics wreak havoc in files #4397
Comments
I can confirm that error; let me check. Maybe I can come with a PR |
Quick update: The error comes when more than just one file is migrated. Expect fix PR soon. |
Thanks @rainerhahnekamp |
In my case the migration fails with this error:
|
@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
but no file is actually changed. angular-errors.log:
Btw, the update to v18 also causes issues with linting:
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). |
@pfeileon, that error message comes from a file that is importing My email address is straightforward: rainer.hahnekamp[at]gmail.com |
I guessed as much ;-) but I'm not sure if that's helpful because all of the imports of 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:
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.)... |
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 |
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. |
Hi @rainerhahnekamp, I’m working on migrating from v17 to v18 and have encountered a blocking issue due to the issue mentioned by @pfeileon.
Could you please let me know when the fix for this will be published? |
@brandonroberts I guess that's your call then 😉 |
Thanks @rainerhahnekamp 🙂. |
Thanks, @brandonroberts and @rainerhahnekamp. The patch works fine. |
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)
Now we run the migration:
After migration, the file now contains the following.
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
The text was updated successfully, but these errors were encountered: