150 improve yjs handling#151
Conversation
📝 WalkthroughWalkthroughRemoved isSynced() gating from multiple Y.js-backed primitives so operations proceed when a Y.Doc exists; tests and UI formatting were adjusted; a Mermaid diagram documenting Client–Server Y.js sync was added to the docs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as Client UI
participant Y as Y.Doc (local)
participant IDB as IndexedDB (cache)
participant WS as WebSocket Sync
participant S as Server (ProjectDoc / DB)
rect rgb(230,245,255)
note right of UI: Local-first edits
UI->>Y: Read/Write updates
Y->>IDB: Persist (Local cache)
end
rect rgb(245,245,230)
note right of Y: Sync phase (async)
Y->>WS: WebSocket sync (Yjs updates)
WS->>S: Apply to ProjectDoc / DB
S-->>WS: Acknowledge / authoritative state
WS-->>Y: Remote updates applied
end
rect rgb(235,250,235)
note right of IDB: Cache usage
IDB->>UI: Serve cached PDFs / state
S->>IDB: (via sync) Update remote PDF/state metadata
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
corates | 3d12dfd | Commit Preview URL | Dec 25 2025, 02:12 PM |
![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)
packages/web/src/primitives/useProject/pdfs.js (1)
24-38: Add explicit null checks and error handling foraddPdfToStudyreturn values.The function now returns
nullwhen YDoc is unavailable or study doesn't exist (lines 26, 64, 68), but callers don't validate the return value. This creates silent failures:
- ChecklistYjsWrapper.jsx (line 165):
setSelectedPdfId(pdfId)stores the null value directly in state, corrupting the selected PDF tracking- pdfs.js (line 159): Returns null to caller without validation
- studies.js (line 86): No null check before attempting metadata operations
Either throw an Error when YDoc is unavailable (converting the pattern to fail-fast), or add explicit null checks at each call site with user-facing error messages via
handleErrorfrom@/lib/error-utils.js(already used in ChecklistYjsWrapper for thrown errors).packages/web/src/primitives/useProject/checklists.js (2)
497-527: Update JSDoc:getQuestionNotenow always returnsY.Text, not nullable.The function now creates notes on demand (lines 524-526), ensuring it always returns a
Y.Textinstance rather than potentially returningnull. While all callers handle this gracefully (particularlyNoteEditorwhich accepts both null andY.Text), the JSDoc at line 495 should be updated from@returns {Y.Text|null}to@returns {Y.Text}to reflect the new contract.
24-196: Clarify that components guard checklist operations with sync status checks; verify Y.js conflict resolution testing for concurrent checklist edits.Components like
ReconciliationWrapperandChecklistYjsWrapperdo checkconnectionState().syncedbefore invoking operations, so operations aren't executed without sync confirmation. However, checklist operations return silently (without error feedback) when the Y.Doc is unavailable, which could mask sync failures from the UI. Additionally, while Y.js provides built-in CRDT-based conflict resolution for concurrent edits, there are no explicit tests verifying this behavior for checklist answer edits—only tests for concurrent connection management. IndexedDB persistence is adequately set up withwhenSyncedpromises.Ensure:
- Silent operation failures propagate as errors to callers or result in user-visible feedback
- Y.js conflict resolution for concurrent checklist edits is tested (especially for multi-part questions like q9/q11)
- Components handle operations that return null/undefined (currently assumed to always succeed)
🧹 Nitpick comments (2)
packages/web/src/primitives/__tests__/useProject.test.js (1)
266-285: Consider adding tests for sync failure scenarios.The test now correctly expects
pdfIdto be truthy, reflecting the new local-first behavior. However, test coverage for sync-related edge cases appears to be missing:
- Operations performed when sync is delayed or fails
- Conflict resolution between local and server state
- Data persistence if the browser closes before sync completes
These scenarios are important to validate for a local-first architecture.
packages/web/src/primitives/useProject/reconciliation.js (1)
25-49: Remove the unusedisSyncedparameter from reconciliation operations.The
isSyncedparameter is passed tocreateReconciliationOperationsbut never used within any of the reconciliation functions. UnlikeupdateStudyNameandupdateStudyDescriptioninstudies.jswhich actively guard operations withisSynced()checks, the reconciliation module doesn't reference this parameter at all.Since reconciliation progress stores only metadata references (checklist IDs and timestamps) and the actual reconciliation work is persisted in a separate reconciled checklist that syncs automatically, removing this unused parameter is safe and improves code clarity. If sync validation is needed, it should be enforced at the caller level—
ReconciliationWrapperalready checksconnectionState().syncedbefore critical operations.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
docs/architecture/diagrams/08-yjs-sync.mdpackages/ui/src/__tests__/setup.tspackages/ui/src/components/Combobox.tsxpackages/ui/src/components/Drawer.tsxpackages/ui/src/components/Tour.tsxpackages/ui/src/components/__tests__/RadioGroup.test.tsxpackages/web/src/primitives/__tests__/useProject.test.jspackages/web/src/primitives/useProject/checklists.jspackages/web/src/primitives/useProject/pdfs.jspackages/web/src/primitives/useProject/reconciliation.jspackages/web/src/primitives/useProject/studies.js
💤 Files with no reviewable changes (1)
- packages/ui/src/components/Combobox.tsx
🧰 Additional context used
📓 Path-based instructions (13)
**/*
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use emojis in code, comments, documentation, or commit messages
NEVER use emojis anywhere in code, comments, documentation, plan files, or commit messages. This includes unicode symbols. For UI icons, use solid-icons library or SVGs only.
Files:
packages/ui/src/components/Drawer.tsxpackages/web/src/primitives/useProject/reconciliation.jspackages/ui/src/components/Tour.tsxpackages/ui/src/components/__tests__/RadioGroup.test.tsxdocs/architecture/diagrams/08-yjs-sync.mdpackages/web/src/primitives/useProject/studies.jspackages/ui/src/__tests__/setup.tspackages/web/src/primitives/useProject/checklists.jspackages/web/src/primitives/__tests__/useProject.test.jspackages/web/src/primitives/useProject/pdfs.js
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,ts,tsx}: Prefer modern ES6+ syntax and features
Use aliases for imports when appropriate to improve readability
**/*.{js,jsx,ts,tsx}: Prefer modern ES6+ syntax and features in JavaScript/TypeScript code
Comments should explain why something is being done, not narrate what the code does. Avoid comments that repeat variable names or describe obvious code behavior.
Files:
packages/ui/src/components/Drawer.tsxpackages/web/src/primitives/useProject/reconciliation.jspackages/ui/src/components/Tour.tsxpackages/ui/src/components/__tests__/RadioGroup.test.tsxpackages/web/src/primitives/useProject/studies.jspackages/ui/src/__tests__/setup.tspackages/web/src/primitives/useProject/checklists.jspackages/web/src/primitives/__tests__/useProject.test.jspackages/web/src/primitives/useProject/pdfs.js
packages/{web,ui}/src/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/corates.mdc)
Group related components in subdirectories with barrel exports
Files:
packages/ui/src/components/Drawer.tsxpackages/ui/src/components/Tour.tsxpackages/ui/src/components/__tests__/RadioGroup.test.tsx
packages/{web,ui}/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)
packages/{web,ui}/**/*.{js,jsx,ts,tsx}: Import UI components from '@corates/ui' package instead of local component files. Do not import Ark UI components from local paths like '@/components/zag/' or 'packages/web/src/components/zag/'
Always use 'solid-icons' library for icons. Never use emoji characters or text as icon replacements. Import from specific icon sets like 'solid-icons/bi', 'solid-icons/fi', 'solid-icons/ai', etc.
Files:
packages/ui/src/components/Drawer.tsxpackages/web/src/primitives/useProject/reconciliation.jspackages/ui/src/components/Tour.tsxpackages/ui/src/components/__tests__/RadioGroup.test.tsxpackages/web/src/primitives/useProject/studies.jspackages/ui/src/__tests__/setup.tspackages/web/src/primitives/useProject/checklists.jspackages/web/src/primitives/__tests__/useProject.test.jspackages/web/src/primitives/useProject/pdfs.js
packages/{web,ui}/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)
Use Tailwind CSS classes for styling components
Files:
packages/ui/src/components/Drawer.tsxpackages/ui/src/components/Tour.tsxpackages/ui/src/components/__tests__/RadioGroup.test.tsx
packages/web/src/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
packages/web/src/**/*.{js,jsx,ts,tsx}: For UI icons, use thesolid-iconslibrary or SVGs only. Do not use emojis
Ensure browser compatibility for all frontend code (Safari is usually problematic)
Keep files small, focused, and modular. If a file exceeds a high number of lines, consider refactoring by extracting sub-modules into a folder with index.jsx and helper components, moving complex logic into separate utility files or primitives, or splitting large forms into section components
Do NOT prop-drill application state. Shared or cross-feature state must live in external stores under packages/web/src/stores/ or relative to the component file
UsecreateMemofor derived values to ensure they update reactivelyUse import aliases from jsconfig.json instead of relative paths
Files:
packages/web/src/primitives/useProject/reconciliation.jspackages/web/src/primitives/useProject/studies.jspackages/web/src/primitives/useProject/checklists.jspackages/web/src/primitives/__tests__/useProject.test.jspackages/web/src/primitives/useProject/pdfs.js
packages/web/src/primitives/**/*.{js,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Create reusable logic in primitives (hooks) that can be shared across components to keep components clean and focused on rendering
Files:
packages/web/src/primitives/useProject/reconciliation.jspackages/web/src/primitives/useProject/studies.jspackages/web/src/primitives/useProject/checklists.jspackages/web/src/primitives/__tests__/useProject.test.jspackages/web/src/primitives/useProject/pdfs.js
packages/web/src/**/*.{jsx,tsx,js,ts}
📄 CodeRabbit inference engine (.cursor/rules/corates.mdc)
In SolidJS, use createMemo for derived values
Files:
packages/web/src/primitives/useProject/reconciliation.jspackages/web/src/primitives/useProject/studies.jspackages/web/src/primitives/useProject/checklists.jspackages/web/src/primitives/__tests__/useProject.test.jspackages/web/src/primitives/useProject/pdfs.js
packages/web/src/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
packages/web/src/**/*.{js,ts,jsx,tsx}: Always usehandleFetchErrorfrom@/lib/error-utils.jsfor fetch calls in frontend code with options like{ showToast: true }for error handling
UsecreateFormErrorSignalsfrom@/lib/form-errors.jsfor form validation error handling with field-level and global error management
Files:
packages/web/src/primitives/useProject/reconciliation.jspackages/web/src/primitives/useProject/studies.jspackages/web/src/primitives/useProject/checklists.jspackages/web/src/primitives/__tests__/useProject.test.jspackages/web/src/primitives/useProject/pdfs.js
packages/{web,workers}/src/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
packages/{web,workers}/src/**/*.{js,ts,jsx,tsx}: Never throw string literals; always throw Error objects or return domain errors from API routes
Use error utility functions likeisErrorCodefrom@corates/sharedor@/lib/error-utils.jsto check specific error types instead of manual string comparisons
Files:
packages/web/src/primitives/useProject/reconciliation.jspackages/web/src/primitives/useProject/studies.jspackages/web/src/primitives/useProject/checklists.jspackages/web/src/primitives/__tests__/useProject.test.jspackages/web/src/primitives/useProject/pdfs.js
{packages/web/**,packages/landing/**}/**/*.{jsx,tsx,js,ts}
📄 CodeRabbit inference engine (.cursor/rules/solidjs.mdc)
{packages/web/**,packages/landing/**}/**/*.{jsx,tsx,js,ts}: Never destructure props in SolidJS components - destructuring breaks reactivity. Access props directly (e.g.,props.name) or wrap in a function (e.g.,const name = () => props.name) to maintain reactivity.
Import stores directly in components rather than prop-drilling store data through component hierarchies.
Use separate read and write patterns for stores: import the store directly for reading data (e.g.,projectStore.getProjectList()) and import action stores separately for writing (e.g.,projectActionsStore.createProject()).
UsecreateSignalfrom solid-js for managing simple reactive values. Prefer derived state with signals or memo over effects when possible.
UsecreateStorefrom solid-js/store for managing complex objects and arrays that require granular reactivity, enabling fine-grained updates where only affected parts re-render.
UsecreateMemofrom solid-js for derived values that depend on reactive state, ensuring computed values update only when their dependencies change.
Always clean up effects that create subscriptions or timers using theonCleanupfunction from solid-js. Use effects sparingly, only when derived values won't work well.
Keep components lean and focused on rendering. Move business logic to stores (for shared state and operations), primitives (for reusable hooks/logic), or utilities (for pure functions).
Use theShowcomponent from solid-js for conditional rendering instead of JavaScript ternary operators or logical AND operators.
Use theForcomponent from solid-js for rendering lists. It provides better performance and keying compared to JavaScript's map function in JSX.
When manipulating children in wrapper components, use thechildrenhelper from solid-js to ensure proper reactivity and handling of child elements.
Files:
packages/web/src/primitives/useProject/reconciliation.jspackages/web/src/primitives/useProject/studies.jspackages/web/src/primitives/useProject/checklists.jspackages/web/src/primitives/__tests__/useProject.test.jspackages/web/src/primitives/useProject/pdfs.js
{packages/web/**,packages/landing/**}/src/primitives/**/*.{jsx,tsx,js,ts}
📄 CodeRabbit inference engine (.cursor/rules/solidjs.mdc)
Create reusable component logic in primitives (custom hooks) rather than embedding business logic directly in components. Primitives should encapsulate store access and complex logic patterns.
Files:
packages/web/src/primitives/useProject/reconciliation.jspackages/web/src/primitives/useProject/studies.jspackages/web/src/primitives/useProject/checklists.jspackages/web/src/primitives/__tests__/useProject.test.jspackages/web/src/primitives/useProject/pdfs.js
packages/web/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)
Use import aliases from 'packages/web/jsconfig.json' instead of relative paths. Aliases include: '@/' (src/), '@components/' (src/components/), '@auth-ui/' (src/components/auth-ui/), '@checklist-ui/' (src/components/checklist-ui/), '@project-ui/' (src/components/project-ui/), '@routes/' (src/routes/), '@primitives/' (src/primitives/), '@api/' (src/api/), '@config/' (src/config/), and '@lib/' (src/lib/)
Files:
packages/web/src/primitives/useProject/reconciliation.jspackages/web/src/primitives/useProject/studies.jspackages/web/src/primitives/useProject/checklists.jspackages/web/src/primitives/__tests__/useProject.test.jspackages/web/src/primitives/useProject/pdfs.js
🧠 Learnings (2)
📚 Learning: 2025-12-24T17:22:48.927Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/corates.mdc:0-0
Timestamp: 2025-12-24T17:22:48.927Z
Learning: Applies to packages/{web,landing}/src/**/*.{jsx,tsx} : Use Ark UI components from corates/ui package, not local component implementations
Applied to files:
packages/ui/src/components/Drawer.tsx
📚 Learning: 2025-12-24T17:23:17.309Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/ui-components.mdc:0-0
Timestamp: 2025-12-24T17:23:17.309Z
Learning: Applies to packages/{web,ui}/**/*.{js,jsx,ts,tsx} : Import UI components from 'corates/ui' package instead of local component files. Do not import Ark UI components from local paths like '@/components/zag/' or 'packages/web/src/components/zag/'
Applied to files:
packages/ui/src/components/Drawer.tsx
🧬 Code graph analysis (2)
packages/ui/src/components/Drawer.tsx (1)
packages/ui/src/index.ts (1)
Z_INDEX(13-13)
packages/web/src/primitives/useProject/pdfs.js (4)
packages/web/src/lib/referenceLookup.js (1)
pdfInfo(406-406)packages/web/src/lib/referenceParser.js (1)
tag(28-28)packages/web/src/primitives/useProject/index.js (1)
getYDoc(171-171)packages/workers/src/durable-objects/__tests__/ProjectDoc.test.js (2)
studiesMap(25-25)studyYMap(35-35)
⏰ 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: Workers Builds: corates
🔇 Additional comments (6)
packages/ui/src/components/__tests__/RadioGroup.test.tsx (1)
64-66: LGTM - Formatting changes only.The trailing comma additions in the
Array.findcallbacks are consistent with modern JavaScript formatting conventions and have no impact on test behavior.Also applies to: 81-83, 96-98, 160-162, 232-234
packages/ui/src/__tests__/setup.ts (1)
66-68: LGTM - Formatting change only.The trailing comma addition is a minor formatting adjustment with no impact on the mock behavior.
packages/ui/src/components/Drawer.tsx (1)
88-121: LGTM - Formatting adjustments with proper accessibility and icon usage.The changes are formatting-only (quote style normalization, class ordering). The component correctly:
- Uses
FiXfromsolid-icons/fiper coding guidelines- Includes proper accessibility attributes (
role='dialog',aria-modal='true',aria-label)- Uses Ark UI components from the package
packages/ui/src/components/Tour.tsx (1)
153-167: LGTM - Formatting improvement with complex type inference.The reformatting improves readability. The conditional type inference extracts the correct return type for the
useMachineoptions parameter. While the type assertion is complex, it ensures type safety for the Zag.js tour machine configuration.docs/architecture/diagrams/08-yjs-sync.md (1)
1-21: Helpful architecture documentation.This diagram effectively illustrates the local-first Y.js synchronization model, providing valuable context for the sync guard changes throughout the codebase.
packages/web/src/primitives/useProject/studies.js (1)
24-63: Document the intentional sync-check pattern to prevent confusion.The sync check inconsistency exists and appears intentional: pure Y.Doc operations (
createStudy,updateStudy,deleteStudy,updateProjectSettings) skip sync validation for local-first collaboration, while server-backed operations (renameProject,updateDescription) checkisSynced()before updating Y.Doc to prevent conflicting edits. However, this design distinction is undocumented and could lead to confusion.The current pattern means server-backed operations always update
projectStore(keeping UI in sync with server state) but conditionally update Y.Doc only if synced—creating possible temporary inconsistency between Y.Doc and store if sync breaks during/after a server operation. Add comments explaining:
- Why pure Y.Doc ops are sync-independent
- Why server-backed ops check sync before Y.Doc updates
- That store is the authoritative local state when Y.Doc sync is unavailable
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/ui/src/components/Tabs.tsx (1)
65-65: Styling approach deviates from coding guidelines.The removal of inline Tailwind classes (
data-[selected]:...) in favor of global CSS rules contradicts the coding guideline: "Use Tailwind CSS classes for styling components."While moving these styles to
global.csscan improve maintainability for shared component states, it deviates from the established pattern. Consider whether this is an intentional architectural shift or if the styles should remain as inline Tailwind classes per the guidelines.As per coding guidelines, Tailwind classes are preferred for component styling.
Also applies to: 72-72
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/ui/src/components/Tabs.tsxpackages/web/src/global.css
🧰 Additional context used
📓 Path-based instructions (5)
**/*
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use emojis in code, comments, documentation, or commit messages
NEVER use emojis anywhere in code, comments, documentation, plan files, or commit messages. This includes unicode symbols. For UI icons, use solid-icons library or SVGs only.
Files:
packages/web/src/global.csspackages/ui/src/components/Tabs.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,ts,tsx}: Prefer modern ES6+ syntax and features
Use aliases for imports when appropriate to improve readability
**/*.{js,jsx,ts,tsx}: Prefer modern ES6+ syntax and features in JavaScript/TypeScript code
Comments should explain why something is being done, not narrate what the code does. Avoid comments that repeat variable names or describe obvious code behavior.
Files:
packages/ui/src/components/Tabs.tsx
packages/{web,ui}/src/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/corates.mdc)
Group related components in subdirectories with barrel exports
Files:
packages/ui/src/components/Tabs.tsx
packages/{web,ui}/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)
packages/{web,ui}/**/*.{js,jsx,ts,tsx}: Import UI components from '@corates/ui' package instead of local component files. Do not import Ark UI components from local paths like '@/components/zag/' or 'packages/web/src/components/zag/'
Always use 'solid-icons' library for icons. Never use emoji characters or text as icon replacements. Import from specific icon sets like 'solid-icons/bi', 'solid-icons/fi', 'solid-icons/ai', etc.
Files:
packages/ui/src/components/Tabs.tsx
packages/{web,ui}/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)
Use Tailwind CSS classes for styling components
Files:
packages/ui/src/components/Tabs.tsx
🧠 Learnings (12)
📚 Learning: 2025-12-24T17:23:10.082Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/solidjs.mdc:0-0
Timestamp: 2025-12-24T17:23:10.082Z
Learning: Applies to {packages/web/**,packages/landing/**}/**/*.{jsx,tsx,js,ts} : Use `createMemo` from solid-js for derived values that depend on reactive state, ensuring computed values update only when their dependencies change.
Applied to files:
packages/ui/src/components/Tabs.tsx
📚 Learning: 2025-12-24T17:22:48.927Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/corates.mdc:0-0
Timestamp: 2025-12-24T17:22:48.927Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx,js,ts} : In SolidJS, use createMemo for derived values
Applied to files:
packages/ui/src/components/Tabs.tsx
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/**/*.{js,jsx,ts,tsx} : Use `createMemo` for derived values to ensure they update reactively
Applied to files:
packages/ui/src/components/Tabs.tsx
📚 Learning: 2025-12-24T17:23:10.082Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/solidjs.mdc:0-0
Timestamp: 2025-12-24T17:23:10.082Z
Learning: Applies to {packages/web/**,packages/landing/**}/**/*.{jsx,tsx,js,ts} : Use `createSignal` from solid-js for managing simple reactive values. Prefer derived state with signals or memo over effects when possible.
Applied to files:
packages/ui/src/components/Tabs.tsx
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/components/**/*.{js,jsx,ts,tsx} : Do not destructure props in SolidJS components as it breaks reactivity. Instead, access props directly from the props object or wrap them in a function to ensure they are always up-to-date
Applied to files:
packages/ui/src/components/Tabs.tsx
📚 Learning: 2025-12-24T17:23:10.082Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/solidjs.mdc:0-0
Timestamp: 2025-12-24T17:23:10.082Z
Learning: Applies to {packages/web/**,packages/landing/**}/**/*.{jsx,tsx,js,ts} : Use `createStore` from solid-js/store for managing complex objects and arrays that require granular reactivity, enabling fine-grained updates where only affected parts re-render.
Applied to files:
packages/ui/src/components/Tabs.tsx
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/primitives/**/*.{js,ts} : Create reusable logic in primitives (hooks) that can be shared across components to keep components clean and focused on rendering
Applied to files:
packages/ui/src/components/Tabs.tsx
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/components/**/*.{js,jsx,ts,tsx} : Components should receive at most 1–5 props, and only for local configuration, not shared state. If a component would need more than 5 props, move the shared data into an external store, a primitive, or Solid context
Applied to files:
packages/ui/src/components/Tabs.tsx
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/components/**/*.{js,jsx,ts,tsx} : Use responsive design principles for UI components
Applied to files:
packages/ui/src/components/Tabs.tsx
📚 Learning: 2025-12-24T17:23:10.082Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/solidjs.mdc:0-0
Timestamp: 2025-12-24T17:23:10.082Z
Learning: Applies to {packages/web/**,packages/landing/**}/**/*.{jsx,tsx,js,ts} : Never destructure props in SolidJS components - destructuring breaks reactivity. Access props directly (e.g., `props.name`) or wrap in a function (e.g., `const name = () => props.name`) to maintain reactivity.
Applied to files:
packages/ui/src/components/Tabs.tsx
📚 Learning: 2025-12-24T17:23:10.082Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/solidjs.mdc:0-0
Timestamp: 2025-12-24T17:23:10.082Z
Learning: Applies to {packages/web/**,packages/landing/**}/**/*.{jsx,tsx,js,ts} : Use the `Show` component from solid-js for conditional rendering instead of JavaScript ternary operators or logical AND operators.
Applied to files:
packages/ui/src/components/Tabs.tsx
📚 Learning: 2025-12-24T17:23:10.082Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/solidjs.mdc:0-0
Timestamp: 2025-12-24T17:23:10.082Z
Learning: Applies to {packages/web/**,packages/landing/**}/**/*.{jsx,tsx,js,ts} : Use the `For` component from solid-js for rendering lists. It provides better performance and keying compared to JavaScript's map function in JSX.
Applied to files:
packages/ui/src/components/Tabs.tsx
🧬 Code graph analysis (1)
packages/ui/src/components/Tabs.tsx (1)
packages/ui/src/components/index.ts (1)
Tabs(27-27)
⏰ 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: Workers Builds: corates
🔇 Additional comments (4)
packages/ui/src/components/Tabs.tsx (3)
6-6: LGTM!Good use of
createMemofor derived values, aligning with SolidJS best practices. Based on learnings, this ensures the computed props update reactively when dependencies change.
59-59: LGTM!Clean refactor to spread the computed
rootProps()instead of explicit props. This works well with the new conditional controlled/uncontrolled logic.
47-56: Handle empty string values correctly in controlled mode.The condition on line 49 (
currentValue !== undefined && currentValue !== null) doesn't handle empty strings correctly. If a parent passesvalue="", this is a valid tab value but would be treated as uncontrolled mode, causing the component to fall back todefaultValue.🔎 Proposed fix to handle empty strings
const rootProps = createMemo(() => { const currentValue = value(); - if (currentValue !== undefined && currentValue !== null) { + if (currentValue !== undefined) { return { value: currentValue, onValueChange: handleValueChange }; } return { defaultValue: defaultValue() || tabsList()[0]?.value, onValueChange: handleValueChange, }; });This change treats only
undefinedas the absence of a controlled value, allowingnull,"", and other falsy values to work correctly in controlled mode.⛔ Skipped due to learnings
Learnt from: CR Repo: InfinityBowman/corates PR: 0 File: .cursor/rules/solidjs.mdc:0-0 Timestamp: 2025-12-24T17:23:10.082Z Learning: Applies to {packages/web/**,packages/landing/**}/**/*.{jsx,tsx,js,ts} : Use `createMemo` from solid-js for derived values that depend on reactive state, ensuring computed values update only when their dependencies change.Learnt from: CR Repo: InfinityBowman/corates PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-12-19T14:49:49.730Z Learning: Applies to packages/web/src/**/*.{js,jsx,ts,tsx} : Use `createMemo` for derived values to ensure they update reactivelyLearnt from: CR Repo: InfinityBowman/corates PR: 0 File: .cursor/rules/corates.mdc:0-0 Timestamp: 2025-12-24T17:22:48.927Z Learning: Applies to packages/web/src/**/*.{jsx,tsx,js,ts} : In SolidJS, use createMemo for derived valuespackages/web/src/global.css (1)
154-165: No action needed—the CSS selectors and styling approach are correct.The data attributes (
data-scope='tabs',data-part='trigger',data-selected) are correctly set by Ark UI v5.30.0, as confirmed by the component tests. The!importantdeclarations on lines 163–164 are necessary and justified: they override Tailwind utility classes (bg-gray-100,text-gray-600) applied to the badge element within the selected tab trigger, which have inherent high specificity. Increasing CSS selector specificity further would be impractical, making!importantthe appropriate solution. This approach is consistent with other style overrides in the file.
2 participants
Summary by CodeRabbit
New Features
Improvements
Style
✏️ Tip: You can customize this high-level summary in your review settings.