Feature/upgrade to angular 19 #2395
Conversation
✅ Deploy Preview for jsonforms-examples ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
… Angular 18 and 19.
There was a problem hiding this comment.
Hi @laurentgrangier , thanks for this PR ❤️
Many of the changes already look quite good. There are some minor comments inline.
One bigger issue seems to be the coverage reporting for the angular-material package. Executing pnpm run test-cov in the angular-material package no longer creates an lcov.info file. This is needed for coverage reports in PRs. You can see the expected result by executing tests on the main branch. For the angular package this still works.
My assumption is that there was some config in the deleted test-runner.js that configured this. Please have a look.
| "serve": { | ||
| "builder": "@angular-devkit/build-angular:dev-server", | ||
| "configurations": { | ||
| "production": { | ||
| "buildTarget": "example:build:production" | ||
| }, | ||
| "development": { | ||
| "buildTarget": "example:build:development" | ||
| } | ||
| }, | ||
| "defaultConfiguration": "development" | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
I think we can remove this section. The used build targets example:build:production and example:build:development do not exist in our package.json. Instead, the dev script builds the example app and serves it via http-server.
There was a problem hiding this comment.
You are right. I just copied them from the default generate file from Angular. I removed them.
| @@ -0,0 +1,87 @@ | |||
| { | |||
| "$schema": "./node_modules/@angular/cli/lib/config/schema.json", | |||
There was a problem hiding this comment.
In our mono-repo, the file is actually located in the root node_modules folder. Thus, I suggest adapting this like the following. With this change, autocompletion worked for me in vscode when opening the full jsonforms repo.
| "$schema": "./node_modules/@angular/cli/lib/config/schema.json", | |
| "$schema": "../../node_modules/@angular/cli/lib/config/schema.json", |
… config (needed for test coverage), remove unnecessary ns serve options.
@lucas-koehler The karma.config.js was missing in angular.json. Adding it properly should fix the missing lcov file. Can you please check? I'm not sure where I should see them in the build. |
There was a problem hiding this comment.
Hi @laurentgrangier ,
thanks for the updates. This LGTM now. Thanks for all your work ❤️
I tested this locally with Angular 18 and 19 in the angular seed. Both worked fine :)
Regarding the code coverage: Executing the tests locally I could see that the code coverage files are now generated at expected location packages/angular-material/coverage/lcov.info.
Apparently, there is another issue with the coverage report not being displayed in the PR as this is also the case for another, independent PR. Thus, nothing additional needs to be done here.
Note that I will close this PR and open a followup one because we generally do not accept re-generated lock file contributions from third party contributors for security reasons. I could not push a re-generated one on your branch as this is disabled in your fork's settings. Thus, I'll push this to a branch here and open a new PR from there.
|
Superseded by #2403 . Thanks again @laurentgrangier for this great PR :) |
|
Thank you for the review and the new PR. 👍 |
#2386