Daily RC#508
Conversation
…nts for improved consistency and styling
…n AlignControl for improved consistency
|
The latest updates on your projects. Learn more about Vercel for GitHub.
5 Skipped Deployments
|
WalkthroughThis PR consolidates UI component refactoring by migrating ActionGroup to ButtonGroup, refactoring Emscripten build configuration in justfile, updating ESLint rules, improving React list key stability, and adding documentation for Rust/Canvas prerequisites. No new features or altered public APIs. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Poem
🚥 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 |
…setup into a shared snippet
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
editor/components/primitives/contenteditable.tsx (1)
50-71: Children rendering is overridden and updates can be skipped.With the new
childrenpath,componentDidUpdatestill overwrites DOM withthis.props.html, andshouldComponentUpdateignoreschildrenchanges. This makeschildrenunreliable (often erased) and can skip legitimate updates. Gate the html sync to the no-children case and allow updates when children change.🐛 Proposed fix
render() { const { tagName, html, innerRef, ...props } = this.props; - const children = this.props.children; + const children = this.props.children; + const hasChildren = children != null; return React.createElement( tagName || "div", { ...props, ref: typeof innerRef === "function" ? (current: HTMLElement) => { innerRef(current); this.el.current = current; } : innerRef || this.el, onInput: this.emitChange, onBlur: this.props.onBlur || this.emitChange, onKeyUp: this.props.onKeyUp || this.emitChange, onKeyDown: this.props.onKeyDown || this.emitChange, contentEditable: this.props.contentEditable, - ...(children == null ? { dangerouslySetInnerHTML: { __html: html } } : {}), + ...(hasChildren ? {} : { dangerouslySetInnerHTML: { __html: html } }), }, - children == null ? undefined : children + hasChildren ? children : undefined ); }shouldComponentUpdate(nextProps: Props): boolean { const { props } = this; const el = this.getEl(); if (!el) return true; - if (normalizeHtml(nextProps.html) !== normalizeHtml(el.innerHTML)) { - return true; - } + const hasChildren = props.children != null || nextProps.children != null; + if (hasChildren) { + if (props.children !== nextProps.children) return true; + } else if (normalizeHtml(nextProps.html) !== normalizeHtml(el.innerHTML)) { + return true; + } return ( props.contentEditable !== nextProps.contentEditable || props.tagName !== nextProps.tagName || props.className !== nextProps.className || props.innerRef !== nextProps.innerRef || !deepEqual(props.style, nextProps.style) ); } componentDidUpdate() { const el = this.getEl(); if (!el) return; - if (this.props.html !== el.innerHTML) { - el.innerHTML = this.props.html; - } - this.lastHtml = this.props.html; + const hasChildren = this.props.children != null; + if (!hasChildren && this.props.html !== el.innerHTML) { + el.innerHTML = this.props.html; + } + this.lastHtml = hasChildren ? el.innerHTML : this.props.html; replaceCaret(el); }
🤖 Fix all issues with AI agents
In `@CONTRIBUTING.md`:
- Around line 24-31: Replace the bold platform labels "**macOS**" and
"**Ubuntu/Debian**" with proper Markdown headings (e.g., "## macOS" and "##
Ubuntu/Debian") so they are treated as section headings rather than emphasized
text; update the surrounding blocks accordingly to maintain spacing and
formatting for the code fences and any subsequent platform-specific
instructions.
In
`@editor/app/`(workbench)/[org]/[proj]/(console)/(campaign)/campaigns/[campaign]/design/_enterprise/west-referral-duo-001-editor.tsx:
- Around line 355-360: The Input's value is reading
values?.components?.referrer?.cta while the onChange calls
props.set("components.referrer-share.cta", ...), causing a mismatch; update the
value binding to the same key used in props.set (e.g., use
values?.components?.["referrer-share"]?.cta) so the Input and props.set target
the same property, ensuring edits are reflected (locate the Input/FieldLabel
block and align the value to the "referrer-share" key).
In `@editor/eslint.config.mjs`:
- Around line 46-47: The config currently turns off critical rules
"react-hooks/rules-of-hooks" and "react-hooks/exhaustive-deps" which risks
runtime bugs; re-enable them instead of blanket "off" by setting them to "error"
or "warn" in eslint.config.mjs (or remove these overrides), then fix existing
violations incrementally in the codebase (or temporarily set to "warn" and
address violations gradually), and if any rule must remain disabled, add an
explicit justification comment in the config and use targeted inline
eslint-disable comments at the specific hook sites referencing rule names to
limit scope.
- Around line 48-55: The listed rules ("react-hooks/set-state-in-effect",
"react-hooks/preserve-manual-memoization", "react-hooks/static-components",
"react-hooks/incompatible-library", "react-hooks/purity",
"react-hooks/immutability", "react-hooks/use-memo", "react-hooks/refs") in
editor/eslint.config.mjs do not exist in eslint-plugin-react-hooks and are
silently ignored; either remove these entries from the rules block or change
their prefix to the correct plugin (react-compiler/) and install that plugin:
update package.json/pnpm-lock to include eslint-plugin-react-compiler, run pnpm
install, and replace each "react-hooks/..." key with "react-compiler/..." (or
simply delete the lines) so the config matches installed plugins and active rule
names.
In `@editor/scaffolds/sidecontrol/controls/ext-ops.tsx`:
- Around line 38-44: The icon-only button rendering <SquaresUniteIcon /> in
ext-ops.tsx lacks an accessible name; update the button element (the one with
onClick={() => onOp?.("union")} and className from buttonVariants) to include an
aria-label describing the action (e.g., "Union" or a more descriptive localized
string) so screen readers can announce its purpose.
🧹 Nitpick comments (3)
editor/components/preview-button/preview-button.tsx (1)
77-77: Consider addingButtonGroupSeparatorfor visual consistency.Other ButtonGroup usages in this PR (e.g.,
customers/page.tsx) include aButtonGroupSeparatorbetween the primary action and the dropdown trigger. Consider adding one here for consistency across the codebase, unless the design intentionally omits it for this component.♻️ Optional: Add separator for consistency
</Button> + <ButtonGroupSeparator /> <DropdownMenu>This would require importing
ButtonGroupSeparator:-import { ButtonGroup } from "@/components/ui/button-group"; +import { ButtonGroup, ButtonGroupSeparator } from "@/components/ui/button-group";editor/scaffolds/sidecontrol/controls/ext-ops.tsx (1)
47-55: Consider usingbuttonVariantsfor styling consistency.The first button uses
buttonVariants({ variant: "ghost", size: "icon" }), but the dropdown trigger uses raw CSS classes. For consistency and maintainability, consider usingbuttonVariantshere as well. This also lacks anaria-labelfor the dropdown trigger.♻️ Suggested refactor
<DropdownMenuTrigger asChild> <button disabled={disabled} - className="p-0 px-1 rounded-md focus-visible:outline-none focus-visible:ring-ring/50 focus-visible:ring-[3px]" + className={cn( + buttonVariants({ variant: "ghost", size: "icon" }), + "p-0 px-1" + )} type="button" + aria-label="More boolean operations" > <CaretDownIcon className="size-3" /> </button> </DropdownMenuTrigger>editor/eslint.config.mjs (1)
6-7: Config spread order differs from other apps in the codebase.Other ESLint configs (e.g.,
apps/backgrounds/eslint.config.mjs,apps/viewer/eslint.config.mjs) spreadnextVitalsbeforenextTs. Here the order is reversed. Since later configs override earlier ones, this could lead to different rule precedence. Consider aligning with the existing pattern for consistency:const eslintConfig = defineConfig([ - ...nextTs, ...nextVitals, + ...nextTs,
| **macOS** | ||
|
|
||
| ```bash | ||
| brew install ninja | ||
| ninja --version | ||
| ``` | ||
|
|
||
| **Ubuntu/Debian** |
There was a problem hiding this comment.
Use headings instead of bold for platform labels.
Markdownlint flags macOS / Ubuntu/Debian as emphasis used as headings. Switch to proper heading levels for consistency.
💡 Suggested change
-**macOS**
+### macOS
...
-**Ubuntu/Debian**
+### Ubuntu/Debian📝 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.
| **macOS** | |
| ```bash | |
| brew install ninja | |
| ninja --version | |
| ``` | |
| **Ubuntu/Debian** | |
| ### macOS | |
🧰 Tools
🪛 LanguageTool
[grammar] ~31-~31: Ensure spelling is correct
Context: ...all ninja ninja --version **Ubuntu/Debian** bash sudo apt-get update sudo apt-get install -y ninja-build ninja --version ``` Then, install the dependencies and run t...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
24-24: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
31-31: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🤖 Prompt for AI Agents
In `@CONTRIBUTING.md` around lines 24 - 31, Replace the bold platform labels
"**macOS**" and "**Ubuntu/Debian**" with proper Markdown headings (e.g., "##
macOS" and "## Ubuntu/Debian") so they are treated as section headings rather
than emphasized text; update the surrounding blocks accordingly to maintain
spacing and formatting for the code fences and any subsequent platform-specific
instructions.
| <FieldLabel>Button Text</FieldLabel> | ||
| <Input | ||
| value={values?.components?.referrer?.cta} | ||
| onChange={(e) => { | ||
| props.set("components.referrer-share.cta", e.target.value); | ||
| }} |
There was a problem hiding this comment.
Fix CTA binding for “Referrer Share” input.
Line 357 binds the value to referrer.cta while updates write to referrer-share.cta, so the field won’t reflect edits.
🔧 Proposed fix
- <Input
- value={values?.components?.referrer?.cta}
+ <Input
+ value={values?.components?.["referrer-share"]?.cta}
onChange={(e) => {
props.set("components.referrer-share.cta", e.target.value);
}}🤖 Prompt for AI Agents
In
`@editor/app/`(workbench)/[org]/[proj]/(console)/(campaign)/campaigns/[campaign]/design/_enterprise/west-referral-duo-001-editor.tsx
around lines 355 - 360, The Input's value is reading
values?.components?.referrer?.cta while the onChange calls
props.set("components.referrer-share.cta", ...), causing a mismatch; update the
value binding to the same key used in props.set (e.g., use
values?.components?.["referrer-share"]?.cta) so the Input and props.set target
the same property, ensuring edits are reflected (locate the Input/FieldLabel
block and align the value to the "referrer-share" key).
| "react-hooks/exhaustive-deps": "off", | ||
| "react-hooks/rules-of-hooks": "off", |
There was a problem hiding this comment.
Disabling critical React Hooks rules carries significant risk.
react-hooks/rules-of-hooks prevents calling hooks inside conditions, loops, or nested functions — violations cause runtime crashes. react-hooks/exhaustive-deps catches stale closure bugs that are notoriously hard to debug.
If these are disabled to silence existing violations, consider fixing the violations incrementally rather than disabling enforcement entirely. At minimum, document why these critical safeguards are being removed.
🤖 Prompt for AI Agents
In `@editor/eslint.config.mjs` around lines 46 - 47, The config currently turns
off critical rules "react-hooks/rules-of-hooks" and
"react-hooks/exhaustive-deps" which risks runtime bugs; re-enable them instead
of blanket "off" by setting them to "error" or "warn" in eslint.config.mjs (or
remove these overrides), then fix existing violations incrementally in the
codebase (or temporarily set to "warn" and address violations gradually), and if
any rule must remain disabled, add an explicit justification comment in the
config and use targeted inline eslint-disable comments at the specific hook
sites referencing rule names to limit scope.
| "react-hooks/set-state-in-effect": "off", | ||
| "react-hooks/preserve-manual-memoization": "off", | ||
| "react-hooks/static-components": "off", | ||
| "react-hooks/incompatible-library": "off", | ||
| "react-hooks/purity": "off", | ||
| "react-hooks/immutability": "off", | ||
| "react-hooks/use-memo": "off", | ||
| "react-hooks/refs": "off", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for package.json files and their dependencies
echo "=== Looking for package.json files ==="
fd -t f "package.json" . --max-depth 3
echo ""
echo "=== Root package.json dependencies ==="
cat package.json 2>/dev/null | jq '.dependencies, .devDependencies' 2>/dev/null | grep -A2 -B2 "eslint-plugin-react"
echo ""
echo "=== Editor directory package.json (if exists) ==="
if [ -f "editor/package.json" ]; then
cat editor/package.json | jq '.dependencies, .devDependencies' 2>/dev/null | grep -A2 -B2 "eslint-plugin-react"
fi
echo ""
echo "=== Checking eslint.config.mjs for plugin configuration ==="
cat editor/eslint.config.mjs 2>/dev/null | head -50Repository: gridaco/grida
Length of output: 3208
🏁 Script executed:
#!/bin/bash
echo "=== editor/package.json ==="
cat editor/package.json
echo ""
echo "=== Full editor/eslint.config.mjs ==="
cat editor/eslint.config.mjs
echo ""
echo "=== Checking for react-compiler references in codebase ==="
rg "react-compiler" editor/ --max-count 5Repository: gridaco/grida
Length of output: 11334
🏁 Script executed:
#!/bin/bash
echo "=== Checking for eslint-plugin-react-compiler in package-lock or node_modules (if exists) ==="
if [ -d "editor/node_modules" ]; then
ls -la editor/node_modules | grep -i "react-compiler"
echo ""
echo "=== Checking react-compiler specifically ==="
find editor/node_modules -name "*react-compiler*" -type d 2>/dev/null | head -5
else
echo "node_modules not available in sandbox"
fi
echo ""
echo "=== Checking eslint-config-next dependencies ==="
cat editor/package-lock.json 2>/dev/null | jq '.dependencies["eslint-config-next"].dependencies' 2>/dev/null | head -50
echo ""
echo "=== Looking for pnpm-lock or similar lock file ==="
ls -la editor/ | grep -E "lock|pnpm"Repository: gridaco/grida
Length of output: 290
🏁 Script executed:
#!/bin/bash
echo "=== Looking for pnpm-lock.yaml in root ==="
ls -la | grep -E "lock|pnpm"
echo ""
echo "=== Checking root pnpm-lock.yaml for react-compiler ==="
if [ -f "pnpm-lock.yaml" ]; then
grep -A5 "react-compiler" pnpm-lock.yaml | head -30
fi
echo ""
echo "=== Web search info: Checking what rules eslint-plugin-react-hooks actually provides ==="
echo "Standard eslint-plugin-react-hooks rules: rules-of-hooks, exhaustive-deps"
echo "Questionable rules in config: set-state-in-effect, preserve-manual-memoization, static-components, incompatible-library, purity, immutability, use-memo, refs"Repository: gridaco/grida
Length of output: 953
🏁 Script executed:
#!/bin/bash
echo "=== Searching for eslint-plugin-react-compiler in pnpm-lock.yaml ==="
grep "eslint-plugin-react-compiler" pnpm-lock.yaml
echo ""
echo "=== Searching for any reference to react-hooks or react-compiler in lock file ==="
grep -i "react.*compiler\|react.*hooks" pnpm-lock.yaml | head -20
echo ""
echo "=== Checking if eslint-config-next includes react-hooks plugin ==="
grep -A20 "eslint-config-next@16.1.3" pnpm-lock.yaml | grep -E "dependencies|react-hooks|react-compiler"Repository: gridaco/grida
Length of output: 747
🌐 Web query:
eslint-plugin-react-hooks 7.0.1 available rules
💡 Result:
In eslint-plugin-react-hooks@7.0.1, the available ESLint rules exported by the plugin are:
react-hooks/rules-of-hooks(akarules-of-hooks)react-hooks/exhaustive-deps(akaexhaustive-deps)
Sources: the package’s exported rules map for v7.0.1 [1] and React’s Hooks linting docs showing the same two rules [2].
These react-hooks rules do not exist in the installed version and will be silently ignored.
eslint-plugin-react-hooks@7.0.1 (confirmed installed in pnpm-lock.yaml) provides only two rules: rules-of-hooks and exhaustive-deps. The rules on lines 48-55 (set-state-in-effect, preserve-manual-memoization, static-components, incompatible-library, purity, immutability, use-memo, refs) are non-existent and will not be enforced.
These appear to belong to React Compiler's ESLint plugin (eslint-plugin-react-compiler), which is not installed in this project. Either:
- Install
eslint-plugin-react-compilerand change the prefix toreact-compiler/, or - Remove these rules from the configuration
🤖 Prompt for AI Agents
In `@editor/eslint.config.mjs` around lines 48 - 55, The listed rules
("react-hooks/set-state-in-effect", "react-hooks/preserve-manual-memoization",
"react-hooks/static-components", "react-hooks/incompatible-library",
"react-hooks/purity", "react-hooks/immutability", "react-hooks/use-memo",
"react-hooks/refs") in editor/eslint.config.mjs do not exist in
eslint-plugin-react-hooks and are silently ignored; either remove these entries
from the rules block or change their prefix to the correct plugin
(react-compiler/) and install that plugin: update package.json/pnpm-lock to
include eslint-plugin-react-compiler, run pnpm install, and replace each
"react-hooks/..." key with "react-compiler/..." (or simply delete the lines) so
the config matches installed plugins and active rule names.
| <button | ||
| disabled={disabled} | ||
| className={cn( | ||
| buttonVariants({ variant: "ghost", size: "icon" }), | ||
| "rounded-r-none" | ||
| )} | ||
| className={buttonVariants({ variant: "ghost", size: "icon" })} | ||
| onClick={() => onOp?.("union")} | ||
| > | ||
| <SquaresUniteIcon /> | ||
| </button> |
There was a problem hiding this comment.
Add aria-label for accessibility on icon-only button.
The button only contains an icon (<SquaresUniteIcon />) without visible text. Screen reader users won't understand the button's purpose. Consider adding an aria-label to describe the action.
♿ Suggested fix
<button
disabled={disabled}
className={buttonVariants({ variant: "ghost", size: "icon" })}
onClick={() => onOp?.("union")}
+ aria-label="Union"
>
<SquaresUniteIcon />
</button>📝 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.
| <button | |
| disabled={disabled} | |
| className={cn( | |
| buttonVariants({ variant: "ghost", size: "icon" }), | |
| "rounded-r-none" | |
| )} | |
| className={buttonVariants({ variant: "ghost", size: "icon" })} | |
| onClick={() => onOp?.("union")} | |
| > | |
| <SquaresUniteIcon /> | |
| </button> | |
| <button | |
| disabled={disabled} | |
| className={buttonVariants({ variant: "ghost", size: "icon" })} | |
| onClick={() => onOp?.("union")} | |
| aria-label="Union" | |
| > | |
| <SquaresUniteIcon /> | |
| </button> |
🤖 Prompt for AI Agents
In `@editor/scaffolds/sidecontrol/controls/ext-ops.tsx` around lines 38 - 44, The
icon-only button rendering <SquaresUniteIcon /> in ext-ops.tsx lacks an
accessible name; update the button element (the one with onClick={() =>
onOp?.("union")} and className from buttonVariants) to include an aria-label
describing the action (e.g., "Union" or a more descriptive localized string) so
screen readers can announce its purpose.
1 participant
Summary by CodeRabbit
Documentation
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.