refactor: remove few barrel exports#7633
Conversation
WalkthroughRemoved multiple barrel re-exports across CE/EE components and updated consumers to import concrete module paths. Also updated several import paths to new files and added permission-based rendering using EUserPermissions in the workspace sidebar dropdown. No runtime logic or exported signatures were changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant DropdownItem
participant Permissions as EUserPermissions
participant UI as "Workspace Links"
User->>DropdownItem: Open workspace dropdown
DropdownItem->>Permissions: Evaluate user role
alt Role is ADMIN or MEMBER
DropdownItem->>UI: Render Settings and Invite Members
else
DropdownItem->>UI: Omit restricted links
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/web/core/components/issues/issue-modal/form.tsx (1)
419-430: Potential runtime crash when duplicateIssues is undefined
duplicateIssues.lengthis accessed without a null/undefined guard. If the SWR hook hasn’t populated yet, this will throw and crash the modal.Apply one of these fixes:
Option A — guard with optional chaining and reuse a narrowed count
- {duplicateIssues.length > 0 && ( + {(duplicateIssues?.length ?? 0) > 0 && ( <DeDupeButtonRoot workspaceSlug={workspaceSlug?.toString()} isDuplicateModalOpen={isDuplicateModalOpen} - label={ - duplicateIssues.length === 1 - ? `${duplicateIssues.length} ${t("duplicate_issue_found")}` - : `${duplicateIssues.length} ${t("duplicate_issues_found")}` - } + label={(() => { + const count = duplicateIssues?.length ?? 0; + return count === 1 ? `${count} ${t("duplicate_issue_found")}` : `${count} ${t("duplicate_issues_found")}`; + })()} handleOnClick={() => handleDuplicateIssueModal(!isDuplicateModalOpen)} /> )}Option B — default the hook result
- const { duplicateIssues } = useDebouncedDuplicateIssues( + const { duplicateIssues = [] } = useDebouncedDuplicateIssues(Either approach prevents a crash while keeping behavior unchanged.
apps/web/core/components/issues/peek-overview/issue-detail.tsx (1)
1-1: "use client" directive is misspelled — component will not be a Client Component
"use-client"won’t be recognized by Next.js. This file uses hooks (useEffect,useRef), so this is critical.-"use-client"; +"use client";apps/web/core/components/inbox/content/issue-root.tsx (1)
169-178: Potential runtime error:duplicateIssuesmay be undefined initially
useDebouncedDuplicateIssuescan returnundefinedbefore SWR resolves. Accessing.lengthwill throw.- {duplicateIssues.length > 0 && ( + {duplicateIssues?.length > 0 && ( <DeDupeIssuePopoverRoot workspaceSlug={workspaceSlug} projectId={issue.project_id} rootIssueId={issue.id} issues={duplicateIssues} issueOperations={issueOperations} isIntakeIssue /> )}
🧹 Nitpick comments (12)
apps/web/core/components/issues/issue-detail-widgets/relations/content.tsx (2)
13-13: Epic modal import path updated correctlyImporting CreateUpdateEpicModal from its concrete module keeps dependencies explicit and prevents future barrel regressions.
If bundle size is a concern, consider lazy-loading this modal since it renders behind a toggle:
+import dynamic from "next/dynamic"; -import { CreateUpdateEpicModal } from "@/plane-web/components/epics/epic-modal"; +const CreateUpdateEpicModal = dynamic( + () => import("@/plane-web/components/epics/epic-modal").then((m) => m.CreateUpdateEpicModal), + { ssr: false } +);
132-132: Incomplete Tailwind class: "gap-"
className="flex flex-col gap-"is likely a typo and does nothing. Consider a concrete spacing value, e.g.,gap-2.- <div className="flex flex-col gap-"> + <div className="flex flex-col gap-2">apps/web/core/components/issues/issue-modal/form.tsx (1)
43-44: Consistent explicit imports — LGTMImporting DeDupeButtonRoot and DuplicateModalRoot from their concrete modules matches the PR’s objective. Consider lazy-loading DuplicateModalRoot to reduce initial bundle size (see suggestion below).
+import dynamic from "next/dynamic"; -import { DuplicateModalRoot } from "@/plane-web/components/de-dupe/duplicate-modal"; +const DuplicateModalRoot = dynamic( + () => import("@/plane-web/components/de-dupe/duplicate-modal").then((m) => m.DuplicateModalRoot), + { ssr: false } +);apps/web/core/components/workspace/sidebar/dropdown-item.tsx (2)
88-109: Revisit role gating for Settings/Invite + avoid re-allocating arrays
- Confirm the product decision: Should MEMBERS see the Settings link? Many apps restrict workspace Settings to ADMIN/OWNER. If members hit a 403 on
/${workspace.slug}/settings, hide it here.- Using array literals inside render allocates each render. Use direct equality checks or a helper.
Apply:
- {[EUserPermissions.ADMIN, EUserPermissions.MEMBER].includes(workspace?.role) && ( + {(workspace?.role === EUserPermissions.ADMIN || workspace?.role === EUserPermissions.MEMBER) && ( <Link href={`/${workspace.slug}/settings`} onClick={handleClose} className="flex border border-custom-border-200 rounded-md py-1 px-2 gap-1 bg-custom-sidebar-background-100 hover:shadow-sm hover:text-custom-text-200 text-custom-text-300 hover:border-custom-border-300 " > <Settings className="h-4 w-4 my-auto" /> <span className="text-sm font-medium my-auto">{t("settings")}</span> </Link> )} - {[EUserPermissions.ADMIN].includes(workspace?.role) && ( + {workspace?.role === EUserPermissions.ADMIN && ( <Link href={`/${workspace.slug}/settings/members`} onClick={handleClose} className="flex border border-custom-border-200 rounded-md py-1 px-2 gap-1 bg-custom-sidebar-background-100 hover:shadow-sm hover:text-custom-text-200 text-custom-text-300 hover:border-custom-border-300 " > <UserPlus className="h-4 w-4 my-auto" /> <span className="text-sm font-medium my-auto"> {t("project_settings.members.invite_members.title")} </span> </Link> )}If there’s a shared permission utility (e.g.,
hasWorkspacePermission), consider using it instead.Please confirm the intended visibility matrix for Settings and Invite across roles (ADMIN, MEMBER, VIEWER/GUEST), and I can align the conditions accordingly.
38-38: Ensureidis a stringIf
workspace.idcan be numeric/UUID-like, React will coerce, but DOM ids must be strings. Defensive cast is cheap.- id={workspace.id} + id={String(workspace.id)}apps/web/core/components/issues/peek-overview/issue-detail.tsx (2)
85-91: Simplify and correct issue description fallback logicThe current condition uses
||(OR) when it likely intendedAND; it evaluates “truthy” forundefinedand returnsundefined. You can make this clearer and robust.- const issueDescription = - issue.description_html !== undefined || issue.description_html !== null - ? issue.description_html != "" - ? issue.description_html - : "<p></p>" - : undefined; + const issueDescription = + typeof issue?.description_html === "string" && issue.description_html.trim() !== "" + ? issue.description_html + : "<p></p>";
72-76: Safer optional chaining forworkspacebeforetoString()If
projectDetails.workspacecould be undefined, calling.toString()will throw.- projectDetails?.workspace.toString(), + projectDetails?.workspace?.toString(),apps/web/core/components/inbox/content/issue-root.tsx (3)
78-87: Optional chaining beforetoString()Same point as in other files; defensive and free.
- projectDetails?.workspace.toString(), + projectDetails?.workspace?.toString(),
91-162:issueOperationsdependencies likely incomplete
removeIssue,archiveIssue,workspaceSlug, andprojectIdare captured but not in deps. If any can change, the callbacks will be stale.- ), - [inboxIssue] + ), + [inboxIssue, removeIssue, archiveIssue, workspaceSlug, projectId, setIsSubmitting]If these are stable by design, ignore.
62-71: Clear timeout to avoid setState after unmountMirror the small safety improvement.
useEffect(() => { + let timer: ReturnType<typeof setTimeout> | undefined; if (isSubmitting === "submitted") { setShowAlert(false); - setTimeout(async () => { - setIsSubmitting("saved"); - }, 3000); + timer = setTimeout(() => setIsSubmitting("saved"), 3000); } else if (isSubmitting === "submitting") { setShowAlert(true); } - }, [isSubmitting, setShowAlert, setIsSubmitting]); + return () => { + if (timer) clearTimeout(timer); + }; + }, [isSubmitting, setShowAlert, setIsSubmitting]);apps/web/core/components/issues/issue-detail/main-content.tsx (2)
66-75: Optional chaining beforetoString()Align with other files to avoid edge-case crashes.
- projectDetails?.workspace.toString(), + projectDetails?.workspace?.toString(),
77-83: Clear timeout in effectTiny safety improvement to prevent setState after unmount.
useEffect(() => { + let timer: ReturnType<typeof setTimeout> | undefined; if (isSubmitting === "submitted") { setShowAlert(false); - setTimeout(async () => setIsSubmitting("saved"), 2000); + timer = setTimeout(() => setIsSubmitting("saved"), 2000); } else if (isSubmitting === "submitting") setShowAlert(true); + return () => { + if (timer) clearTimeout(timer); + }; }, [isSubmitting, setShowAlert, setIsSubmitting]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/intake/layout.tsx(1 hunks)apps/web/app/(all)/[workspaceSlug]/(settings)/settings/projects/[projectId]/members/page.tsx(1 hunks)apps/web/ce/components/common/index.ts(0 hunks)apps/web/ce/components/common/subscription/index.ts(0 hunks)apps/web/ce/components/de-dupe/index.ts(0 hunks)apps/web/ce/components/de-dupe/issue-block/index.ts(0 hunks)apps/web/ce/components/epics/index.ts(0 hunks)apps/web/ce/components/projects/navigation/index.ts(0 hunks)apps/web/ce/components/projects/settings/intake/index.ts(0 hunks)apps/web/ce/components/projects/teamspaces/index.ts(0 hunks)apps/web/core/components/core/app-header.tsx(1 hunks)apps/web/core/components/inbox/content/issue-root.tsx(1 hunks)apps/web/core/components/inbox/modals/create-modal/create-root.tsx(1 hunks)apps/web/core/components/issues/issue-detail-widgets/relations/content.tsx(1 hunks)apps/web/core/components/issues/issue-detail/main-content.tsx(1 hunks)apps/web/core/components/issues/issue-modal/form.tsx(1 hunks)apps/web/core/components/issues/peek-overview/issue-detail.tsx(1 hunks)apps/web/core/components/settings/sidebar/header.tsx(1 hunks)apps/web/core/components/workspace/sidebar/dropdown-item.tsx(1 hunks)apps/web/ee/components/de-dupe/index.ts(0 hunks)apps/web/ee/components/epics/index.ts(0 hunks)
💤 Files with no reviewable changes (10)
- apps/web/ce/components/common/index.ts
- apps/web/ce/components/de-dupe/issue-block/index.ts
- apps/web/ce/components/de-dupe/index.ts
- apps/web/ce/components/epics/index.ts
- apps/web/ce/components/common/subscription/index.ts
- apps/web/ee/components/de-dupe/index.ts
- apps/web/ce/components/projects/navigation/index.ts
- apps/web/ce/components/projects/settings/intake/index.ts
- apps/web/ee/components/epics/index.ts
- apps/web/ce/components/projects/teamspaces/index.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-05-14T13:16:23.323Z
Learnt from: vamsikrishnamathala
PR: makeplane/plane#7061
File: web/core/components/workspace-notifications/root.tsx:18-18
Timestamp: 2025-05-14T13:16:23.323Z
Learning: In the Plane project codebase, the path alias `@/plane-web` resolves to the `ce` directory, making imports like `@/plane-web/hooks/use-notification-preview` correctly resolve to files in `web/ce/hooks/`.
Applied to files:
apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/intake/layout.tsxapps/web/core/components/issues/issue-modal/form.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (11)
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/projects/[projectId]/members/page.tsx (2)
19-19: Direct import looks good and aligns with barrel removalSwitching to the concrete module path should improve clarity and tree-shaking. No behavioral changes in this file.
19-19: ✅ Confirmed named export and no leftover barrel importsVerified that
apps/web/ce/components/projects/teamspaces/teamspace-list.tsxexportsProjectTeamspaceListas a named export (export const ProjectTeamspaceList…), and there are no remaining imports using the old barrel path. The existing import remains correct:import { ProjectTeamspaceList } from "@/plane-web/components/projects/teamspaces/teamspace-list";No further changes needed.
apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/intake/layout.tsx (2)
6-6: Direct import after barrel removal looks correct and aligns with PR goals.Switching to "@/plane-web/components/projects/settings/intake/header" reduces coupling on the intake barrel and prevents accidental side‑effect imports. Per our prior learning, the "@/plane-web" alias resolves to the CE directory, so this path should resolve cleanly in web/ce. No behavioral changes in this file.
6-6: Client boundary verified and no barrel imports remain
- Searched for any imports from the old
@/plane-web/components/projects/settings/intakebarrel—none were found.- Located the header component at
apps/web/ce/components/projects/settings/intake/header.tsx
and confirmed it begins with the"use client";directive on line 1.Everything lines up correctly—
ProjectInboxHeaderis a Client Component and there are no stale barrel imports.apps/web/core/components/settings/sidebar/header.tsx (1)
9-9: LGTM: no lingering barrel imports detectedRan the provided ripgrep check across the
@/plane-web/components/common,epics, andde-dupepaths and found no remaining barrel-style imports. Changes are clean—approving the code.apps/web/core/components/core/app-header.tsx (1)
8-8: Explicit import path is correctSwitching ExtendedAppHeader to its concrete module path aligns with the barrel clean-up and should reduce accidental re-exports drift.
apps/web/core/components/inbox/modals/create-modal/create-root.tsx (1)
22-23: Approve explicit DeDupe imports; optional dynamic import forDuplicateModalRoot
- The separate imports for
DeDupeButtonRootandDuplicateModalRootkeep the module graph lean and improve clarity.- I ran the ripgrep check and found no remaining barrel imports under
@/plane-web/components/de-dupe, confirming that all components are now imported explicitly.If you’d like to defer the cost of the conditionally rendered
DuplicateModalRoot, you could dynamically import it:+import dynamic from "next/dynamic"; -import { DuplicateModalRoot } from "@/plane-web/components/de-dupe/duplicate-modal"; +const DuplicateModalRoot = dynamic( + () => import("@/plane-web/components/de-dupe/duplicate-modal").then((m) => m.DuplicateModalRoot), + { ssr: false } +);apps/web/core/components/workspace/sidebar/dropdown-item.tsx (1)
13-13: No lingering barrel imports found – ready to mergeThe regex scan returned no matches for any old barrel-style imports under
@/plane-web/components/common/subscription(excluding the updatedsubscription-pillpath), confirming that all references have been updated correctly. Feel free to proceed with merging.apps/web/core/components/issues/peek-overview/issue-detail.tsx (1)
17-17: Import path update verified—no remaining barrel importsRan a project-wide search for any
from "@/plane-web/components/de-dupe"imports and found zero occurrences, confirming that all code now uses the specificduplicate-popovermodule. LGTM—approving these changes.apps/web/core/components/inbox/content/issue-root.tsx (1)
29-29: Path fix LGTMImport now targets
duplicate-popover. Consistent with the refactor.apps/web/core/components/issues/issue-detail/main-content.tsx (1)
19-19: Path fix LGTMImport points to
duplicate-popover. Good.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/web/ce/components/breadcrumbs/project-feature.tsx (4)
47-55: Remove unsafe asserts by tightening types; avoid double casts for icon.Casting to
FC<ISvgIcons>twice suggestsTNavigationItem.iconisn’t typed precisely. Prefer strengthening the type ofTNavigationItem.icontoFC<ISvgIcons>and drop the casts here.Apply in this file (after updating
TNavigationItemupstream):- customContent: <SwitcherLabel name={item.name} LabelIcon={item.icon as FC<ISvgIcons>} />, + customContent: <SwitcherLabel name={item.name} LabelIcon={item.icon} />, action: () => router.push(item.href), - icon: item.icon as FC<ISvgIcons>, + icon: item.icon,And upstream (outside this hunk), in
@/components/workspace/sidebar/project-navigation:// ensure icon is fully typed to eliminate casts at call sites export type TNavigationItem = { key: string; name: string; href: string; icon: FC<ISvgIcons>; shouldRender?: boolean; // ... };
47-49: Be explicit about default rendering; treat undefined shouldRender as true.If
additionalNavigationItemsdon’t setshouldRender, the current filter hides them. Defaulting to true is safer for optional fields.- .filter((item) => item.shouldRender) + .filter((item) => item.shouldRender ?? true)
36-40: Memoize derived collections to reduce re-renders of the dropdown items.These arrays are recomputed on each render. Memoizing can cut unnecessary work and prop-churn into
BreadcrumbNavigationDropdown, especially when MobX triggers frequent updates.-3 import { FC } from "react"; +3 import { FC, useMemo } from "react"; @@ -36 const navigationItems = getProjectFeatureNavigation(workspaceSlug, projectId, project); +36 const navigationItems = useMemo( +37 () => getProjectFeatureNavigation(workspaceSlug, projectId, project), +38 [workspaceSlug, projectId, project] +39 ); @@ -39 const allNavigationItems = [...(additionalNavigationItems || []), ...navigationItems]; +40 const allNavigationItems = useMemo( +41 () => [...(additionalNavigationItems || []), ...navigationItems], +42 [additionalNavigationItems, navigationItems] +43 );Also applies to: 47-55, 3-3
57-60: Consider centralizing the fallback route construction.The fallback push builds a feature route that mirrors navigation helper logic. Centralizing this in the same helper (e.g.,
buildProjectFeatureHref) would keep route logic DRY and consistent with the items’hrefgeneration.Would you like me to draft a small
buildProjectFeatureHref(workspaceSlug, projectId, featureKey)utility and update both the helper and this call site?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/web/ce/components/breadcrumbs/project-feature.tsx(1 hunks)apps/web/ee/components/projects/settings/intake/index.ts(0 hunks)
💤 Files with no reviewable changes (1)
- apps/web/ee/components/projects/settings/intake/index.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
apps/web/ce/components/breadcrumbs/project-feature.tsx (1)
15-15: Verified removal of barrel imports and helper export
- No remaining imports from the old
projects/navigationbarrel inapps/webgetProjectFeatureNavigationis correctly exported fromapps/web/ce/components/projects/navigation/helper.tsxLGTM.
2 participants
Description
This PR removes a few barrel exports from the web app.
Type of Change
Summary by CodeRabbit
New Features
Refactor