fix: move lucide related constants to ui package#7226
fix: move lucide related constants to ui package#7226sriramveeraghanta merged 2 commits intopreviewfrom
Conversation
WalkthroughThe changes reorganize icon-related exports and imports, shifting icon constants and utilities from the constants package to the UI package. New icons are added, utility functions are relocated, and import paths are updated throughout the codebase to reflect the new structure. Export statements are consolidated for clarity. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant UI_Constants
participant UI_Utils
participant LucideIcons
App->>UI_Constants: import { LUCIDE_ICONS_LIST }
App->>UI_Utils: import { getRandomIconName }
UI_Utils->>UI_Constants: access LUCIDE_ICONS_LIST
UI_Constants->>LucideIcons: import { Search, User }
UI_Utils-->>App: return random icon name
Suggested labels
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
web/core/components/common/logo.tsx (1)
11-11: Consider importing the constant directly to optimise bundle size
@plane/uiis the barrel for the whole UI package; pulling from it forces the bundler to evaluate the complete public surface, which may hinder tree-shaking.
Importing from the dedicated constants entry (e.g.@plane/ui/constants) keeps the dependency graph lean and avoids accidental circular refs.-import { LUCIDE_ICONS_LIST } from "@plane/ui"; +import { LUCIDE_ICONS_LIST } from "@plane/ui/constants";packages/ui/src/constants/icons.ts (2)
150-157: Import list got out of alphabetical order
Search(S) is placed afterToggleLeft(T), andUser(U) is split fromUsersRound. Keeping the Lucide import block strictly alphabetical greatly reduces merge conflicts and eases future diff reviews.- ToggleLeft, - Search, - User, + Search, + ToggleLeft, + User,
917-920: Duplicate / unsorted entries inLUCIDE_ICONS_LIST
SearchandUserare added correctly, but the surrounding block is no longer sorted alphabetically (Searchshould precedeToggleLeft). Re-ordering avoids accidental duplicates and simplifies look-ups.- { name: "ToggleLeft", element: ToggleLeft }, - { name: "User", element: User }, + { name: "Search", element: Search }, + { name: "ToggleLeft", element: ToggleLeft }, + { name: "User", element: User },packages/ui/src/utils/icons.ts (1)
6-7: Minor: micro-optimise random selectionNo action required, but
Math.truncis marginally faster and semantically clearer thanMath.floorhere.- LUCIDE_ICONS_LIST[Math.floor(Math.random() * LUCIDE_ICONS_LIST.length)].name; + LUCIDE_ICONS_LIST[Math.trunc(Math.random() * LUCIDE_ICONS_LIST.length)].name;packages/ui/src/index.ts (1)
1-38: Barrel export list growing – enforce ordering or groupingThis file now exports 30+ sub-modules. Without a consistent order (alphabetical or grouped), merge conflicts become painful.
Consider:
- Alphabetically sorting the export lines, or
- Splitting into thematic sections with comments (e.g. Primitives, Data-display, Helpers).
This is a low-effort future-proofing tweak.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/constants/src/index.ts(0 hunks)packages/ui/src/constants/icons.ts(2 hunks)packages/ui/src/constants/index.ts(1 hunks)packages/ui/src/emoji/icons-list.tsx(1 hunks)packages/ui/src/emoji/logo.tsx(1 hunks)packages/ui/src/emoji/lucide-icons-list.tsx(1 hunks)packages/ui/src/index.ts(1 hunks)packages/ui/src/utils/icons.ts(1 hunks)packages/ui/src/utils/index.ts(1 hunks)packages/utils/src/emoji.ts(1 hunks)web/core/components/common/logo.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- packages/constants/src/index.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/ui/src/utils/icons.ts (1)
packages/ui/src/constants/icons.ts (1)
LUCIDE_ICONS_LIST(765-921)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
packages/ui/src/utils/index.ts (1)
1-1: 👍 Good addition – utilities now surfaced from a single entry pointRe-exporting
./iconsthroughutilsimproves DX; no issues spotted.packages/ui/src/constants/index.ts (1)
1-1: Re-export looks fine, but validate bundle impactRe-exporting the full icons list from the UI package means every consumer of
@plane/uican now pull ~900 Lucide imports by accident. Please double-check that:
- Consumers only import what they actually need (tree-shaking works).
- There are no name clashes with other constants exported from this barrel.
If either is a concern, consider exposing the icons list under a dedicated sub-path (e.g.
@plane/ui/icons) instead of the generic@plane/ui.packages/utils/src/emoji.ts (1)
4-4: LGTM – removed unused importDropping
LUCIDE_ICONS_LISTkeeps this util focused on emoji. No further action.
| // icons | ||
| import useFontFaceObserver from "use-font-face-observer"; | ||
| import { MATERIAL_ICONS_LIST } from "@plane/constants"; | ||
| import { MATERIAL_ICONS_LIST } from ".."; |
There was a problem hiding this comment.
Wrong barrel used – high chance of circular dependency or missing export
"..” resolves to the root src/index.ts, which in many libraries re-exports this component, leading to a circular import at runtime (or, worse, MATERIAL_ICONS_LIST being undefined if the constant is initialised later).
Import the constant from the dedicated constants barrel instead:
-import { MATERIAL_ICONS_LIST } from "..";
+import { MATERIAL_ICONS_LIST } from "../constants";🤖 Prompt for AI Agents
In packages/ui/src/emoji/icons-list.tsx at line 5, the import of
MATERIAL_ICONS_LIST from ".." causes a circular dependency or may result in
MATERIAL_ICONS_LIST being undefined. To fix this, change the import to reference
the dedicated constants barrel file where MATERIAL_ICONS_LIST is explicitly
exported, avoiding indirect imports through the root index.ts.
| // plane imports | ||
| import { LUCIDE_ICONS_LIST } from "@plane/constants"; | ||
| // local imports | ||
| import { LUCIDE_ICONS_LIST } from ".."; |
There was a problem hiding this comment.
Same barrel-circularity problem as above
Use the constants sub-module to avoid circular references and clarify intent.
-import { LUCIDE_ICONS_LIST } from "..";
+import { LUCIDE_ICONS_LIST } from "../constants";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { LUCIDE_ICONS_LIST } from ".."; | |
| import { LUCIDE_ICONS_LIST } from "../constants"; |
🤖 Prompt for AI Agents
In packages/ui/src/emoji/logo.tsx at line 5, the import of LUCIDE_ICONS_LIST
from the parent directory causes a circular dependency. To fix this, change the
import to come from the constants sub-module instead, which will avoid circular
references and make the import purpose clearer.
| // plane imports | ||
| import { LUCIDE_ICONS_LIST } from "@plane/constants"; | ||
| // local imports | ||
| import { LUCIDE_ICONS_LIST } from ".."; |
There was a problem hiding this comment.
Import path should point at the constants barrel
For consistency and to prevent cyclical imports, fetch the list from the constants module, not the package root.
-import { LUCIDE_ICONS_LIST } from "..";
+import { LUCIDE_ICONS_LIST } from "../constants";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { LUCIDE_ICONS_LIST } from ".."; | |
| import { LUCIDE_ICONS_LIST } from "../constants"; |
🤖 Prompt for AI Agents
In packages/ui/src/emoji/lucide-icons-list.tsx at line 4, the import path for
LUCIDE_ICONS_LIST should be updated to import from the constants barrel module
instead of the package root. Change the import statement to point directly to
the constants module to maintain consistency and avoid cyclical imports.
| @@ -0,0 +1,7 @@ | |||
| import { LUCIDE_ICONS_LIST } from ".."; | |||
There was a problem hiding this comment.
Circular import & wrong source – import the list directly
icons.ts lives under utils/ but pulls LUCIDE_ICONS_LIST from the UI package barrel ("..").
At runtime this introduces an avoidable circular-dependency chain:
ui/index → utils/index → utils/icons → back to ui/index
and couples a tiny util to the heavy top-level barrel.
Import the source constant directly to break the cycle and shave bundle size:
-import { LUCIDE_ICONS_LIST } from "..";
+import { LUCIDE_ICONS_LIST } from "../constants";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { LUCIDE_ICONS_LIST } from ".."; | |
| -import { LUCIDE_ICONS_LIST } from ".."; | |
| +import { LUCIDE_ICONS_LIST } from "../constants"; |
🤖 Prompt for AI Agents
In packages/ui/src/utils/icons.ts at line 1, the import of LUCIDE_ICONS_LIST
from the UI package barrel ("..") causes a circular dependency and increases
bundle size. To fix this, change the import to reference the original source
file where LUCIDE_ICONS_LIST is defined directly, avoiding the top-level barrel
import and breaking the circular dependency chain.
* fix: move lucide related constants to ui package * chore: update yarn.lock
2 participants
Description
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Summary by CodeRabbit
New Features
Refactor
Chores