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

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

Closed
3 of 7 tasks
d3lio opened this issue Jun 2, 2022 · 9 comments
Closed
3 of 7 tasks

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

d3lio opened this issue Jun 2, 2022 · 9 comments

Comments

@d3lio
Copy link

d3lio commented Jun 2, 2022

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:

  • v12.x
  • v13.x
  • Other:

Framework:

  • Angular
  • React
  • Vue
  • Other:

Framework version:
Angular 12

Device:
Any

bbogdanov added a commit to bbogdanov/ng-clarity that referenced this issue Jun 7, 2022
Closes vmware-clarity#147

Signed-off-by: Bogdan Bogdanov <bbogdanov@vmware.com>
@bbogdanov bbogdanov self-assigned this Jun 7, 2022
bbogdanov added a commit to bbogdanov/ng-clarity that referenced this issue Jun 7, 2022
Closes vmware-clarity#147

Signed-off-by: Bogdan Bogdanov <bbogdanov@vmware.com>
@ashleyryan
Copy link

@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

@kevinbuhmann
Copy link
Member

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

@ashleyryan
Copy link

oops, thanks. My bad

bbogdanov added a commit to bbogdanov/ng-clarity that referenced this issue Jun 13, 2022
Closes vmware-clarity#147

Signed-off-by: Bogdan Bogdanov <bbogdanov@vmware.com>
@d3lio
Copy link
Author

d3lio commented Jul 6, 2022

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

@kevinbuhmann
Copy link
Member

kevinbuhmann commented Jul 6, 2022

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.

@bbogdanov
Copy link
Contributor

@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?

@kevinbuhmann
Copy link
Member

kevinbuhmann commented Jul 7, 2022

@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 kevinbuhmann closed this as not planned Won't fix, can't repro, duplicate, stale Nov 14, 2022
@kevinbuhmann
Copy link
Member

kevinbuhmann commented Nov 14, 2022

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.

@github-actions
Copy link
Contributor

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 github-actions bot locked and limited conversation to collaborators Nov 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants