chore: migrate radix, bump packages, audit#534
Conversation
WalkthroughReplaced many scoped Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 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. Comment |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
- Replaced imports from `@radix-ui/react-*` to `radix-ui` for various components including Dialog, Tooltip, DropdownMenu, and Progress. - Updated component usage across the application to reflect the new import structure, ensuring consistency and adherence to the latest Radix UI practices.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
editor/app/(tenant)/~/[tenant]/(r)/r/[slug]/t/[code]/_components/invitation.tsx (1)
50-50:⚠️ Potential issue | 🔴 CriticalFix incorrect Dialog component property:
DialogPrimitive.DialogContentshould beDialogPrimitive.Content.In the
radix-uipackage (and its predecessor@radix-ui/react-dialog), the Dialog content component is accessed asDialog.Content, notDialog.DialogContent. The codebase confirms this pattern—other files using the same import syntax (Dialog as DialogPrimitive) correctly useDialogPrimitive.Content. This will cause a runtime error at lines 50 and 68.🐛 Proposed fix
- <DialogPrimitive.DialogContent className="fixed inset-0 p-0 border-none outline-none bg-background data-[state=closed]:animate-out data-[state=closed]:fade-out-0 z-10"> + <DialogPrimitive.Content className="fixed inset-0 p-0 border-none outline-none bg-background data-[state=closed]:animate-out data-[state=closed]:fade-out-0 z-10">- </DialogPrimitive.DialogContent> + </DialogPrimitive.Content>editor/components/ui/aspect-ratio.tsx (1)
1-11:⚠️ Potential issue | 🔴 CriticalMissing
Reactimport —React.ComponentPropson line 7 will fail.The file references
React.ComponentPropsbut never importsReact. The new JSX transform handles JSX without an explicit import, but the type reference toReact.ComponentPropsstill requires the namespace to be in scope.Proposed fix
"use client"; +import * as React from "react"; import { AspectRatio as AspectRatioPrimitive } from "radix-ui";
🤖 Fix all issues with AI agents
In `@editor/components/data-table/data-table-view-options.tsx`:
- Line 3: The file currently imports DropdownMenu as DropdownMenuPrimitive from
"radix-ui" and uses DropdownMenuPrimitive.Trigger; replace that by removing the
raw Radix import and instead import DropdownMenuTrigger from the shadcn
dropdown-menu wrapper alongside DropdownMenu, DropdownMenuContent,
DropdownMenuLabel, DropdownMenuSeparator, and DropdownMenuCheckboxItem, then
update usages of DropdownMenuPrimitive.Trigger to use DropdownMenuTrigger
(keeping the rest of the shadcn components unchanged).
In `@editor/components/monaco/index.tsx`:
- Around line 16-22: The fetch for "/monaco-themes/grida-dark.json" lacks error
handling and can leave themeloaded false; wrap the promise chain for
monaco.editor.defineTheme and monaco.editor.setTheme in a catch so any
fetch/network/JSON error still calls setThemeLoaded(true) and logs the error.
Update the promise chain that currently calls monaco.editor.defineTheme("dark",
...) and setThemeLoaded(true) to include a .catch handler that calls
setThemeLoaded(true) (so the light theme fallback and the second useEffect can
run) and optionally logs the error for debugging.
In `@editor/scaffolds/editor/editor.tsx`:
- Line 19: The PR is importing and using TooltipPrimitive.Provider directly
which uses Radix's 700ms delay; replace usage by importing the shared
TooltipProvider (exported from '@/components/ui/tooltip') and swap all instances
of <TooltipPrimitive.Provider>...</TooltipPrimitive.Provider> to
<TooltipProvider>...</TooltipProvider> so the default delayDuration={0} from the
shared component is applied (search for TooltipPrimitive.Provider and update to
TooltipProvider; ensure you add the TooltipProvider import and remove the direct
radix TooltipPrimitive import).
🧹 Nitpick comments (4)
editor/grida-canvas-react-starter-kit/starterkit-loading/loading.tsx (1)
324-344: Consider reusing the sharedProgresscomponent from@/components/ui/progress.This local
Progressduplicates the shared one with only minor styling differences (h-1vsh-2, norounded-full). Since both acceptclassName, you could use the shared component and override styles via theclassNameprop at the call site (line 224).editor/scaffolds/sidecontrol/controls/x-userdata.tsx (1)
11-11: Consider importingDialogClosefrom the shadcn wrapper instead of reaching intoradix-uidirectly.Lines 3-10 already import Dialog sub-components from
@/components/ui/dialog. The shadcn wrapper exportsDialogClose, but the current code importsDialog as DialogPrimitivefromradix-uiand usesDialogPrimitive.Closeat lines 96–103. This mixes abstraction layers—prefer the shadcn wrapper for consistency.Proposed fix
import { Dialog, DialogContent, DialogDescription, DialogFooter, DialogHeader, DialogTitle, DialogTrigger, + DialogClose, } from "@/components/ui/dialog"; -import { Dialog as DialogPrimitive } from "radix-ui";Then replace
DialogPrimitive.ClosewithDialogCloseat lines 96–103.editor/public/monaco-themes/grida-dark.json (1)
1-1: Consider formatting the JSON for maintainability.The theme definition is valid, but being a single minified line makes it difficult to review diffs or make targeted edits. Since this is a static asset (not a build artifact), pretty-printing would improve readability at negligible cost.
editor/kits/minimal-tiptap/components/toolbar-button.tsx (1)
2-2: Consider using a type-only import since this is only used for type references.The
TooltipPrimitiveimport is only used for theTooltipContentPropstype on line 14. A type-only import makes the intent clearer and guarantees it's erased at compile time.♻️ Suggested change
-import { Tooltip as TooltipPrimitive } from "radix-ui"; +import type { Tooltip as TooltipPrimitive } from "radix-ui";
| "use client"; | ||
|
|
||
| import { DropdownMenuTrigger } from "@radix-ui/react-dropdown-menu"; | ||
| import { DropdownMenu as DropdownMenuPrimitive } from "radix-ui"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, locate and read the file
find . -name "data-table-view-options.tsx" -type fRepository: gridaco/grida
Length of output: 115
🏁 Script executed:
#!/bin/bash
# Check the shadcn dropdown-menu component to see what it exports
find . -path "*/components/ui/dropdown-menu*" -type f | head -10Repository: gridaco/grida
Length of output: 97
🏁 Script executed:
#!/bin/bash
# Look for the actual data-table-view-options file and examine its imports/usage
fd "data-table-view-options.tsx" --type fRepository: gridaco/grida
Length of output: 113
🏁 Script executed:
#!/bin/bash
# Read the data-table-view-options.tsx file
cat -n editor/components/data-table/data-table-view-options.tsxRepository: gridaco/grida
Length of output: 2114
🏁 Script executed:
#!/bin/bash
# Read the dropdown-menu.tsx file to check exports
cat -n editor/components/ui/dropdown-menu.tsxRepository: gridaco/grida
Length of output: 10315
Use the shadcn DropdownMenuTrigger instead of the raw Radix primitive.
This file imports DropdownMenu as DropdownMenuPrimitive directly from "radix-ui" and uses DropdownMenuPrimitive.Trigger at lines 24 and 33, while using shadcn wrappers everywhere else (DropdownMenu, DropdownMenuContent, DropdownMenuLabel, DropdownMenuSeparator, DropdownMenuCheckboxItem). The shadcn dropdown-menu module exports a DropdownMenuTrigger wrapper that should be used instead. Remove the raw Radix import and add DropdownMenuTrigger to the shadcn imports.
Proposed fix
-import { DropdownMenu as DropdownMenuPrimitive } from "radix-ui";
import { Table } from "@tanstack/react-table";
import { Settings2 } from "lucide-react";
import { Button } from "@/components/ui/button";
import {
DropdownMenu,
DropdownMenuCheckboxItem,
DropdownMenuContent,
DropdownMenuLabel,
DropdownMenuSeparator,
+ DropdownMenuTrigger,
} from "@/components/ui/dropdown-menu"; <DropdownMenu>
- <DropdownMenuPrimitive.Trigger asChild>
+ <DropdownMenuTrigger asChild>
<Button
variant="outline"
size="sm"
className="ml-auto hidden h-8 lg:flex"
>
<Settings2 className="size-4" />
View
</Button>
- </DropdownMenuPrimitive.Trigger>
+ </DropdownMenuTrigger>🤖 Prompt for AI Agents
In `@editor/components/data-table/data-table-view-options.tsx` at line 3, The file
currently imports DropdownMenu as DropdownMenuPrimitive from "radix-ui" and uses
DropdownMenuPrimitive.Trigger; replace that by removing the raw Radix import and
instead import DropdownMenuTrigger from the shadcn dropdown-menu wrapper
alongside DropdownMenu, DropdownMenuContent, DropdownMenuLabel,
DropdownMenuSeparator, and DropdownMenuCheckboxItem, then update usages of
DropdownMenuPrimitive.Trigger to use DropdownMenuTrigger (keeping the rest of
the shadcn components unchanged).
| fetch("/monaco-themes/grida-dark.json") | ||
| .then((res) => res.json()) | ||
| .then((data) => { | ||
| monaco.editor.defineTheme("dark", data as any); | ||
| monaco.editor.setTheme(theme === "dark" ? "dark" : "light"); | ||
| setThemeLoaded(true); | ||
| }); |
There was a problem hiding this comment.
Missing error handling on theme fetch — silent failure.
If /monaco-themes/grida-dark.json fails to load (404, network error, invalid JSON), the promise rejects unhandled and setThemeLoaded(true) is never called. This means the second useEffect (line 27–31) that reacts to theme changes will be permanently gated by themeloaded === false, silently breaking all theme switching.
Add a .catch() to handle the failure gracefully — at minimum, still mark the theme as loaded so the light theme fallback works.
Proposed fix
fetch("/monaco-themes/grida-dark.json")
.then((res) => res.json())
.then((data) => {
monaco.editor.defineTheme("dark", data as any);
monaco.editor.setTheme(theme === "dark" ? "dark" : "light");
setThemeLoaded(true);
- });
+ })
+ .catch((err) => {
+ console.error("Failed to load Monaco theme:", err);
+ // Still mark as loaded so light theme fallback works
+ monaco.editor.setTheme("light");
+ setThemeLoaded(true);
+ });📝 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.
| fetch("/monaco-themes/grida-dark.json") | |
| .then((res) => res.json()) | |
| .then((data) => { | |
| monaco.editor.defineTheme("dark", data as any); | |
| monaco.editor.setTheme(theme === "dark" ? "dark" : "light"); | |
| setThemeLoaded(true); | |
| }); | |
| fetch("/monaco-themes/grida-dark.json") | |
| .then((res) => res.json()) | |
| .then((data) => { | |
| monaco.editor.defineTheme("dark", data as any); | |
| monaco.editor.setTheme(theme === "dark" ? "dark" : "light"); | |
| setThemeLoaded(true); | |
| }) | |
| .catch((err) => { | |
| console.error("Failed to load Monaco theme:", err); | |
| // Still mark as loaded so light theme fallback works | |
| monaco.editor.setTheme("light"); | |
| setThemeLoaded(true); | |
| }); |
🤖 Prompt for AI Agents
In `@editor/components/monaco/index.tsx` around lines 16 - 22, The fetch for
"/monaco-themes/grida-dark.json" lacks error handling and can leave themeloaded
false; wrap the promise chain for monaco.editor.defineTheme and
monaco.editor.setTheme in a catch so any fetch/network/JSON error still calls
setThemeLoaded(true) and logs the error. Update the promise chain that currently
calls monaco.editor.defineTheme("dark", ...) and setThemeLoaded(true) to include
a .catch handler that calls setThemeLoaded(true) (so the light theme fallback
and the second useEffect can run) and optionally logs the error for debugging.
| import { FormFieldDefinition } from "@/grida-forms-hosted/types"; | ||
| import { FormFieldUpsert, EditorApiResponse } from "@/types/private/api"; | ||
| import { TooltipProvider } from "@radix-ui/react-tooltip"; | ||
| import { Tooltip as TooltipPrimitive } from "radix-ui"; |
There was a problem hiding this comment.
Using raw TooltipPrimitive.Provider loses the delayDuration=0 default.
The TooltipProvider exported from @/components/ui/tooltip sets delayDuration={0} by default. By importing TooltipPrimitive directly from radix-ui and using <TooltipPrimitive.Provider> without props, you'll get the Radix default delay of 700ms, making tooltips noticeably slower to appear. This is likely a UX regression.
Consider importing TooltipProvider from the shared UI component instead:
Proposed fix
-import { Tooltip as TooltipPrimitive } from "radix-ui";
+import { TooltipProvider } from "@/components/ui/tooltip";Then replace all <TooltipPrimitive.Provider> / </TooltipPrimitive.Provider> with <TooltipProvider> / </TooltipProvider>.
Also applies to: 84-92
🤖 Prompt for AI Agents
In `@editor/scaffolds/editor/editor.tsx` at line 19, The PR is importing and using
TooltipPrimitive.Provider directly which uses Radix's 700ms delay; replace usage
by importing the shared TooltipProvider (exported from
'@/components/ui/tooltip') and swap all instances of
<TooltipPrimitive.Provider>...</TooltipPrimitive.Provider> to
<TooltipProvider>...</TooltipProvider> so the default delayDuration={0} from the
shared component is applied (search for TooltipPrimitive.Provider and update to
TooltipProvider; ensure you add the TooltipProvider import and remove the direct
radix TooltipPrimitive import).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@editor/package.json`:
- Around line 65-72: Remove the duplicate Radix packages listed
individually—@radix-ui/react-checkbox, `@radix-ui/react-popover`,
`@radix-ui/react-select`, `@radix-ui/react-slider`, `@radix-ui/react-toggle`, and
`@radix-ui/react-toggle-group`—and keep only `@radix-ui/react-icons` and
`@radix-ui/react-use-controllable-state` in package.json; if
`@radix-ui/react-select`@^2.2.6 intentionally needs a different major than the
aggregate exported by `@radix-ui/react`, either move that pinned version into an
overrides (or resolutions) entry or document the deliberate override (e.g., in
README) so the version discrepancy is explicit.
🧹 Nitpick comments (5)
apps/viewer/package.json (1)
15-15:eslint-config-nextstill at 16.1.3 whilenextis bumped to 16.1.6.Line 29 pins
eslint-config-nextto16.1.3. Consider bumping it to16.1.6to keep them in sync and avoid potential lint rule mismatches.apps/backgrounds/package.json (1)
14-19:@next/third-partiesandeslint-config-nextlag behindnextversion.
@next/third-parties(line 14) andeslint-config-next(line 34) are still at16.1.3whilenexthas been bumped to16.1.6. These packages are part of the same Next.js release and should ideally stay aligned.editor/package.json (3)
61-62:@next/mdx,@next/third-parties, andeslint-config-nextnot bumped withnext.Same pattern as the other apps —
nextis at16.1.6but@next/mdx(line 61),@next/third-parties(line 62), andeslint-config-next(line 248) remain at16.1.3. Keeping these in sync avoids subtle compatibility issues.Also applies to: 157-157, 248-248
113-113: Looseaxiosrange^1is effectively overridden by root.The root
pnpm.overridespinsaxiosto1.13.4, so this^1range has no practical effect. Consider aligning the declared version here with the override for clarity, or noting that the override governs the resolved version.
159-159: Looseopenairange^6is pinned to6.18.0by root override.Same situation as
axios— the root override controls the actual resolved version. The wide range here is cosmetically misleading. Consider using6.18.0directly or adding a comment.
| "@radix-ui/react-checkbox": "^1.3.3", | ||
| "@radix-ui/react-icons": "^1.3.2", | ||
| "@radix-ui/react-label": "^2.1.7", | ||
| "@radix-ui/react-menubar": "^1.1.14", | ||
| "@radix-ui/react-navigation-menu": "^1.2.12", | ||
| "@radix-ui/react-popover": "^1.1.13", | ||
| "@radix-ui/react-portal": "^1.1.8", | ||
| "@radix-ui/react-progress": "^1.1.7", | ||
| "@radix-ui/react-radio-group": "^1.3.6", | ||
| "@radix-ui/react-scroll-area": "^1.2.9", | ||
| "@radix-ui/react-select": "^2.2.5", | ||
| "@radix-ui/react-separator": "^1.1.7", | ||
| "@radix-ui/react-slider": "^1.3.4", | ||
| "@radix-ui/react-slot": "^1.2.3", | ||
| "@radix-ui/react-switch": "^1.2.4", | ||
| "@radix-ui/react-tabs": "^1.1.11", | ||
| "@radix-ui/react-toggle": "^1.1.8", | ||
| "@radix-ui/react-toggle-group": "^1.1.9", | ||
| "@radix-ui/react-toolbar": "^1.1.9", | ||
| "@radix-ui/react-tooltip": "^1.2.7", | ||
| "@radix-ui/react-popover": "^1.1.15", | ||
| "@radix-ui/react-select": "^2.2.6", | ||
| "@radix-ui/react-slider": "^1.3.6", | ||
| "@radix-ui/react-toggle": "^1.1.10", | ||
| "@radix-ui/react-toggle-group": "^1.1.11", | ||
| "@radix-ui/react-use-controllable-state": "^1.2.2", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
radix-ui aggregate package what does it export include
💡 Result:
The radix-ui “aggregate” package is a barrel that re-exports each Radix Primitives package as a single named export per component, where the value is the module namespace (so you can do import { Dialog } from "radix-ui" and then use Dialog.Root, Dialog.Trigger, etc.). (cdn.jsdelivr.net)
Main entry (radix-ui)
It exports these namespaces: AccessibleIcon, Accordion, AlertDialog, AspectRatio, Avatar, Checkbox, Collapsible, ContextMenu, Dialog, Direction, DropdownMenu, Form, HoverCard, Label, Menubar, NavigationMenu, Popover, Portal, Progress, RadioGroup, ScrollArea, Select, Separator, Slider, Slot, Switch, Tabs, Toast, Toggle, ToggleGroup, Toolbar, Tooltip, VisuallyHidden, plus unstable_OneTimePasswordField and unstable_PasswordToggleField. (cdn.jsdelivr.net)
Subpath exports (notably radix-ui/internal)
The package export map includes subpath exports ("./*"), and dist/internal.* exports internal building blocks/utilities like Arrow, Collection, Context, DismissableLayer, FocusGuards, FocusScope, Menu, Popper, Presence, RovingFocus, Primitive, plus helpers/hooks like composeRefs/useComposedRefs, useCallbackRef, useControllableState(+Reducer), useEffectEvent, useEscapeKeydown, useIsHydrated, useLayoutEffect, useSize, composeEventHandlers, and PrimitivePropsWithRef. (unpkg.com)
Citations:
- 1: https://cdn.jsdelivr.net/npm/radix-ui%401.4.3/dist/index.d.ts
- 2: https://cdn.jsdelivr.net/npm/radix-ui%401.4.3/dist/index.d.ts
- 3: https://unpkg.com/radix-ui%401.4.3/package.json
🏁 Script executed:
# First, let's locate and read the editor/package.json file
fd -t f 'package.json' | grep -E 'editor/package.json'Repository: gridaco/grida
Length of output: 76
🏁 Script executed:
# Read the editor/package.json to verify the lines mentioned
cat -n editor/package.json | sed -n '60,75p; 155,170p'Repository: gridaco/grida
Length of output: 1376
Remove duplicate @radix-ui/* packages or explicitly pin versions if intentional.
The radix-ui aggregate (^1.4.3, line 163) re-exports Checkbox, Popover, Select, Slider, Toggle, and ToggleGroup. These same packages are listed individually at lines 65–71, creating duplicate dependencies.
Keep only: @radix-ui/react-icons (standalone package, not in aggregate) and @radix-ui/react-use-controllable-state (internal utility).
For the others: Either remove them (if versions align with aggregate's re-exports) or, if @radix-ui/react-select@^2.2.6 intentionally pins a newer major version than the aggregate includes, add a brief comment explaining the version override.
🤖 Prompt for AI Agents
In `@editor/package.json` around lines 65 - 72, Remove the duplicate Radix
packages listed individually—@radix-ui/react-checkbox, `@radix-ui/react-popover`,
`@radix-ui/react-select`, `@radix-ui/react-slider`, `@radix-ui/react-toggle`, and
`@radix-ui/react-toggle-group`—and keep only `@radix-ui/react-icons` and
`@radix-ui/react-use-controllable-state` in package.json; if
`@radix-ui/react-select`@^2.2.6 intentionally needs a different major than the
aggregate exported by `@radix-ui/react`, either move that pinned version into an
overrides (or resolutions) entry or document the deliberate override (e.g., in
README) so the version discrepancy is explicit.
Summary by CodeRabbit
Chores
New Features
UI Improvements