Skip to content

chore: 🤖 remove leaked styling library types from public api#847

Merged
punkbit merged 297 commits intomainfrom
chore/remove-leaked-styling-library-types-from-public-api
Mar 6, 2026
Merged

chore: 🤖 remove leaked styling library types from public api#847
punkbit merged 297 commits intomainfrom
chore/remove-leaked-styling-library-types-from-public-api

Conversation

@punkbit
Copy link
Collaborator

@punkbit punkbit commented Feb 25, 2026

Why?

Removed StyledLinkProps and linkStyles from the public API to prevent leaking styled-components implementation details, e.g., the $size and $weight transient props. The Link component already accepts a component prop to render as any element type, has size and weight props, and forwards all other props, e.g. onClick, etc.

⚠️ WARNING: Depends on #845 which should be merged first
🤖 On #845 merge, change base branch to main

How?

  • Move/List as deprecated
  • Provide changeset migration details

Preview?

N/A

Base automatically changed from chore/public-api-as-exports-generator-source-of-truth to main March 6, 2026 13:58
…into chore/remove-leaked-styling-library-types-from-public-api
@github-actions
Copy link

github-actions bot commented Mar 6, 2026

Claude finished @punkbit's task in 1m 14s —— View job


PR Review

Clean approach — moving deprecated exports into a clearly labeled section with JSDoc and a migration guide is exactly right. The changes in src/index.ts are well-structured.

Two items from the prior review are still unaddressed:


🚨 Must Fix

  • Changeset bump level (.changeset/cool-onions-wink.md:2): Still set to minor. Since this PR only adds @deprecated JSDoc annotations without removing any exports, this is a patch bump per semver. minor implies new backward-compatible functionality.

    Fix this →

  • Changeset description (.changeset/cool-onions-wink.md:5): Still says "Removed StyledLinkProps and linkStyles from the public API" — misleading in the generated CHANGELOG since both are still exported. Should say "Deprecated".

    Fix this →


⚠️ Worth checking (from merged #845 base)

  • fiery-shirts-brush.md bump level: The merged-in changeset removes actual subpath exports (@clickhouse/click-ui/Collapsible, @clickhouse/click-ui/IconWrapper, @clickhouse/click-ui/MiddleTruncator) but is set to minor. Removing a previously accessible import path is a breaking change — consumers importing from those subpaths will get a runtime error. This likely warrants major unless those paths were never documented as stable. Worth a deliberate call before merge.

💡 Suggestions (non-blocking)

  • LinkProps not exported (src/index.ts:155): Link is exported but LinkProps isn't. The migration guide recommends <Link component={RouterLink} ...>, but consumers who want to type their own wrapper have no type to reference. Pre-existing gap, but this PR is touching the Link section — good opportunity to add it.

@github-actions
Copy link

github-actions bot commented Mar 6, 2026

Claude finished @punkbit's task in 1m 2s —— View job


PR Review

Both issues flagged in the previous review have been addressed — nice work! 🙌

The changeset (cool-onions-wink.md) is now correct:

  • Bump type is patch
  • Summary says "Deprecated" (not "Removed") ✅

The deprecated exports in src/index.ts:306-317 are well-structured — clearly sectioned, with accurate @deprecated JSDoc tags and concrete migration examples.


⚠️ Worth checking (from merged #845 base)

  • fiery-shirts-brush.md:2 bump level: The merged-in changeset removes actual subpath exports (@clickhouse/click-ui/Collapsible, @clickhouse/click-ui/IconWrapper, @clickhouse/click-ui/MiddleTruncator) but is set to minor. Removing a previously accessible import path is a breaking change — consumers importing from those subpaths will get a runtime error. This likely warrants major unless those paths were never documented as stable. Worth a deliberate call before merge.

💡 Suggestions (non-blocking)

  • LinkProps not exported (src/index.ts:155): Link is exported but LinkProps isn't. The migration guide recommends <Link component={RouterLink} ...>, but consumers who want to type their own wrapper have no type to reference. This PR touches the Link section — good opportunity to add it if desired.

  • Trailing note in changeset (cool-onions-wink.md:43): The line "These deprecated APIs will be removed in a future minor release" is inconsistent — they should be removed in a future major release since removal is a breaking change. Consider changing to "future major release" or just "future release".

    Fix this →


✅ The core goal of this PR is solid — deprecations are clean, backward-compatible, and well-documented. Ready to merge once the fiery-shirts-brush.md bump level question is resolved.

@punkbit
Copy link
Collaborator Author

punkbit commented Mar 6, 2026

⚠️ Worth checking (from merged #845 base)

  • fiery-shirts-brush.md:2 bump level: The merged-in changeset removes actual subpath exports (@clickhouse/click-ui/Collapsible, @clickhouse/click-ui/IconWrapper, @clickhouse/click-ui/MiddleTruncator) but is set to minor. Removing a previously accessible import path is a breaking change — consumers importing from those subpaths will get a runtime error. This likely warrants major unless those paths were never documented as stable. Worth a deliberate call before merge.

These are not consumed and shouldn't be exported in the first place. Ignore.

@github-actions
Copy link

github-actions bot commented Mar 6, 2026

Claude finished @punkbit's task in 43s —— View job


PR Review

All previously flagged items have been addressed — great work! 🙌

  • ✅ Changeset bump is patch
  • ✅ Summary says "Deprecated" (not "Removed")
  • ✅ Note correctly reads "future major release"
  • LinkProps is now exported at src/index.ts:157
  • linkStyles and StyledLinkProps moved to a clearly labeled deprecated section with accurate @deprecated JSDoc and concrete migration examples

The fiery-shirts-brush.md bump level concern (subpath export removals) has been acknowledged by the author as intentional — those paths were never meant to be consumed publicly.

No remaining issues. This PR is ready to merge. ✅

@github-actions
Copy link

github-actions bot commented Mar 6, 2026

Claude finished @punkbit's task in 33s —— View job


PR Review

✅ All previous issues have been addressed — this is clean and well-executed!

The PR correctly:

  • Moves linkStyles and StyledLinkProps to a clearly labelled Deprecated Exports section at the bottom of src/index.ts
  • Adds @deprecated JSDoc with concrete migration examples on both exports
  • Adds LinkProps to the public API (src/index.ts:156) — consumers can now properly type Link wrappers
  • Changeset is patch, summary says "Deprecated" (not "Removed"), and the note correctly references a future major release

💡 Suggestions (non-blocking)

  • LinkProps re-export placement (src/index.ts:156): LinkProps is now exported in the main Label & Link section, which is correct. Just a heads-up that this addition is a net improvement for consumers who were previously unable to type their own wrappers around Link.

✅ No blocking issues. Looks great — nice work! 🙌

@workflow-authentication-public
Copy link
Contributor

📚 Storybook Preview Deployed

✅ Preview URL: https://click-j8zjn42as-clickhouse.vercel.app

Built from commit: 71335e42915ef4ee18d70fb8633eb993b3ae8979

@punkbit punkbit merged commit 5831d60 into main Mar 6, 2026
8 checks passed
@punkbit punkbit deleted the chore/remove-leaked-styling-library-types-from-public-api branch March 6, 2026 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants