refactor: 💡 modular file architecture (first pass)

[punkbit]

Why?

Provide an elegant file architecture pattern inspired by major component libraries. It has a main component, whose name serves as a namespace for types, styles, tests, stories (storybook) and a public export file. This is a first pass; further passes and iterations will be required, which is done to lower the risk of breaking changes.

⚠️ WARNING: Depends on #798 which should go first
🤖 TODO: once #798 merged change base to main branch

It aims to provide the following:

components/
├── Button/
 │   ├── Button.tsx
 │   ├── Button.types.ts
 │   ├── Button.styles.ts
 │   ├── Button.test.tsx
 │   ├── Button.stories.tsx
 │   └── index.ts
├── Input/
 │   ├── Input.tsx
 │   ├── index.ts
 │   a└── ...
└── index.ts

How?

• Rename/Move files (file architecture)
• Prefer relative path where applicable

Preview?

Button

src/components/Button
├── Button.stories.tsx
├── Button.test.tsx
├── Button.tsx
└── index.ts

1 directory, 4 files

Primary card

src/components/CardPrimary
├── CardPrimary.stories.ts
├── CardPrimary.test.tsx
├── CardPrimary.tsx
├── CardPrimaryTopBadge.tsx
├── index.ts
└── withTopBadge.tsx

1 directory, 6 files

Toast

src/components/Toast
├── Toast.stories.tsx
├── Toast.tsx
├── index.ts
├── toastEmitter.ts
└── useToast.tsx

1 directory, 5 files

[punkbit]

Do all types go in the $component.types.ts file? What if they're not meant to be exported from a component?

If that helps.

So, if you're declaring a types file, that'd be component.types.ts and not types.ts.

At the moment, we shouldn't have cross-type imports, unless I miss something in this pass, let me know if you found something.

For internal exports, you have control over the types you want exposed in the ./index.js.
For public exports, you have control over the types under src/index.ts (this is available in a related PR).

If you'd find a situation where a particular type is shared (which I found in a few cases like that), you'd probably be interested in @/types, e.g. IconSize (in the next PR, that's AssetSize), which effectively replaces the src/components/commonTypes.ts and patterns like import and export which caused circular dependencies as seen here https://github.com/ClickHouse/click-ui/blob/b0fd26debc2c1e6da9cb3685fae862ea8cb7c7a1/src/components/types.ts as seen in more details in circular dependency resolution PR here #841

What if we have multiple files in a directory, and only one of them makes use of the type? Does that type need to be moved to a separate file? Can't it just be exported directly from the index.ts file with the same effect?

At the component level, you're not allowed to import from ./index that's related to the safeguards for circular dependencies. I'm not sure if I'd block your PR because of an inline or local type? But if you do create a types file, that should be component.types.ts and not types.ts, e.g. we have component.test.ts, component.stories.ts, we'll have something similar for styles

My concern is that rigid expectations will create a lot of work and boilerplate. If we only have a single component in a directory, having a separate file for types is nice for consistency, but it splits up all the context among multiple files somewhat needlessly

That's fine, if you don't need a types file and wish to declare it inline or locally. But as the project grows, and you need one then I guess for consistency you'd use component.types.ts.

[github-actions]

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

---

Code Review

Great structural work here — the modular file pattern is clean, consistent, and aligns with how major component libraries organize their code. The migration is well-scoped as a first pass and the new src/types/index.ts and src/components/Common/ consolidation are solid improvements. 🙌

---

🚨 Must Fix

• ContextMenu: ContextMenuItemProps is declared twice in ContextMenu.tsx (lines 15 and 183), and the exported version in ContextMenu.types.ts is missing type?: 'default' | 'danger'. TypeScript will merge the two declarations in the same file, but the public-facing type exported via index.ts → ContextMenu.types.ts is incomplete. Any consumer importing ContextMenuItemProps from the package loses the type prop for danger-styled items — a silent API regression.

  The new interface block added at the top of ContextMenu.tsx (lines 11–18) duplicates the already-correct one at line 183. The fix is to add type?: 'default' | 'danger' to ContextMenu.types.ts and remove the duplicate declaration in ContextMenu.tsx. Fix this →

---

💡 Suggestions (non-blocking)

• Button.tsx:7: type Alignment = 'center' | 'left' is declared locally in Button.tsx but the intent of this PR is to move types to Button.types.ts. It's not exported so it's not a bug, but it's a minor consistency gap.

• Accordion.types.ts: Size, Gap, and Color are exported from the types file (line 6–8) but are internal implementation details. Worth considering whether they should be part of the public API via index.ts. For now, index.ts only exports AccordionProps which is correct — just something to revisit intentionally.

• Changeset (silly-cups-arrive.md): Marked as minor which is appropriate for this structural refactor. The description is clear about intent. A note about the ContextMenu.types.ts gap (once fixed) would be helpful for future pass tracking.

---

Overall this is a well-executed, careful refactor. The one ContextMenuItemProps regression needs to be fixed before merge, but the architecture direction is spot-on. 👍

[github-actions]

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

---

Code Review

Great structural work — the modular file pattern is clean, consistent, and well-executed for a first pass. The architecture makes components much easier to navigate. 🙌

---

🚨 Must Fix

• ContextMenu: ArrowProps and ContextMenuItemProps are still exported from both ContextMenu.tsx and ContextMenu.types.ts. The index.ts correctly re-exports from ContextMenu.types.ts, so the public API is fine — but ContextMenu.tsx shouldn't be the origin of these exports. The intended pattern is for ContextMenu.tsx to import from ./ContextMenu.types, not re-declare the same interfaces. As-is, the types have two canonical sources within the package, which is confusing.

  Fix: in ContextMenu.tsx, remove the export interface ArrowProps block (lines 11–13) and export interface ContextMenuItemProps block (lines 178–185), and instead import both from './ContextMenu.types'. Fix this →

---

💡 Suggestions (non-blocking)

• Accordion.types.ts: Size, Gap, and Color are exported from the types file (lines 6–8) but don't appear in the component's index.ts export, so they're not part of the public API — which is correct. Worth a quick comment on the type or a note in a follow-up pass to decide if they should ever be public (e.g., for consumers building custom accordion variants).

• Common/ directory: The new Common/ barrel exports a collection of shared internal primitives (BaseButton, FormRoot, CrossButton, etc.). Worth confirming whether any of these are intended to be public-facing vs. library-internal only, since they're currently accessible if someone digs into Common/index.ts. A follow-up pass might add them to or explicitly exclude them from the top-level src/index.ts.

• Changeset: The updated silly-cups-arrive.md documents the ContextMenu fix nicely. Once the duplicate export issue above is cleaned up, the changeset description is accurate and complete for a minor bump.

---

Overall this is a clean, well-scoped refactor. The one residual issue with duplicate ContextMenu type exports needs resolving, but the architecture direction and consistency across components is spot-on. 👍

[github-actions]

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

---

Code Review

Great iteration — the issues flagged in previous reviews have been addressed cleanly. The ContextMenu type consolidation is done correctly: ContextMenu.tsx now imports from ./ContextMenu.types, ContextMenuItemProps has the full type?: 'default' | 'danger' field, and Button.tsx no longer has the duplicate Alignment type. The changeset documents these fixes clearly. 🙌

---

🚨 Must Fix

• Flyout.types.ts: FlyoutHeaderProps and FlyoutFooterProps are each declared twice in the same file (lines 17–19 & 25–27 for FlyoutHeaderProps, lines 21–23 & 29–31 for FlyoutFooterProps). TypeScript merges duplicate interface declarations silently, so there's no compile error — but it's redundant dead code that could cause confusion. The second set of declarations (lines 25–31) should be removed. Fix this →

---

💡 Suggestions (non-blocking)

• Accordion.types.ts: Size, Gap, and Color are exported from the types file but intentionally absent from index.ts — that's the right call. Worth a brief inline comment (// internal) on those exports so future contributors don't second-guess whether they belong in the public API.

• Common/ barrel: The TextSize, TextWeight, and CursorOptions types are exported from Common/index.ts. Worth a follow-up pass to confirm whether any of these should appear in the top-level src/index.ts or remain strictly internal — the Common/ directory looks intentionally library-private, which is fine.

---

Architecture direction is solid and consistent across all 100 files. One small duplication to clean up in Flyout.types.ts, then this is ready. 👍

[github-actions]

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

---

Code Review

Great structural work overall — the modular file pattern is consistent and well-executed across all 100 files. The previous issues (ContextMenu duplicate types, Button Alignment) have all been addressed. 🙌

---

🚨 Must Fix

• Flyout: Flyout.types.ts contains stale, simplified type definitions that don't match what the component actually implements in Flyout.tsx. Since index.ts exports from Flyout.types.ts, consumers of the package receive the wrong types — this is a silent public API regression:

  Type	Exported via Flyout.types.ts (what consumers see)	Actual in Flyout.tsx (what the component uses)
  FlyoutHeaderProps	interface extends ContainerProps<'div'> { title: string }	TitleHeaderProps | ChildrenHeaderProps union with title, description, type, showClose, showSeparator, etc.
  FlyoutFooterProps	interface { children: ReactNode }	Omit<ContainerProps<'div'>, ...> & { type?: FlyoutType }
  DialogContentProps	{ children, showClose?, forceMount?, container? }	{ container?, showOverlay?, size?, type?, strategy?, closeOnInteractOutside?, width?, align? }

  The fix is to replace the body of Flyout.types.ts with the correct types (moving them from Flyout.tsx), and update Flyout.tsx to import from ./Flyout.types instead of re-declaring them. Fix this →

---

💡 Suggestions (non-blocking)

• Accordion.types.ts: Size, Gap, and Color are exported but intentionally absent from index.ts — correct call. A brief // internal comment on those exports would help future contributors understand the intent.

• Common/index.ts: TextSize, TextWeight, and CursorOptions are exported from Common.types.ts via the barrel. Worth a follow-up to confirm these are intentionally library-private and don't need to be in the top-level src/index.ts.

---

[github-actions]

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

---

Code Review

• Read CLAUDE.md and conventions
• Review changed files
• Post final review

---

All previously flagged issues have been resolved across the review iterations: ContextMenu duplicate type declarations, Flyout stale types, Flyout duplicate declarations, and the Button Alignment duplicate. The current state of the PR is clean. 🙌

---

💡 Suggestions (non-blocking)

• Flyout.types.ts (lines 35, 54, 73): The typo 'orientaion' (carried forward from the original Flyout.tsx) appears in three Omit calls. Since ContainerProps actually defines orientation?: Orientation, this misspelling means orientation is never truly omitted from TitleHeaderProps, ChildrenHeaderProps, or FlyoutFooterProps — consumers could pass an orientation prop that the internal layout ignores. Not a blocker for this structural PR, but worth a follow-up fix. Fix this →

• Flyout/index.ts: FlyoutSizeType, Strategy, FlyoutType, and DialogContentAlignmentType are exported from Flyout.types.ts but not re-exported via index.ts. If consumers ever need to type the size, strategy, type, or align props explicitly, they can't import these without reaching into internals. Worth deciding whether these should join the public API surface in a follow-up pass.

• Common/index.ts: TextSize, TextWeight, and CursorOptions are re-exported from the Common barrel. Confirming these are intentionally library-private (they don't appear in src/index.ts) looks correct — just worth a comment on the barrel noting they're internal, so future contributors don't accidentally add them to the public API.

---

✅ Architecture is clean and consistent across all 100 files. The modular Component.types.ts pattern is well-executed and aligns with the conventions in .llm/CONVENTIONS.md. Good to merge once the minor typo is optionally cleaned up.

[github-actions]

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

---

• Read CLAUDE.md and conventions
• Review changed files
• Post final review

---

Code Review

All previously flagged issues have been resolved cleanly across this review cycle. The architecture is consistent, the type declarations are correct, and the changeset documents every fix. 🙌

Resolved in this iteration:

• ✅ Flyout.types.ts — 'orientaion' typo fixed to 'orientation' in all three Omit calls (lines 35, 54, 73)
• ✅ ContextMenu.tsx — no more duplicate type exports; cleanly imports from ./ContextMenu.types
• ✅ Button.tsx — Alignment imported from ./Button.types, not re-declared inline
• ✅ Flyout.types.ts — correct, complete type definitions (not stale simplified ones)

---

💡 Suggestions (non-blocking, carry-forward notes for future passes)

• Flyout/index.ts: FlyoutSizeType, Strategy, FlyoutType, and DialogContentAlignmentType are defined in Flyout.types.ts but not re-exported via index.ts. Consumers who want to type the size, strategy, type, or align props explicitly will need to reach into internals. Worth deciding whether these belong in the public API in a follow-up.

• Accordion.types.ts: Size, Gap, and Color are exported from the types file but intentionally absent from index.ts — correct call. A brief // internal comment on those exports would help future contributors understand the intent without second-guessing.

• Common/index.ts: TextSize, TextWeight, and CursorOptions are re-exported from the Common barrel. Looks intentionally library-private (they don't appear in src/index.ts), but worth a comment noting that to prevent them from accidentally landing in a future public API surface.

---

✅ Architecture is clean and consistent across all 100 files. The modular Component.types.ts pattern is well-executed and aligns with .llm/CONVENTIONS.md. Ready to merge.

[workflow-authentication-public]

📚 Storybook Preview Deployed

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

Built from commit: b686c9e179b56a158abe822e344d2c0b8863c4a0