fix: 🐛 resolve circular dependencies (first pass)#841
fix: 🐛 resolve circular dependencies (first pass)#841
Conversation
…ar-file-architecture
…ependencies-first-pass
…ar-file-architecture
…ependencies-first-pass
There was a problem hiding this comment.
I looked through commits 1 by 1, w/o reading too deep into them. Looks good to me. Appreciate your approach to atomic commits
tbh I don't think PRs like this require thorough reviewing. And they can't be thoroughly reviewed, for that matter. As long as there are no functional changes, if it builds — ship it.
@ariser Thanks for your time and support! |
|
Claude finished @punkbit's task in 2m 34s —— View job Code Review: fix/circular-dependencies-first-pass
Great cleanup work here — the approach of making 🚨 Must Fix
💡 Suggestions (non-blocking)
Nice systematic approach to this — the before/after dependency graph screenshots make the improvement very clear. Once the missing exports are addressed this will be solid. 🙌 |
… exported ContextMenuItemProps interface and remove duplicate
…ared twice in the same file
…definitions from Flyout.tsx
|
Claude finished @punkbit's task in 2m 44s —— View job Code Review: fix/circular-dependencies-first-pass
Good systematic work here — the move from a three-line wildcard barrel ( 🚨 Must Fix
💡 Suggestions (non-blocking)
Nice approach overall — the atomic commits and before/after dependency graph make this easy to reason about. Once the missing exports and the broken |
📚 Storybook Preview Deployed✅ Preview URL: https://click-mos11na69-clickhouse.vercel.app Built from commit: |
Why?
The Click-UI source code has several circular dependencies that must be resolved.
During the resolution of component path redundancies and public API encapsulation in #798, several circular dependencies were exposed. There, some quick basic fixes were applied to allow to progress, but it was found that a separate PR was needed to resolve them.
🤖 Once #832 is merged, change base to main.
How?
Preview?
❌ Before
👍 After