chore: eslint#510
Merged
softmarshmallow merged 14 commits intomainfrom Jan 28, 2026
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
WalkthroughAdds numerous lint-suppression comments (mostly for native img usage and TanStack virtualizer hooks), migrates several react-hook-form watchers to useWatch, introduces accessibility id/aria-controls for comboboxes, refactors some internal components and signatures, and adjusts ESLint configuration and miscellaneous formatting across many files. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
- Added "react-hooks/incompatible-library" rule to ESLint configuration. - Updated form components to utilize `useWatch` from `react-hook-form` for better state management. - Adjusted various components to ensure compatibility with the new ESLint rule.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (1)
editor/scaffolds/props-editor/index.ts (1)
139-141: MissingonChangein useEffect dependency array.The
onChangecallback is not included in the dependency array, which will cause a stale closure issue if the parent component provides a newonChangecallback. Additionally, this effect will fire on initial mount, potentially triggering an unwantedonChangecall withinitialProps.🐛 Proposed fix
Consider using a ref to track the previous props and only call
onChangewhen props actually change after initialization:+import { useRef } from "react"; + export function usePropsEditor<T extends Record<string, any>>({ schema, initialProps, onChange, }: UseTemplateEditorArgs<T>): PropsEditorInstance { const [state, dispatch] = useReducer(reducer, { schema, props: initialProps, }); + const isInitialMount = useRef(true); + useEffect(() => { - onChange?.(state.props); - }, [state.props]); + if (isInitialMount.current) { + isInitialMount.current = false; + return; + } + onChange?.(state.props); + }, [state.props, onChange]);
🤖 Fix all issues with AI agents
In `@editor/components/formfield/file-upload-field/file-upload-field.tsx`:
- Around line 172-174: The isImage calculation can misclassify when accept
contains image types but file.type is a non-image; change logic to prioritize
file.type and only check accept when file.type is missing/empty. Update the
isImage expression (referenced variable isImage and inputs file.type and accept)
so it first returns true/false based on file.type.startsWith("image/") when
file.type is present, and only inspects accept (e.g., parse accept entries for
ones that start with "image/") when file.type is falsy or undefined.
In `@editor/grida-forms-hosted/e/formview.tsx`:
- Around line 664-665: The image render is insecure and inaccessible: extend the
ClientImageRenderBlock/FormBlock to include an optional alt:string, update the
component to use block.alt (with a sensible fallback like an empty string or
descriptive text), and remove the hardcoded alt="". Also validate/sanitize
block.src before rendering—e.g., ensure the URL uses an allowed scheme
(https/http or a safe data: whitelist) and normalize/parse it (use URL
constructor or a utility) to reject javascript: or other unsafe values; apply
this validation where blocks are created/loaded and again in the renderer (the
JSX return that currently uses __shared_root_attr and block.src) so only
validated, whitelisted image sources are rendered.
In `@editor/www/pricing/pricing-comparison-table.tsx`:
- Around line 31-40: Remove the unused description prop from the component props
and from all call sites (the prop in the props object and any JSX that passes
description) so the component signature no longer declares description; then
eliminate any references to it. Fix the unsafe non-null assertion on
selectedPlan by replacing the forced "!" usage on the result of plans.find((p)
=> p.name === plan) in pricing-comparison-table.tsx: handle the undefined case
explicitly (e.g., early return/fallback UI or throw a clear error) before
accessing selectedPlan.cta, ensuring selectedPlan is properly type-guarded or
given a safe default rather than asserting non-null. Ensure references to
PricingInformation, plans, plan, and selectedPlan are used as described when
locating the changes.
🧹 Nitpick comments (4)
editor/components/ai-elements/shimmer.tsx (1)
38-38: Minor typo in the ESLint disable comment.The justification is clear and appropriate. Small grammar fix: "its" should be "it's" (contraction of "it is").
📝 Suggested fix
- // eslint-disable-next-line react-hooks/static-components -- its from https://ai-sdk.dev/elements/components/shimmer, we dont have code ownership over it. + // eslint-disable-next-line react-hooks/static-components -- it's from https://ai-sdk.dev/elements/components/shimmer, we don't have code ownership over it.editor/scaffolds/sidebar/sidebar-menu-group.tsx (1)
96-102: Consider applying consistent formatting.Line 98 uses the same nullish coalescing pattern as line 39 but without parentheses. For consistency within the file, consider adding parentheses to match the formatting on line 39.
♻️ Suggested formatting consistency fix
{item.children.map((subItem, index) => ( <Tree - key={subItem.id ?? `subitem-${index}`} + key={(subItem.id ?? `subitem-${index}`)} item={subItem} selection={selection} /> ))}editor/app/(workbench)/[org]/[proj]/(console)/(campaign)/campaigns/[campaign]/_components/overview.tsx (1)
225-269: Consider movingprocessDatalogic insideuseEffectto satisfy exhaustive-deps.The
processDatafunction is defined inside the component but not included in theuseEffectdependency array. While this works correctly (since it only closes over the stablesetChartDatasetter), thereact-hooks/exhaustive-depsrule will likely flag this pattern.♻️ Suggested refactor: inline the processing logic
- function processData(data: AnalyzedData) { + useEffect(() => { if (!data.events.length) return; const grouped = Analytics.serialize(data.events, { dateKey: "bucket", interval: data.interval, }); - // Get all unique event names const eventNames = Array.from(new Set(data.events.map((e) => e.name))); const buckets = grouped.reduce( (acc, cur) => { const key = cur.date.toISOString(); acc[key] = { ...cur, date: cur.date, bucket: Analytics.formatDateByInterval(cur.date, data.interval), }; - // Initialize all event names to 0 eventNames.forEach((name) => { acc[key][name] = 0; }); return acc; }, {} as Record<string, any> ); data.events.forEach((event) => { const iso = new Date(event.bucket).toISOString(); if (!buckets[iso]) return; buckets[iso][event.name] += event.count; }); const chartData = Object.values(buckets).sort( (a, b) => a.date.getTime() - b.date.getTime() ); setChartData(chartData); - } - - useEffect(() => { - processData(data); }, [data]);editor/eslint.config.mjs (1)
68-74: Consider tracking re-enablement of critical React hooks rules.The temporarily disabled rules include
react-hooks/exhaustive-depsandreact-hooks/rules-of-hooks, which are foundational for correct React behavior. While disabling them during migration is understandable, consider adding a tracking issue or TODO comment with a target milestone for re-enabling these, as they catch real bugs (stale closures, conditional hook calls).
Comment on lines
+172
to
+174
| const isImage = Boolean( | ||
| file.type?.startsWith("image/") || accept?.includes("image/") | ||
| ); |
There was a problem hiding this comment.
Potential false positive when accept includes mixed types.
If accept contains both image and non-image types (e.g., "video/*,image/*"), the current OR logic may incorrectly flag a non-image file as an image when file.type is a non-image MIME type but accept includes "image/".
Consider prioritizing file.type and only falling back to accept when file.type is unavailable:
Suggested fix
const isImage = Boolean(
- file.type?.startsWith("image/") || accept?.includes("image/")
+ file.type
+ ? file.type.startsWith("image/")
+ : accept?.includes("image/")
);📝 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.
Suggested change
| const isImage = Boolean( | |
| file.type?.startsWith("image/") || accept?.includes("image/") | |
| ); | |
| const isImage = Boolean( | |
| file.type | |
| ? file.type.startsWith("image/") | |
| : accept?.includes("image/") | |
| ); |
🤖 Prompt for AI Agents
In `@editor/components/formfield/file-upload-field/file-upload-field.tsx` around
lines 172 - 174, The isImage calculation can misclassify when accept contains
image types but file.type is a non-image; change logic to prioritize file.type
and only check accept when file.type is missing/empty. Update the isImage
expression (referenced variable isImage and inputs file.type and accept) so it
first returns true/false based on file.type.startsWith("image/") when file.type
is present, and only inspects accept (e.g., parse accept entries for ones that
start with "image/") when file.type is falsy or undefined.
Comment on lines
+664
to
665
| // eslint-disable-next-line @next/next/no-img-element -- Intentional: renders user-provided form content (external URLs). | ||
| return <img {...__shared_root_attr} src={block.src} alt="" />; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the file and check its context
echo "=== Checking file structure ==="
cd /tmp/repo 2>/dev/null || cd . 2>/dev/null
git ls-files | grep -E "(formview|types)" | head -20
echo -e "\n=== Checking formview.tsx around lines 664-665 ==="
if [ -f "editor/grida-forms-hosted/e/formview.tsx" ]; then
sed -n '660,670p' "editor/grida-forms-hosted/e/formview.tsx"
fi
echo -e "\n=== Searching for ClientRenderBlock type definition ==="
rg -n "type ClientRenderBlock|interface ClientRenderBlock" --type ts --type tsx -A 10 | head -50
echo -e "\n=== Searching for image block validation/sanitization ==="
rg -n "block\.src|sanitize|validate.*url" --type ts --type tsx "editor/grida-forms-hosted" -B 2 -A 2 | head -100
echo -e "\n=== Searching for alt text in block data ==="
rg -n "\.alt|alt.*:" --type ts --type tsx "editor/grida-forms-hosted" | grep -i "block\|image" | head -30Repository: gridaco/grida
Length of output: 1751
🏁 Script executed:
#!/bin/bash
echo "=== Reading types.ts to find block definitions ==="
if [ -f "editor/grida-forms-hosted/types.ts" ]; then
head -200 "editor/grida-forms-hosted/types.ts"
fi
echo -e "\n=== Searching for image block type ==="
rg -n "image.*:" --type-list -U "editor/grida-forms-hosted" -A 3 | head -100
echo -e "\n=== Searching for src property validation ==="
grep -rn "src" "editor/grida-forms-hosted/types.ts" --include="*.ts"
echo -e "\n=== Checking for sanitization/validation helpers ==="
find "editor/grida-forms-hosted" -name "*.ts" -o -name "*.tsx" | xargs grep -l "sanitize\|escape\|validate" 2>/dev/null
echo -e "\n=== Full context around formview.tsx image rendering ==="
sed -n '640,680p' "editor/grida-forms-hosted/e/formview.tsx"Repository: gridaco/grida
Length of output: 5987
🏁 Script executed:
#!/bin/bash
echo "=== Full types.ts content (limited) ==="
wc -l "editor/grida-forms-hosted/types.ts"
cat -n "editor/grida-forms-hosted/types.ts" | head -400
echo -e "\n=== Searching for image block definition ==="
grep -n "image" "editor/grida-forms-hosted/types.ts" -i -C 5Repository: gridaco/grida
Length of output: 12314
🏁 Script executed:
#!/bin/bash
echo "=== Full IFormBlock and related interfaces in types.ts ==="
sed -n '233,268p' "editor/grida-forms-hosted/types.ts"
echo -e "\n=== Search for alt property in types ==="
grep -n "alt" "editor/grida-forms-hosted/types.ts" -i
echo -e "\n=== Checking formview.tsx for validation before rendering blocks ==="
sed -n '1,100p' "editor/grida-forms-hosted/e/formview.tsx" | head -100
echo -e "\n=== Checking if there's sanitization in formview.tsx ==="
grep -n "sanitize\|validate\|escape\|DOMPurify" "editor/grida-forms-hosted/e/formview.tsx"
echo -e "\n=== Context before the image case statement ==="
sed -n '600,664p' "editor/grida-forms-hosted/e/formview.tsx"Repository: gridaco/grida
Length of output: 6292
🏁 Script executed:
#!/bin/bash
echo "=== Check how blocks are created/loaded - find ClientRenderBlock definition ==="
grep -n "ClientRenderBlock\|ClientFileUploadFieldRenderBlock\|ClientSectionRenderBlock" "editor/grida-forms-hosted/e/formview.tsx" -B 2 -A 8 | head -50
echo -e "\n=== Search for where blocks are validated/sanitized during load ==="
find "editor/grida-forms-hosted" -type f \( -name "*.ts" -o -name "*.tsx" \) | xargs grep -l "FormDocument\|FormBlock" | head -10
echo -e "\n=== Check if there's any validation when blocks are fetched/loaded ==="
find "editor/grida-forms-hosted" -type f \( -name "*.ts" -o -name "*.tsx" \) -exec grep -l "validate\|sanitize" {} \; | head -10Repository: gridaco/grida
Length of output: 2017
🏁 Script executed:
#!/bin/bash
echo "=== Find grida-forms lib directory ==="
find . -type d -name "grida-forms" | head -10
echo -e "\n=== Check ClientRenderBlock type definition ==="
find . -path "*grida-forms*" -name "*.ts" -o -name "*.tsx" | xargs grep -l "ClientRenderBlock" | head -10
echo -e "\n=== Look at grida-forms lib structure ==="
find . -path "*grida-forms/lib*" -type f | head -20
echo -e "\n=== Search for image block specific handling ==="
find . -path "*grida-forms*" -type f \( -name "*.ts" -o -name "*.tsx" \) -exec grep -l "image.*block\|block.*image" {} \; | head -10Repository: gridaco/grida
Length of output: 793
🏁 Script executed:
#!/bin/bash
echo "=== Check ClientRenderBlock type definition ==="
grep -n "ClientRenderBlock\|type ClientRenderBlock\|interface ClientRenderBlock" "editor/grida-forms/lib/types.ts" -A 15
echo -e "\n=== Check renderer.ts for block processing ==="
head -200 "editor/grida-forms/lib/renderer.ts"
echo -e "\n=== Check lib index.ts exports ==="
cat "editor/grida-forms/lib/index.ts"Repository: gridaco/grida
Length of output: 5116
🏁 Script executed:
#!/bin/bash
echo "=== Check how blocks are transformed to ClientRenderBlock ==="
grep -n "ClientImageRenderBlock\|new ClientImageRenderBlock\|src:" "editor/grida-forms/lib/renderer.ts" -B 3 -A 5 | head -100
echo -e "\n=== Look for sanitization or validation functions ==="
grep -rn "sanitize\|validate.*src\|DOMPurify\|escape" "editor/grida-forms" --include="*.ts" --include="*.tsx" | grep -v node_modules | head -30
echo -e "\n=== Check if FormBlock src is validated before rendering ==="
grep -rn "src.*validate\|validate.*src" "editor/grida-forms-hosted" --include="*.ts" --include="*.tsx" | head -20
echo -e "\n=== Check full ClientImageRenderBlock context in renderer.ts ==="
sed -n '1,300p' "editor/grida-forms/lib/renderer.ts" | tail -100Repository: gridaco/grida
Length of output: 5090
Address missing alt text and unvalidated image sources.
Two issues with the image rendering:
-
Security:
block.srcis passed directly from the form block definition without visible sanitization or validation. If this data isn't validated upstream when blocks are created/loaded, it could be a vector for XSS or malicious content injection. -
Accessibility: The hardcoded
alt=""attribute fails WCAG standards for content images. To support meaningful alt text, theClientImageRenderBlockinterface and correspondingFormBlocktype would need to include analtproperty.
🤖 Prompt for AI Agents
In `@editor/grida-forms-hosted/e/formview.tsx` around lines 664 - 665, The image
render is insecure and inaccessible: extend the ClientImageRenderBlock/FormBlock
to include an optional alt:string, update the component to use block.alt (with a
sensible fallback like an empty string or descriptive text), and remove the
hardcoded alt="". Also validate/sanitize block.src before rendering—e.g., ensure
the URL uses an allowed scheme (https/http or a safe data: whitelist) and
normalize/parse it (use URL constructor or a utility) to reject javascript: or
other unsafe values; apply this validation where blocks are created/loaded and
again in the renderer (the JSX return that currently uses __shared_root_attr and
block.src) so only validated, whitelisted image sources are rendered.
Comment on lines
+31
to
+40
| }: { | ||
| plans: PricingInformation[]; | ||
| description: string; | ||
| priceDescription: string; | ||
| price: string; | ||
| plan: string; | ||
| showDollarSign?: boolean; | ||
| from?: boolean; | ||
| }) { | ||
| const selectedPlan = plans.find((p) => p.name === plan)!; |
There was a problem hiding this comment.
Unused prop and unsafe non-null assertion.
Two issues in this component:
-
Unused
descriptionprop: Thedescriptionparameter is declared in the props type (line 33) and passed by all callers, but it's never rendered in the component JSX. -
Unsafe non-null assertion on line 40: If
plandoesn't match any plan name inplans,find()returnsundefined. The!assertion silences TypeScript, but accessingselectedPlan.ctaon line 57 will throw at runtime.
Suggested fix
function PricingMobileHeader({
plans,
- description,
priceDescription,
price,
plan,
showDollarSign = true,
from = false,
}: {
plans: PricingInformation[];
- description: string;
priceDescription: string;
price: string;
plan: string;
showDollarSign?: boolean;
from?: boolean;
}) {
- const selectedPlan = plans.find((p) => p.name === plan)!;
+ const selectedPlan = plans.find((p) => p.name === plan);
+ if (!selectedPlan) {
+ return null;
+ }Then remove the description prop from all call sites (lines 97, 154-156, 205-207, 255-257).
🤖 Prompt for AI Agents
In `@editor/www/pricing/pricing-comparison-table.tsx` around lines 31 - 40, Remove
the unused description prop from the component props and from all call sites
(the prop in the props object and any JSX that passes description) so the
component signature no longer declares description; then eliminate any
references to it. Fix the unsafe non-null assertion on selectedPlan by replacing
the forced "!" usage on the result of plans.find((p) => p.name === plan) in
pricing-comparison-table.tsx: handle the undefined case explicitly (e.g., early
return/fallback UI or throw a clear error) before accessing selectedPlan.cta,
ensuring selectedPlan is properly type-guarded or given a safe default rather
than asserting non-null. Ensure references to PricingInformation, plans, plan,
and selectedPlan are used as described when locating the changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
enforces
jsx-a11y/alt-textjsx-a11y/role-has-required-aria-propsreact/display-namereact/no-danger-with-childrenreact/jsx-keyimport/no-anonymous-default-exportreact-hooks/use-memoreact-hooks/incompatible-libraryreact-hooks/immutabilityreact-hooks/static-components@next/next/no-img-elementSummary by CodeRabbit
Accessibility Improvements
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.