Skip to content

chore: 🤖 remove leaked radix ui types#848

Open
punkbit wants to merge 273 commits intomainfrom
chore/remove-leaked-radix-ui-types
Open

chore: 🤖 remove leaked radix ui types#848
punkbit wants to merge 273 commits intomainfrom
chore/remove-leaked-radix-ui-types

Conversation

@punkbit
Copy link
Collaborator

@punkbit punkbit commented Feb 25, 2026

Why?

Introduces click-ui's own DialogProps and DialogTriggerProps types, replacing direct Radix UI type re-exports (leaks). This decouples the public API from internal implementation details.

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

How?

  • Create custom click ui types
  • Export old leaks as deprecated

Preview?

N/A

punkbit added 27 commits March 4, 2026 16:25
…into chore/remove-leaked-styling-library-types-from-public-api
…into chore/remove-leaked-styling-library-types-from-public-api
…into chore/remove-leaked-styling-library-types-from-public-api
…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 2m 8s —— View job


Code Review

Nice work decoupling the public API from Radix UI internals! The approach is solid. There's one build-breaking bug to fix before merge.


🚨 Must Fix

  • src/index.ts — Duplicate exports: linkStyles and StyledLinkProps are exported twice (lines 300–307 and again 352–359). This will cause a TypeScript compilation error (Duplicate identifier). The second "Deprecated Exports" block at the bottom of the file is a copy-paste artifact and should be removed. Fix this →

⚠️ Potential Type Mismatch

  • FlyoutContentPropsonPointerDownOutside / onFocusOutside typed as CustomEvent: These are typed as (event: CustomEvent) => void in Flyout.types.ts, but they get spread via ...props into FlyoutContent (a styled(RadixDialog.Content)), which expects Radix-specific event types (PointerDownOutsideEvent, FocusOutsideEvent). With strictFunctionTypes, this may produce a TypeScript error. Consider either using any / Radix's types directly, or handling these in the component (like onInteractOutside) rather than forwarding them.

💡 Suggestions (non-blocking)

  • FlyoutHeaderProps / FlyoutFooterProps type discrepancy: Flyout.tsx defines and exports richer versions of these types (the union type TitleHeaderProps | ChildrenHeaderProps for FlyoutHeaderProps, and an Omit<ContainerProps<'div'>, ...> extension for FlyoutFooterProps), but Flyout/index.ts re-exports the simpler versions from Flyout.types.ts. Consumers end up with incomplete types. This is pre-existing, but this PR would be a good opportunity to consolidate — move the richer definitions to Flyout.types.ts. Fix this →

  • DialogContentProps still extends Radix's type: DialogContentProps in Dialog.types.ts still extends RadixDialog.DialogContentProps. Keeping it in scope for a follow-up is fine, just worth tracking as the next step in this cleanup.


The ThemeName narrowing improvement ((typeof THEMES)[keyof typeof THEMES] instead of a hardcoded union) is a nice bonus. 🙌

@workflow-authentication-public
Copy link
Contributor

📚 Storybook Preview Deployed

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

Built from commit: 8b8997688c8606ea8d82d734cf018e3c184e8a61

Base automatically changed from chore/public-api-as-exports-generator-source-of-truth to main March 6, 2026 13:58
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.

1 participant