-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Update the Angular wrapper to work with Angular@16. #10396
Conversation
- Remove examples for angular@<12 - Add examples for angular@14-16 - Add examples for angular@next - Add missing types to Handsontable - Modify the helper scripts and GHA workflows - Regenerate package-lock
- Update package-lock
…rkflow." This reverts commit fb0edd2.
…dd changelog, correct a problem found by the code quality workflow.
…vents in the code examples workflow.
…ions/setup-node in the code examples workflow.
examples/next/docs/angular-14/basic-example/src/app/app.component.html
Outdated
Show resolved
Hide resolved
examples/next/docs/angular-15/basic-example/src/app/app.component.html
Outdated
Show resolved
Hide resolved
examples/next/docs/angular-next/basic-example/spec/support/jasmine.config.js
Outdated
Show resolved
Hide resolved
examples/next/docs/angular-next/basic-example/src/app/app.component.html
Outdated
Show resolved
Hide resolved
examples/next/docs/angular/basic-example/src/app/app.component.html
Outdated
Show resolved
Hide resolved
Co-authored-by: Krzysztof Budnik <571316+budnix@users.noreply.github.com>
…sontable into feature/dev-issue-1292
…t's using the correct version.
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 good 👌 I tested the bundle 0.0.0-next-44d778b-20230607 with simple apps that use Angular 12 and Angular 16, and I didn't find any issues.
…sontable into feature/dev-issue-1292
Launch the local version of documentation by running: npm run docs:review 80e131154ff3019cb77253b21135f08eb1a08317 |
"@angular/animations": "^16.0.0", | ||
"@angular/common": "^16.0.0", | ||
"@angular/compiler": "^16.0.0", | ||
"@angular/core": "^16.0.0", | ||
"@angular/forms": "^16.0.0", | ||
"@angular/platform-browser": "^16.0.0", | ||
"@angular/platform-browser-dynamic": "^16.0.0", | ||
"@angular/router": "^16.0.0", |
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.
For /next/
examples, shouldn't the "latest" tag be used here?
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.
Yup 9cab13b
@@ -92,12 +92,13 @@ const buildDependencyGetter = (version) => { | |||
rxjs: ['https://cdn.jsdelivr.net/npm/rxjs@6/bundles/rxjs.umd.js', [/* todo */]], | |||
'core-js': ['https://cdn.jsdelivr.net/npm/core-js@2/client/core.min.js', [/* todo */]], | |||
zone: ['https://cdn.jsdelivr.net/npm/zone.js@0.9/dist/zone.min.js', [/* todo */]], |
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.
WDYT to bump the zonejs
deps? I am thinking of bumping to the same version as in the wrapper deps ~0.11.4
.
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.
Seems like a good idea 👍 80e1311
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 good. Also, I've checked the Docs by deploying the latest changes to the staging, and it also looks good.
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.
I have verified that the build process using npm run all build
successfully builds the correct version and examples, including Angular 16.0.3 (angular-16 and currently angular-next).
The Handsontable-Angular integration is functional in environments built with Angular versions 12, 13, 14, 15 and 16. Tested with 0.0.0-next-44d778b-20230607.
Context
The main changes made in this PR:
Since the release of angular@16 the
@handsontable/angular
package was no longer compatible with the latest Angular release. This PR aims to change that.I moved the current wrapper logic to a new library template based on
@angular/cli@12
. The new wrapper build uses the Ivy compiler, and being built withangular@12
makes it compatible with Angular versions 12 up to 16 (currently the latest release).Unfortunately, it breaks the compatibility with Angular versions 11 and earlier.
It's worth noting, that Angular currently provides active support only for versions 14+.
Currently, the examples workflow does not test the angular-based examples properly.
When building the
/next/
section of the examples onlyhandsontable
is being symlinked in the examples'node_modules
. What's even worse, the wrapper version being used in the build and testing process is notlatest
, but12.0.1
.After the changes made in this PR, both
handsontable
and@handsontable/angular
should be linked properly in the examples build/test workflow.I removed the Angular 9-11 examples, as these versions are no longer compatible with the wrapper.
I added Angular 14-16 examples + created an
angular-next
section which builds and tests the wrapper against the version of Angular tagged asnext
.While rewriting the wrapper to later versions of Angular (mostly 16 and 12), I had to make some corrections regarding the typescript typing. There are places in the wrapper logic that would benefit from a bigger refactor, but I did not make any major changes as it was not in the scope of this task.
For the same reason as above, I had to make some changes to Handsontable, namely, adding some missing type definitions that were not tested with the wrapper based on Angular@9.
I reverted the temporary changes done in the release process of
12.4.0
, which purpose was to prevent the workflows from using the latestangular
version.I modified the
code-examples
workflow to utilize theactions/setup-node
's caching mechanism instead ofactions/cache
. The latter would break the new angular wrapper build + it makes sense to use the pre-built caching that is already being used in our other workflows.I had to make some modifications to other workflows, as the new set of dependencies being introduced with
@angular/cli@12
broke some things, like the documentation linter. This is a bigger issue regarding dependency management in the monorepo, which will have to be tackled at some point in the future.For the same shared-dependency-based reasons, I had to modify the
vue3
wrapper tests, as some of them no longer passed. (The changes make sense, as the types were mismatched in one of the tests, but it was not being analyzed before)How has this been tested?
next
.code-examples
workflow built the examples for the same versions on the CI.Types of changes
Related issue(s):
Affected project(s):
handsontable
@handsontable/angular
@handsontable/react
@handsontable/vue
@handsontable/vue3
Checklist: