fix(link-with-icon): refactor LINK_SIZE enum #11071

andy-blum · 2023-10-27T16:21:00Z

Related Ticket(s)

Description

We've been digging into a sticky bug that we think we finally have an explanation for. To see the problem in action see this codepen.

As we understand it, this issue is:

originating in link.ts with the LINK_SIZE enum.
Then we use that enum to set a default value in link-with-icon.
Then we extend link-with-icon to card-footer
Then we extend card-footer to card-cta-footer
Then we extend DDSCardCTAFooter to CAEMCardCTAFooter

There are lots of other places in the code base that we use ENUMs that don’t cause issues, but this one does and as far as we can tell it’s the only one that:

is defined in CWC
is used to set a default value in C4IBM

Changelog

Changed

Refactor LINK_SIZE from enum to a union

ibmdotcom-bot · 2023-10-27T16:42:12Z

Deploy preview created for package React:
https://ibmdotcom-react.s3.us-south.cloud-object-storage.appdomain.cloud/deploy-previews/11071/index.html

Built with commit: 10794959219464696a2847ca1b62cef65d2370d5

ibmdotcom-bot · 2023-10-27T16:42:48Z

Deploy preview created for package Carbon Web Components:
https://carbon-web-components.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/11071/index.html

Built with commit: 10794959219464696a2847ca1b62cef65d2370d5

ibmdotcom-bot · 2023-10-30T13:44:23Z

Deploy preview created for package Web Components:
https://ibmdotcom-webcomponents.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/11071/index.html

Built with commit: 10794959219464696a2847ca1b62cef65d2370d5

ibmdotcom-bot · 2023-10-30T14:21:37Z

Deploy preview created for package Web Components HTML Test Application:
https://ibmdotcom-web-components-html-test-upstream.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/11071/index.html

Built with commit: 10794959219464696a2847ca1b62cef65d2370d5

ibmdotcom-bot · 2023-10-30T14:31:05Z

Deploy preview created for package Web Components (React wrapper):
https://ibmdotcom-web-components-react.s3-web.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/11071/index.html

Built with commit: 10794959219464696a2847ca1b62cef65d2370d5

jkaeser · 2023-10-30T14:46:09Z

packages/web-components/src/components/link-with-icon/link-with-icon.ts

@@ -50,7 +50,7 @@ class DDSLinkWithIcon extends StableSelectorMixin(BXLink) {
   * @internal
   */
  @property()
-  size = LINK_SIZE.LARGE;
+  size = 'lg' as LINK_SIZE;


Hmm, having to cast this string seems like a smell. Does the solution from https://dev.to/ivanzm123/dont-use-enums-in-typescript-they-are-very-dangerous-57bh obviate the need for this?

Updated to this method.

I had originally gone with the union instead to avoid having to import both an object and a type, but it seems that the type cascades through the inheritance chain on its own just fine, and if the object keeps the same name as the enum, we can make future refactors simpler.

jkaeser · 2023-10-30T16:51:00Z

packages/carbon-web-components/src/components/link/link-story.ts

+  [`Small size (sm)`]: 'sm',
+  [`Large size (lg)`]: 'lg',


Given #11071 (comment), what do you think about reverting this?

jkaeser · 2023-10-30T16:51:42Z

packages/carbon-web-components/src/components/link/link.ts

@@ -190,7 +182,7 @@ class BXLink extends FocusMixin(LitElement) {
   * Link size.
   */
  @property({ reflect: true })
-  size = LINK_SIZE.REGULAR;
+  size: LINK_SIZE_TYPE = '';


Same question here – can you go back to setting this to LINK_SIZE.REGULAR?

- @carbon/[email protected] - @carbon/[email protected] - @carbon/[email protected] - @carbon/[email protected] - @carbon/[email protected] - @carbon/[email protected] - @carbon/[email protected] - @carbon/[email protected] - @carbon/[email protected] - @carbon/[email protected] - @carbon/[email protected] - @carbon/[email protected]

This reverts commit af5f295.

) * fix(link-with-icon): refactor LINK_SIZE enum * fix(link-with-icon): export LINK_SIZE * fix(link-with-icon): change union to object-to-type * fix(link-with-icon): revert LINK_SIZE reference removals * chore(release): publish - @carbon/[email protected] - @carbon/[email protected] - @carbon/[email protected] - @carbon/[email protected] - @carbon/[email protected] - @carbon/[email protected] - @carbon/[email protected] - @carbon/[email protected] - @carbon/[email protected] - @carbon/[email protected] - @carbon/[email protected] - @carbon/[email protected] * Revert "chore(release): publish" This reverts commit af5f295. --------- Co-authored-by: ibmdotcom-bot <[email protected]>

andy-blum requested a review from a team as a code owner October 27, 2023 16:21

andy-blum requested review from kennylam and annawen1 October 27, 2023 16:21

jkaeser reviewed Oct 30, 2023

View reviewed changes

jkaeser approved these changes Oct 30, 2023

View reviewed changes

andy-blum added the hacktoberfest-accepted label Oct 31, 2023

IgnacioBecerra approved these changes Oct 31, 2023

View reviewed changes

andy-blum changed the base branch from main to release/v1.53.0 November 1, 2023 15:22

andy-blum and others added 5 commits November 1, 2023 11:27

fix(link-with-icon): refactor LINK_SIZE enum

ebd9686

fix(link-with-icon): export LINK_SIZE

473b896

fix(link-with-icon): change union to object-to-type

0dccd19

fix(link-with-icon): revert LINK_SIZE reference removals

c6f3573

andy-blum force-pushed the fix/enum-compilation-bug branch from 5443497 to af5f295 Compare November 1, 2023 15:27

Revert "chore(release): publish"

1079495

This reverts commit af5f295.

IgnacioBecerra added the Ready to merge Label for the pull requests that are ready to merge label Nov 1, 2023

IgnacioBecerra merged commit cf570e1 into carbon-design-system:release/v1.53.0 Nov 1, 2023

andy-blum deleted the fix/enum-compilation-bug branch November 1, 2023 20:37

andy-blum mentioned this pull request Nov 3, 2023

fix(link-with-icon): remove property set-by-enum #11092

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(link-with-icon): refactor LINK_SIZE enum #11071

fix(link-with-icon): refactor LINK_SIZE enum #11071

andy-blum commented Oct 27, 2023

ibmdotcom-bot commented Oct 27, 2023 •

edited

Loading

ibmdotcom-bot commented Oct 27, 2023 •

edited

Loading

ibmdotcom-bot commented Oct 30, 2023 •

edited

Loading

ibmdotcom-bot commented Oct 30, 2023 •

edited

Loading

ibmdotcom-bot commented Oct 30, 2023 •

edited

Loading

jkaeser Oct 30, 2023

andy-blum Oct 30, 2023

jkaeser Oct 30, 2023

jkaeser Oct 30, 2023

fix(link-with-icon): refactor LINK_SIZE enum #11071

fix(link-with-icon): refactor LINK_SIZE enum #11071

Conversation

andy-blum commented Oct 27, 2023

Related Ticket(s)

Description

Changelog

ibmdotcom-bot commented Oct 27, 2023 • edited Loading

ibmdotcom-bot commented Oct 27, 2023 • edited Loading

ibmdotcom-bot commented Oct 30, 2023 • edited Loading

ibmdotcom-bot commented Oct 30, 2023 • edited Loading

ibmdotcom-bot commented Oct 30, 2023 • edited Loading

jkaeser Oct 30, 2023

Choose a reason for hiding this comment

andy-blum Oct 30, 2023

Choose a reason for hiding this comment

jkaeser Oct 30, 2023

Choose a reason for hiding this comment

jkaeser Oct 30, 2023

Choose a reason for hiding this comment

ibmdotcom-bot commented Oct 27, 2023 •

edited

Loading

ibmdotcom-bot commented Oct 27, 2023 •

edited

Loading

ibmdotcom-bot commented Oct 30, 2023 •

edited

Loading

ibmdotcom-bot commented Oct 30, 2023 •

edited

Loading

ibmdotcom-bot commented Oct 30, 2023 •

edited

Loading