ng-clarity icon indicating copy to clipboard operation
ng-clarity copied to clipboard

Allow @clr/ui@12 to use @cds/core@6

Open d3lio opened this issue 2 years ago • 7 comments

Describe the bug

Currently in our project we use Clarity UI 12 with CDS Core 6 as we want to utilise the theming capabilities of Core 6 with the variables shim without upgrading Angular and the Clarity components. The issue is that we need to have two dependencies - @cds/core 5 and 6 because @clr/ui 12 has a peer dependency @cds/core 5 which we don't use.

Expected behavior

@clr/ui@12 should have a peer dependency @cds/core@>=5 since CDS Core is aiming to have backwards compatibility with older versions of @clr/ui.

Versions

Clarity version:

  • [x] v12.x
  • [x] v13.x
  • [ ] Other:

Framework:

  • [x] Angular
  • [ ] React
  • [ ] Vue
  • [ ] Other:

Framework version: Angular 12

Device: Any

d3lio avatar Jun 02 '22 08:06 d3lio

@bbogdanov your PR is not what this issue is about. The default size of icons (when not including the core css) changed in v6 compared to v5

ashleyryan avatar Jun 07 '22 15:06 ashleyryan

It looks like the icon issue is 138, not this issue.

kevinbuhmann avatar Jun 07 '22 16:06 kevinbuhmann

oops, thanks. My bad

ashleyryan avatar Jun 07 '22 16:06 ashleyryan

Hey guys can we get an update on this? @ashleyryan @steve-haar @kevinbuhmann

d3lio avatar Jul 06 '22 14:07 d3lio

My vote is that we don't do this.

  1. Angular 12 is no longer under active support by the Angular team at Google, and backporting changes sometimes fails due to bugs in the legacy View Engine compiler.
  2. Ensuring that @clr/angular works with @cds/core@5 and @cds/core@6 is non-trivial. We have already been burned at least once on this and had to revert a new feature.

kevinbuhmann avatar Jul 06 '22 17:07 kevinbuhmann

@kevinbuhmann Even that Angular team doesn't actively support, the company does. Another way to look into this is to figure out a way to port the theming capabilities.

The feature you reverted used a code that was part of v6 but not v5 which we know caused issues. Since that feature is not in the code the only thing left used between the two packages are the icons that haven't changed in between v5 and v6.

So what's not trivial about it?

bbogdanov avatar Jul 07 '22 10:07 bbogdanov

@bbogdanov, it's non-trivial to ensure that we don't accidentally break something for @cds/core@5 users because we develop against @cds/core@6. After I posted my earlier comment, I found a way to build PRs against @cds/core@5 in CI, but I haven't discussed that with the team yet. It should help prevent us from getting burned again, but it's not a complete solution as there are no visual checks.

I will discuss using @cds/core@6 with @clr/angular@13 with the team next week.

But long term, the solution is to update to Angular 13 or 14. My alternative to the a11y fix that was reverted due to this issue seemingly cannot be backported because of a weird bug in the legacy View Engine compiler. I wouldn't be surprised if that happens with other fixes as well which would leave Angular 12 users further behind.

kevinbuhmann avatar Jul 07 '22 13:07 kevinbuhmann

The solution here is to use a supported version of Angular. Angular 12 is not even receiving security updates from Google anymore, so it should no longer be used in products.

kevinbuhmann avatar Nov 14 '22 16:11 kevinbuhmann

Hi there 👋, this is an automated message. To help Clarity keep track of discussions, we automatically lock closed issues after 14 days. Please look for another open issue or open a new issue with updated details and reference this one as necessary.

github-actions[bot] avatar Nov 29 '22 01:11 github-actions[bot]