bugfix(className): keep the sc-* hook class when a consumer styles a component (CUI-42)

[JeanMarcMilletScality]

TL;DR

Wrapping a core-ui component in styled() silently stripped its documented sc-* class from the DOM, so every CSS override, selector and trace lookup keyed on that class stopped matching. The class is now declared as the styled-components componentId instead of as a JSX prop, which puts it beyond the reach of a props spread. The emitted class is byte-identical, so nothing downstream changes.

Context

styled(Component) restyles by generating a class and passing it as className — so a consumer using the documented way to restyle was overwriting the hook class. Nothing failed loudly: the component still rendered and still looked right, which is why it went unnoticed. Surfaced downstream while tracing an e2e failure during a dependency uplift; see CUI-42.

Approach

className is an ordinary prop with exactly one slot, and two parties wanted to write it: the library (sc-modal) and the consumer (via styled() or directly). We wrote ours where it loses — a JSX spread is last-wins.

Before — the hook class in the contested slot:

const ModalContainer = styled.div`
  position: fixed;
  z-index: ${zIndex.modal};
`;

const Modal = ({ isOpen, title, role = 'dialog', className, ...rest }: Props) => (
  <ModalContainer
    className="sc-modal"   // ←  we write the className slot…
    role={role}
    {...rest}              // ←  …and the consumer overwrites it
  >
);

After — the hook class declared on the component, never passed as a prop:

const ModalContainer = styled.div.withConfig({
  componentId: 'sc-modal',   // ←
})`
  position: fixed;
  z-index: ${zIndex.modal};
`;

const Modal = ({ isOpen, title, role = 'dialog', ...rest }: Props) => (
  <ModalContainer role={role} {...rest}>   // ←  we never touch className
);

Only the consumer writes the className prop now, so there is nothing to clobber. sc-modal never enters the prop slot at all: styled-components emits it after props are resolved, in the same pass that appends the generated class and the folded ids.

Why componentId rather than merging the two by hand

Reordering the spread is not an option — it keeps sc-modal but drops the consumer's generated class, so styled() silently does nothing. One slot cannot hold two authors, so something has to concatenate. Three candidates:

	mechanism	verdict
merge helper at each site	ours	correct, but re-implements the merge at 23 sites and nothing stops site 24
.attrs({ className })	styled-components	works on 6.4.3, but undocumented for className, with a long history of ordering/dropping bugs across majors
withConfig({ componentId })	styled-components	typed and public; the identity ${Component} selectors already resolve to; cannot be clobbered

The library's documented contract is that a wrapped component "attach the passed className prop to a DOM element" — the third option satisfies it by never competing for the prop in the first place.

Measured

Real DOM output, styled(Button) inside a ButtonGroup (the group styles .sc-button to strip each child's framing, so a lost class silently detaches its CSS):

	sc-button present	group's hairline reaches it
before	❌	❌ silently no-ops
after	✅	✅ border-right: 0.0625rem

And the class is unchanged for everyone else, which is what makes this a non-breaking change:

render	emitted class
<Modal>	sc-modal cRtjOL
<Modal className="mine">	sc-modal cRtjOL mine
styled(Modal)	sc-modal cRtjOL sc-gWWYmq bDeILH

componentId alone emits exactly the literal string — no hash, no prefix. The babel plugin's displayName option would prefix it (ModalContainer-sc-modal), but this package builds with plain tsc and no babel-plugin-styled-components, so what is declared is what ships.

Usage

No API change. What the fix restores, as the regression test in Modal.test.tsx exercises it:

const WideModal = styled(Modal)`
  width: 80vw;
`;

Before, that dialog root carried only the generated classes. Now it carries sc-modal as well, so consumer CSS and selectors keyed on it keep matching through a styled() wrapper.

Review focus

• 🟡 src/lib/components/*/… › the 22 withConfig({ componentId }) declarations — the consumer's className now reaches the DOM only through the component's {...rest} spread, since nothing merges it any more. All 23 call sites were audited to confirm the spread lands on the DOM node; the bit worth confirming is that none was missed, because the failure mode is silent (class quietly absent, no error).
• 🟡 src/lib/components/error-pages/ErrorPageStyle.ts › the four wrappers — one shared ErrorPageContainer has to yield four distinct classes, so each page gets a thin styled(ErrorPageContainer).withConfig({ componentId }). This adds the base container's generated class alongside the page's own; verified each page emits its own class and not its siblings'.
• ⚪ src/lib/utils.ts › mergeClassNames removed — added earlier in this branch, unused once the class stops being a prop.

How to test

No screenshot: the change is a DOM class, not a visual, and the emitted class is deliberately identical. The meaningful check is that a styled() wrapper no longer detaches descendant CSS.

1. npm run storybook → Components / Button / Button Group.
2. In the story source wrap a child in styled(Button) (any rule, e.g. letter-spacing: 1px) and reload.
3. Inspect that button: it carries sc-button alongside the generated classes, and still renders de-framed — no border, transparent background, hairline separator from its neighbour.
4. On development/1.0 the same wrap drops sc-button, and the button visibly regains its own border and background inside the group.
5. npx jest — 44 suites / 436 tests, and npx tsc --noEmit clean.

Follow-up

• Internal descendant styling should move to component selectors. ButtonGroup, Navbar and Sidebar target children by writing .sc-button in CSS; styled-components' intended mechanism for that is ${ButtonStyled} { … }, which resolves to the same class and needs no naming convention. ButtonStyled is already exported and the repo already does this in Card and Tablestyle. Once internal styling stops depending on the classes, what remains is a deliberate public CSS API rather than a convention maintained by hand — which also settles whether test hooks should move to data-*. Own ticket, not this PR.
• Dropdown.component.tsx opens with // @ts-nocheck. Removing it surfaces 9 pre-existing errors, all the same shape: transient style props ($variant, $size, $isOpen, $isSelected, $text) read in CSS interpolations on styled definitions that were never given a generic type parameter. Worth typing properly — a blanket @ts-nocheck hides real API defects, which is exactly what happened to className here. Separate scope from this fix.
• Tests should not select on sc-* classes. Out of scope here, but worth making the rule: assertions belong on user-facing behaviour, and test hooks on data-* attributes.
• role / aria-* are exposed the same way. CUI-42 notes that {...rest} still lands after role, aria-modal, aria-labelledby and aria-describedby, so a consumer can override core-ui's accessibility wiring. Deliberately unchanged: this PR alters no prop precedence, and fixing it would change override behaviour across 21 components — a separate decision from the class bug.

References

• CUI-42 — "A consumer className silently clobbers the internal sc-* hook class". Bug / Severity Major / Impact Internal. Carries the downstream report, the original DOM evidence, and the merge-based fix this supersedes.

What changed

22 componentId declarations across 19 styled containers, covering every sc-* hook class the library exposed through a JSX prop: sc-breadcrumb, sc-button, sc-button-group, sc-circularprogressbar, sc-dropdown, sc-error-page401/404/500/auth, sc-lateralnavbarlayout, sc-layout, sc-loader, sc-modal, sc-navbar, sc-notification, sc-notifications, sc-progressbar, sc-searchinput, sc-sidebar, sc-sidebar-item, sc-steppers, sc-textarea. Each site drops its hardcoded className and stops destructuring className out of props, so the consumer's class flows through the existing spread untouched.

className?: string is added to 19 Props types that never declared it. That is a separate, compile-time defect: those components already forwarded className to a DOM element, but tsc rejected <Comp className="x" /> (TS2322) and <Styled className="x" /> (TS2769). The clobber itself was runtime-only — styled(Comp) with no explicit className compiled fine. Dropdown was the one component where this was easy to miss: the file opens with // @ts-nocheck, so tsc never reads it and the omission raised nothing locally while still breaking consumers in type-checked files. A type-level probe over all 21 components confirms every one now declares it.

CUI-42 proposed merging the classes and moving {...rest} ahead of the props core-ui owns. The merge is superseded by componentId; the reordering is deliberately left out, since it changes prop precedence for role and the aria-* attributes too.

The regression test is a sibling describe in the existing Modal.test.tsx, not a second test file for the same component.

Deliberately not in this PR: converting internal descendant styling to component selectors, any data-* test hooks, and the role/aria-* precedence question.

🤖 Generated with Claude Code

[bert-e]

Hello jeanmarcmilletscality,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Available options

name	description	privileged	authored
/after_pull_request	Wait for the given pull request id to be merged before continuing with the current one.
/bypass_author_approval	Bypass the pull request author's approval	⭐
/bypass_build_status	Bypass the build and test status	⭐
/bypass_commit_size	Bypass the check on the size of the changeset TBA	⭐
/bypass_incompatible_branch	Bypass the check on the source branch prefix	⭐
/bypass_jira_check	Bypass the Jira issue check	⭐
/bypass_peer_approval	Bypass the pull request peers' approval	⭐
/bypass_leader_approval	Bypass the pull request leaders' approval	⭐
/approve	Instruct Bert-E that the author has approved the pull request.		✍️
/create_pull_requests	Allow the creation of integration pull requests.
/create_integration_branches	Allow the creation of integration branches.
/no_octopus	Prevent Wall-E from doing any octopus merge and use multiple consecutive merge instead
/unanimity	Change review acceptance criteria from one reviewer at least to all reviewers
/wait	Instruct Bert-E not to run until further notice.

Available commands

name	description	privileged
/help	Print Bert-E's manual in the pull request.
/status	Print Bert-E's current status in the pull request.
/clear	Remove all comments from Bert-E from the history TBA
/retry	Re-start a fresh build TBA
/build	Re-start a fresh build TBA
/force_reset	Delete integration branches & pull requests, and restart merge process from the beginning.
/reset	Try to remove integration branches unless there are commits on them which do not appear on the source branch.

Status report is not available.

[bert-e]

Waiting for approval

The following approvals are needed before I can proceed with the merge:

• the author

• one peer

Peer approvals must include at least 1 approval from the following list:

• @ZiyangLinScality

• @damiengillesscality

• @JeanMarcMilletScality

• @JBWatenbergScality

• @PierreAlexandreDelgado

• @ChengYanJin

• @sanassicisse

• @hervedombya

• @MonPote

[bert-e]

Waiting for approval

The following approvals are needed before I can proceed with the merge:

• the author

• one peer

Peer approvals must include at least 1 approval from the following list:

• @hervedombya

• @PierreAlexandreDelgado

• @ZiyangLinScality

• @JBWatenbergScality

• @MonPote

• @ChengYanJin

• @sanassicisse

• @JeanMarcMilletScality

• @damiengillesscality

[claude]

Modal.test.tsx already exists in this directory (testing stacking order). Adding Modal.component.test.tsx creates two test files for the same module — the exact duplication the project's CLAUDE.md convention warns against:

"Two names for one component (Modal.test.tsx and Modal.component.test.tsx) is the failure this prevents."

Add the new describe('className preservation', …) block inside the existing Modal.test.tsx instead.

[claude]

className?: string is missing from Props. Every other component in this PR adds it to its Props type, but Dropdown was skipped. The // @ts-nocheck at the top masks the omission locally, but consumers in type-checked files writing <Dropdown className="x" /> will get TS2322.

Suggested change

	};
	};
	const DropdownStyled = styled.div.withConfig({

(Can't suggest the Props change in this hunk — add className?: string; to the Props type on line 20, alongside the other fields.)