Skip to content
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

Support Angular 15 #1079

Closed
jattasNI opened this issue Feb 23, 2023 · 3 comments · Fixed by #1655
Closed

Support Angular 15 #1079

jattasNI opened this issue Feb 23, 2023 · 3 comments · Fixed by #1655
Labels
needs investigation Further research is needed to identify solution tech debt

Comments

@jattasNI
Copy link
Contributor

jattasNI commented Feb 23, 2023

🧹 Tech Debt

nimble-angular currently supports Angular 14 but the latest version is 15. We need to support the latest.

This is currently blocked on #810.

Once that is fixed we have two options:

  1. migrate Nimble to only support 15. This is what we have done historically.
  2. claim support for both 14 and 15. This should be possible now that both versions are on Ivy and it would make it easier for our clients to upgrade on their own cadence. But there are open questions:
  • would we compile using 14 and claim it's forwards compatible? or compile using 15 and claim it's backwards compatible? are both of these possible?
  • what testing do we need to do with each version? Probably both due to the issue below
  • will this be made more difficult because of us extending internal Angular APIs (Nimble Angular extends the Angular public API #732)

@TrevorKarjanis did some research on these options and believes that 2 is possible but he won't have bandwidth to drive progress for a while.

@jattasNI jattasNI added blocked Blocked on a third-party issue tech debt triage New issue that needs to be reviewed labels Feb 23, 2023
@m-akinc
Copy link
Contributor

m-akinc commented Feb 23, 2023

In Angular 15 they reimplemented RouterLink (including merging it with RouterLinkWithHref), which will be a problem for our Nimble versions of the router link directives (e.g. NimbleAnchorRouterLinkWithHrefDirective, NimbleAnchorButtonRouterLinkWithHrefDirective, NimbleAnchorLinkRouterLinkWithHrefDirective, and NimbleBreadcrumbItemRouterLinkWithHrefDirective). The new way it's implemented bakes in the notion of being for a and area elements only. Previously this assumption was just in the selector, which we were able to override by providing our own version that extended the original. Now there's not a clear way to reuse their implementation for our link-related elements.

@m-akinc m-akinc added needs investigation Further research is needed to identify solution and removed triage New issue that needs to be reviewed labels Feb 28, 2023
@PetitCalgon
Copy link

Angular 14 is now in LTS mode till november 2023.
https://angular.io/guide/releases#actively-supported-versions
An upgrade to Angular 15 or better Angular 16 should be considered quickly 😊

Thanks

@jattasNI
Copy link
Contributor Author

@PetitCalgon thanks for your interest! It's always a struggle for us to prioritize this sustaining work but we agree we should upgrade before 14 stops being supported. We'll find a way to get to it eventually but if you happen to have bandwidth on your team to help, we'd be happy to collaborate!

@m-akinc m-akinc removed the blocked Blocked on a third-party issue label Oct 17, 2023
@mollykreis mollykreis mentioned this issue Nov 7, 2023
1 task
mollykreis added a commit that referenced this issue Nov 28, 2023
# Pull Request

## 🤨 Rationale

Update to Angular 15.2 for nimble-angular.

Resolves #1079 
Resolves #1017 
Resolves #1570
Resolves #732
Resolves #1665 

## 👩‍💻 Implementation

- Update Angular version to 15.2 using Angular's upgrade utility
- Create forks of some of Angular's CVAs and Angular's RouterLink
directive under
`angular-workspace\projects\ni\nimble-angular\src\thirdparty\directives\`
with modifications to get them to work as expected as base classes for
nimble directives
- Add explicit dependency on `source-map-loader` to `nimble-components`
to ensure the tests continue to run as they previously did

## 🧪 Testing

- Ran the Angular app and verified it worked as expected
- Existing unit tests pass
- Copied & adapted Angular tests for CVAs and RouterLink directive into
`angular-workspace\projects\ni\nimble-angular\src\thirdparty\directives\tests\`
- Added new unit tests for each nimble CVA that verifies that
`ngModelChange` is only called once when a control's value changes
- Added new unit tests for each routerLink directive that verifies that
the href was sanitized
- Used the built package to verify it works correctly in
SystemLinkShared

## ✅ Checklist

<!--- Review the list and put an x in the boxes that apply or ~~strike
through~~ around items that don't (along with an explanation). -->

- [ ] I have updated the project documentation to reflect my changes or
determined no changes are needed.

---------

Co-authored-by: Mert Akinc <7282195+m-akinc@users.noreply.github.com>
Co-authored-by: Milan Raj <rajsite@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs investigation Further research is needed to identify solution tech debt
Projects
Development

Successfully merging a pull request may close this issue.

3 participants