-
Notifications
You must be signed in to change notification settings - Fork 1
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
Feature/angular 19 #91
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much for the extensive PR description - that really helped to understand the changes (apart from the the update itself).
As for this review: I can approve this PR as it is with the following suggestions:
- I'd say we remove the package
eslint-plugin-rxjs-angular
for now as you already suggested yourself. And re-add it in a separate branch which should update Eslint to 9. It would be wise to create this Eslint-Update branch ASAP - otherwise we'll forget about those rules. - Any reason that e.g.
@angular-eslint/schematics
was not updated to 19? Similar question with@angular-eslint/template-parser
,angular-oauth2-oidc
- Shouldn't
typescript
have been updated as well? - Fix the pipeline 😉
- Remove the following lines from the
.eslintrc.json
file. Otherwise Eslint isn't working anymore. At least on my machine.
{
"files": ["*.ts"],
"extends": ["plugin:@ngrx/recommended-requiring-type-checking"]
}
About your Angular 18 inspection comment: I couldn't reproduce this on my machine. My IDE already detects Angular 19 and does not show false positiv errors regarding e.g. wrong inputs on standalone components.
db0c51a
to
7c723df
Compare
@wendlans Thanks. I adjusted your feedback - I forgot about the Typescript cannot be updated because As for the pipelines - they cannot be fixed right now, there is another PR open which I will tackle in the next sprint, so this PR cannot be merged until then - or we just merge it, since the tests run locally ;) |
7c723df
to
7458c74
Compare
7458c74
to
a6b3336
Compare
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks all good now, thanks for the adjustments 👌
This PR holds the Angular 19 update alongside other dependencies (i.e. #62, #45, #70), because I did not want to pollute the renovate PR (and they all need to be within one and the same PR :)). It basically updates typescript, zone.js, Angular + all its libraries as well as ngrx.
Major changes:
standalone: true
is the new default, so the migration basically touched all our componentsI did some tests and it seems to work the same and look the same - but as always: Test it rigorously, since Material updates tend to break stuff as well, and I'm not sure I tested all edge cases with the new CSS fixes :)
Open issues
Even after clearing all caches, I still get Angular 18 inspections - this is most likely a configuration issue on my part, but be aware of it:

We cannot update
@ngrx/eslint-plugin
because it conflicts with a peer dependency ofeslint-plugin-rxjs-angular
. This dependency is also what restricts us from updating to eslint v9; so we essentially are forced to handle this now. Our options are:In my opinion, I think we should remove the plugin for now, merge this PR (with updated ngrx plugin) and then move to eslint v9 in a separate branch with one of the forks.