feat: cell selection feature#6455
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds the table-core cell-selection feature with rectangular multi-range state, pointer and keyboard APIs, framework examples, tests, guides, API reference pages, navigation metadata, and related row-aggregation type renames. ChangesCell Selection Feature
Estimated code review effort: 5 (Critical) | ~120 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
|
View your CI Pipeline Execution ↗ for commit 4fc39a7
☁️ Nx Cloud last updated this comment at |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 9
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (27)
packages/table-core/skills/cell-selection/SKILL.md-136-138 (1)
136-138: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRewrite the performance guidance sentence for clarity.
“With a table-wide subscription that reconciles every cell each time.” is a sentence fragment. For example: “A table-wide subscription reconciles every cell on each drag update; subscribe per row instead.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/table-core/skills/cell-selection/SKILL.md` around lines 136 - 138, Rewrite the performance guidance in the cell-selection documentation to replace the sentence fragment with a complete, clear sentence contrasting table-wide subscriptions with per-row subscriptions. Preserve the existing recommendation to subscribe per row and the surrounding details about cellSelection, getCellSelectionBounds(), and affected neighboring rows.packages/table-core/skills/cell-selection/SKILL.md-44-44 (1)
44-44: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winDescribe this as a two-corner or four-field state.
CellSelectionRangecontains four scalar IDs (anchorRowId,anchorColumnId,focusRowId, andfocusColumnId). Calling this a “two-field write” contradicts the documented API shape; use “two-corner write” or “four-field write” instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/table-core/skills/cell-selection/SKILL.md` at line 44, Update the cell-selection explanation to describe the range update as a “two-corner write” or “four-field write,” replacing “two-field write.” Preserve the existing explanation of the anchor and focus corners and the CellSelectionRange state.packages/table-core/tests/implementation/features/cell-selection/cellSelectionFeature.test.ts-137-153 (1)
137-153: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winThis test doesn't actually exercise controlled state.
state: { cellSelection: state }captures the initial[]value; reassigning the localstateinside the handler never feeds back into the table'sstateoption, so the table'scellSelectionstays empty throughout. The assertions only inspectseen, so the test passes without proving the controlled round-trip. DrivingsetOptionsfrom the handler (or assertingtable.atoms.cellSelection.get()after each write) would make it meaningful.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/table-core/tests/implementation/features/cell-selection/cellSelectionFeature.test.ts` around lines 137 - 153, The test using makeTable does not feed updated cell selection state back into the table, so it does not verify controlled-state behavior. Update the onCellSelectionChange handler in “routes writes through onCellSelectionChange when provided” to propagate the resolved state through table.setOptions, or otherwise update table.atoms.cellSelection, and assert the table’s state after the write while preserving the existing seen assertion.packages/table-core/src/features/cell-selection/cellSelectionFeature.utils.ts-642-675 (1)
642-675: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winNavigation dead-ends when the current column is opted out.
columns.findIndexsearches only selectable columns, so if the active anchor sits on a column withenableCellSelection: false(reachable viasetFocusedCell, which does not consult the flag),columnIndexis-1and every arrow key becomes a no-op — including vertical moves that don't need the column at all. Consider resolving the row/column indexes independently, or snapping to the nearest selectable column.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/table-core/src/features/cell-selection/cellSelectionFeature.utils.ts` around lines 642 - 675, Update stepCoordinate so row navigation remains functional when the current column is not selectable: resolve the row index independently and handle the active column outside getSelectableColumns, snapping to an appropriate selectable column when horizontal movement requires one while preserving vertical movement in the current column where valid. Ensure anchors on opted-out columns no longer make every arrow-key navigation return null.packages/table-core/src/features/cell-selection/cellSelectionFeature.utils.ts-1044-1059 (1)
1044-1059: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winDrag flag can latch on when no document is available.
table._isSelectingCellsis set todragEnabledbefore thecontextDocumentcheck, so in a non-DOM/SSR context (or when an explicitly passed document is absent) the flag staystruewith nomouseuplistener to clear it — latermouseenterhandlers then extend the range without a real drag. Setting the flag only when a listener is actually attached would keep the two in sync.♻️ Proposed change
- // the open-drag flag is instance data, so closing it never writes state - // `@ts-ignore` - _isSelectingCells is part of the CellSelection feature - table._isSelectingCells = dragEnabled - - if (dragEnabled && contextDocument) { + const dragStarted = dragEnabled && !!contextDocument + + // the open-drag flag is instance data, so closing it never writes state + // `@ts-ignore` - _isSelectingCells is part of the CellSelection feature + table._isSelectingCells = dragStarted + + if (dragStarted && contextDocument) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/table-core/src/features/cell-selection/cellSelectionFeature.utils.ts` around lines 1044 - 1059, Update the cell-drag initialization around _isSelectingCells so the flag is set to true only when dragEnabled and contextDocument are both available and the mouseup listener is attached; otherwise leave it false. Preserve the mouseup cleanup that resets table._isSelectingCells.docs/framework/alpine/guide/cell-selection.md-319-319 (1)
319-319: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse framework-neutral effect wording.
useEffectis a React hook, so this is misleading in the Alpine guide. Refer to an Alpine reactive effect/subscription or simply a userland layout-change handler.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/framework/alpine/guide/cell-selection.md` at line 319, Update the cell-selection guidance around the layout-change example to remove the React-specific useEffect reference. Describe the solution using framework-neutral wording, such as a userland layout-change handler or Alpine reactive effect/subscription.examples/alpine/cell-selection/src/index.css-114-114 (1)
114-114: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the reported Stylelint violations.
Use
currentcolorand add the required blank line beforebox-shadow; the current file fails Stylelint.Proposed fix
.demo-button, .page-size-input, .filter-input { - border: 1px solid currentColor; + border: 1px solid currentcolor; border-radius: 0.25rem; } .cell-selectable { user-select: none; cursor: cell; --cell-edge-top: 0 0 0 0 transparent; --cell-edge-right: 0 0 0 0 transparent; --cell-edge-bottom: 0 0 0 0 transparent; --cell-edge-left: 0 0 0 0 transparent; + box-shadow:Also applies to: 160-164
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/alpine/cell-selection/src/index.css` at line 114, Update the affected style rules in index.css, including the declarations around the currentColor border and the additional occurrences near lines 160-164: use the Stylelint-compliant lowercase currentcolor value and insert the required blank line before box-shadow declarations.Source: Linters/SAST tools
examples/react/cell-selection/src/main.tsx-523-533 (1)
523-533: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
rowSelectionKeyencodes the anchor cell while focus styling follows the focus cell. The same helper was copied into both examples, so the memoization key can stay identical across a focus-only move whose union bounds are unchanged, leaving a row'scell-focusedoutline stale.
examples/react/cell-selection/src/main.tsx#L523-L533: key onactive.focusRowId/active.focusColumnIdinstead of the anchor ids.examples/preact/cell-selection/src/main.tsx#L523-L533: apply the identical change so both examples stay in sync.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/react/cell-selection/src/main.tsx` around lines 523 - 533, Update rowSelectionKey in examples/react/cell-selection/src/main.tsx:523-533 to build the selection key from active.focusRowId and active.focusColumnId rather than the anchor identifiers. Apply the identical change in examples/preact/cell-selection/src/main.tsx:523-533 so both examples invalidate memoization when focus moves.docs/framework/svelte/guide/cell-selection.md-304-304 (1)
304-304: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winReplace copied React-specific effect wording. Both guides describe their framework-specific layout-reset mechanism as
useEffect, although neither example uses React.
docs/framework/svelte/guide/cell-selection.md#L304-L304: replaceuseEffectwith$effect.docs/framework/vue/guide/cell-selection.md#L302-L302: replaceuseEffectwith a Vue watcher orwatch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/framework/svelte/guide/cell-selection.md` at line 304, Update the layout-reset wording in docs/framework/svelte/guide/cell-selection.md at lines 304-304 to refer to Svelte’s $effect instead of useEffect. Also update docs/framework/vue/guide/cell-selection.md at lines 302-302 to refer to a Vue watcher or watch, with no other changes required.examples/angular/cell-selection/src/styles.css-123-137 (1)
123-137: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the configured Stylelint violations. These files currently fail linting due to
currentColorcasing and missing blank lines beforebox-shadow.
examples/angular/cell-selection/src/styles.css#L123-L137: use the configuredcurrentcolorkeyword casing.examples/angular/cell-selection/src/styles.css#L215-L219: use the configuredcurrentcolorkeyword casing.examples/angular/cell-selection/src/styles.css#L318-L324: use the configuredcurrentcolorkeyword casing.examples/angular/cell-selection/src/styles.css#L385-L396: insert the required blank line beforebox-shadow.examples/ember/cell-selection/app/app.css#L213-L223: use the configuredcurrentcolorkeyword casing.examples/ember/cell-selection/app/app.css#L321-L332: insert the required blank line beforebox-shadow.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/angular/cell-selection/src/styles.css` around lines 123 - 137, Fix the Stylelint violations in the listed sites: in examples/angular/cell-selection/src/styles.css lines 123-137, 215-219, and 318-324, and examples/ember/cell-selection/app/app.css lines 213-223, change currentColor to the configured currentcolor casing; in examples/angular/cell-selection/src/styles.css lines 385-396 and examples/ember/cell-selection/app/app.css lines 321-332, insert the required blank line immediately before box-shadow.Source: Linters/SAST tools
examples/preact/cell-selection/src/index.css-142-142 (1)
142-142: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winResolve the reported Stylelint violations before merge.
examples/preact/cell-selection/src/index.css#L142-L142: changecurrentColortocurrentcolor.examples/preact/cell-selection/src/index.css#L223-L223: changecurrentColortocurrentcolor.examples/preact/cell-selection/src/index.css#L328-L328: changecurrentColortocurrentcolor.examples/preact/cell-selection/src/index.css#L399-L403: insert the required empty line beforebox-shadow.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/preact/cell-selection/src/index.css` at line 142, Resolve the Stylelint violations in examples/preact/cell-selection/src/index.css: replace currentColor with currentcolor at lines 142, 223, and 328, and insert the required empty line before box-shadow at lines 399-403.Source: Linters/SAST tools
examples/preact/cell-selection/index.html-7-7 (1)
7-7: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse a cell-selection-specific page title.
The browser tab currently shows
Vite + Preactrather than identifying this TanStack Table example.Proposed fix
- <title>Vite + Preact</title> + <title>TanStack Table Preact Cell Selection</title>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/preact/cell-selection/index.html` at line 7, Update the document title in the cell-selection example page from the generic Vite + Preact label to a title that identifies the TanStack Table cell-selection example.examples/preact/cell-selection/tests/e2e/smoke.spec.ts-6-6 (1)
6-6: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winResolve
exampleDirfrom the test location instead ofprocess.cwd().
path.resolve()makes this depend on where Playwright is invoked; the sharedstartExampleServerthen reads the repo root’svite.config.tsinstead of this example’s Vite config. Use animport.meta.url-based path two levels abovetests/e2eunless the runner explicitly changes directories.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/preact/cell-selection/tests/e2e/smoke.spec.ts` at line 6, Update the exampleDir initialization in smoke.spec.ts to derive the example directory from the test module’s import.meta.url, resolving two levels above tests/e2e. Remove the path.resolve() dependence on process.cwd() so startExampleServer uses this example’s Vite configuration.examples/react/cell-selection/src/index.css-135-135 (1)
135-135: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winResolve the repeated Stylelint keyword-casing errors. The configured rule requires
currentcolor, so all three declarations currently fail lint.
examples/react/cell-selection/src/index.css#L135-L135: changecurrentColortocurrentcolor.examples/react/cell-selection/src/index.css#L216-L216: changecurrentColortocurrentcolor.examples/react/cell-selection/src/index.css#L321-L321: changecurrentColortocurrentcolor.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/react/cell-selection/src/index.css` at line 135, Update the border declarations in examples/react/cell-selection/src/index.css at lines 135-135, 216-216, and 321-321, replacing currentColor with the required lowercase currentcolor keyword to satisfy Stylelint.Source: Linters/SAST tools
examples/react/cell-selection/src/index.css-392-396 (1)
392-396: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the Stylelint declaration-spacing error.
Add the required blank line before
box-shadow; this currently failsdeclaration-empty-line-before.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/react/cell-selection/src/index.css` around lines 392 - 396, Add the required empty line immediately before the box-shadow declaration in the cell-selection stylesheet, preserving the existing shadow values and formatting.Source: Linters/SAST tools
examples/react/mantine-react-table/src/mantine-react-table/hooks/useMRT_TableOptions.ts-128-129 (1)
128-129: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRecompute when
aggregationFnschanges.This memoized option captures the caller’s custom aggregation functions from the first render; passing new aggregation functions later is ignored. Add
aggregationFnsto the dependency list unless it is intentionally initialization-only.Suggested fix
aggregationFns = useMemo( () => ({ ...MRT_RowAggregationFns, ...aggregationFns }), - [], + [aggregationFns], )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/react/mantine-react-table/src/mantine-react-table/hooks/useMRT_TableOptions.ts` around lines 128 - 129, Update the aggregation-functions memo in useMRT_TableOptions so it recomputes when the caller-provided aggregationFns changes by adding aggregationFns to the dependency list, while preserving the merge with MRT_RowAggregationFns.examples/solid/cell-selection/src/index.css-135-135 (1)
135-135: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winResolve the reported Stylelint errors.
Use
currentcolorat the three reported declarations and insert the required blank line beforebox-shadow; the configured Stylelint check reports all four locations as errors.Proposed fix
- border: 1px solid currentColor; + border: 1px solid currentcolor; ... - border: 1px solid currentColor; + border: 1px solid currentcolor; ... - border: 1px solid currentColor; + border: 1px solid currentcolor; ... --cell-edge-bottom: 0 0 0 0 transparent; --cell-edge-left: 0 0 0 0 transparent; + box-shadow:Also applies to: 216-216, 321-321, 392-396
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/solid/cell-selection/src/index.css` at line 135, Update the three reported border declarations to use the Stylelint-compliant lowercase currentcolor value, and add the required blank line before box-shadow in the declaration block near the final reported location. Preserve all other styling unchanged.Source: Linters/SAST tools
examples/solid/cell-selection/index.html-7-7 (1)
7-7: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRename the page title to cell selection.
The browser tab and bookmarks currently label this as a row-selection example.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/solid/cell-selection/index.html` at line 7, Update the title element in the Solid selection example so it identifies the page as “Cell Selection” rather than “Solid Row Selection,” ensuring the browser tab and bookmarks use the correct label.examples/svelte/cell-selection/index.html-6-6 (1)
6-6: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCorrect the example title.
The page is for cell selection, but the title still says “Row Selection Example”.
Proposed fix
- <title>Svelte Row Selection Example</title> + <title>Svelte Cell Selection Example</title>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/svelte/cell-selection/index.html` at line 6, Update the title element in the cell selection example from “Svelte Row Selection Example” to accurately identify it as the Svelte Cell Selection Example.examples/svelte/cell-selection/src/index.css-135-135 (1)
135-135: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the reported Stylelint errors.
Use
currentcolorat lines 135, 216, and 321, and add the required blank line beforebox-shadowat line 392.Also applies to: 216-216, 321-321, 392-396
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/svelte/cell-selection/src/index.css` at line 135, Update the CSS declarations at the referenced border lines to use the lowercase currentcolor keyword, and add the required blank line immediately before the box-shadow declaration in the relevant style block. Preserve all other styles unchanged.Source: Linters/SAST tools
examples/vue/cell-selection/index.html-7-7 (1)
7-7: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winSet a descriptive page title.
Replace the generic
Vite Apptitle with the example name, such asCell Selection, so the browser tab and accessibility context identify the demo correctly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/vue/cell-selection/index.html` at line 7, Update the HTML document title from the generic “Vite App” value to a descriptive “Cell Selection” title in the page head, preserving the rest of the document unchanged.examples/vue/cell-selection/src/index.css-135-135 (1)
135-135: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse the configured keyword casing.
Stylelint rejects
currentColoron Lines 135, 216, and 321. Replace it withcurrentcolor.Also applies to: 216-216, 321-321
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/vue/cell-selection/src/index.css` at line 135, Update the border declarations at the affected locations in index.css to use the configured lowercase CSS keyword currentcolor instead of currentColor, including all three occurrences.Source: Linters/SAST tools
examples/vue/cell-selection/src/index.css-385-396 (1)
385-396: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd the required separator before
box-shadow.Stylelint requires an empty line before the
box-shadowdeclaration on Line 392.Proposed fix
--cell-edge-bottom: 0 0 0 0 transparent; --cell-edge-left: 0 0 0 0 transparent; + box-shadow:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/vue/cell-selection/src/index.css` around lines 385 - 396, Insert an empty line between the final --cell-edge-left custom property and the box-shadow declaration in the .cell-selectable rule to satisfy the required CSS style separation.Source: Linters/SAST tools
docs/framework/preact/guide/cell-selection.md-338-343 (1)
338-343: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDefine
rowSelectionKeyin the Preact guide.This example calls
rowSelectionKey(...)inside theSubscribeselector, butrowSelectionKeyis only defined in the React guide. Add the equivalent implementation here or inline the selector so Preact readers can copy/type-check the example as written.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/framework/preact/guide/cell-selection.md` around lines 338 - 343, Define the missing rowSelectionKey helper in the Preact guide near the Subscribe selector, matching the equivalent implementation from the React guide and its arguments (ranges, cell-selection bounds, display index, and row ID). Keep the existing selector call unchanged so the example remains copyable and type-checkable.docs/framework/lit/guide/cell-selection.md-341-346 (1)
341-346: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAlign with the documented
subscribedirective API.The current snippet calls
subscribe(...)without importing it, and references indocs/framework/lit/reference/variables/subscribe.mdandexamples/lit/basic-subscribe/src/main.tstreat it as a Lit expression/directive (e.g.,${subscribe(...)}), so the standalone expression does not match the Lit API being documented.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/framework/lit/guide/cell-selection.md` around lines 341 - 346, Update the cell-selection example’s subscribe usage to match the documented Lit directive API: import or reference the established subscribe symbol as appropriate, and place its invocation inside the template interpolation rather than as a standalone call. Preserve the existing atom, range-count selector, and rendered “ranges” output.examples/vue/cell-selection/vite.config.ts-5-9 (1)
5-9: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winConfigure the hosted dev-server host allow-list.
examples/vue/cell-selection/vite.config.tsleavesserver.allowedHostsunset; add the trusted CodeSandbox hostname explicitly instead of enablingserver.allowedHosts: true, which allows anyHostheader.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/vue/cell-selection/vite.config.ts` around lines 5 - 9, Add the trusted CodeSandbox hostname to the Vite server configuration’s server.allowedHosts in defineConfig, keeping the allow-list explicit and avoiding the unrestricted true setting.Source: Learnings
docs/framework/angular/guide/cell-selection.md-323-330 (1)
323-330: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAvoid clearing the initial selection before tracking changes.
The shown examples run on mount: Angular/Preact effects run initially, and Ember/Lit compare the first
layoutKeyagainst the unset previous keys. Each run callsresetCellSelection(true), losing an initialcellSelectionbefore any actual layout change. Track and store the first layout key without resetting, then reset only when a later key differs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/framework/angular/guide/cell-selection.md` around lines 323 - 330, Update the cell-selection layout tracking examples so the first effect run only records the initial layout key and preserves the existing cellSelection; call resetCellSelection(true) only when a subsequent layout key differs. Apply this behavior in docs/framework/angular/guide/cell-selection.md (323-330), docs/framework/ember/guide/cell-selection.md (301-313), docs/framework/lit/guide/cell-selection.md (301-313), and docs/framework/preact/guide/cell-selection.md (298-305), adapting each example’s existing layout-key state or comparison symbols.
🧹 Nitpick comments (7)
packages/table-core/src/features/cell-selection/cellSelectionFeature.utils.ts (1)
338-358: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTwo doc blocks stacked on
resolveCellPosition;cell_getIsSelectedends up undocumented.The "Checks whether this cell falls inside any selected range / Deliberately not memoized" block (Lines 338-349) belongs to
cell_getIsSelectedat Line 396, but sits aboveresolveCellPosition. Move it down to the function it describes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/table-core/src/features/cell-selection/cellSelectionFeature.utils.ts` around lines 338 - 358, Move the “Checks whether this cell falls inside any selected range” documentation block from above resolveCellPosition to immediately above cell_getIsSelected, leaving the resolveCellPosition documentation directly above its function and preserving both descriptions unchanged.examples/ember/cell-selection/app/templates/application.gts (2)
161-162: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
notlooks unused.The template uses
{{#unless}}/eq, nevernot, so the exported helper (and its comment) is dead code in this example.🧹 Proposed cleanup
-const not = (value: unknown): boolean => !value const eq = (a: unknown, b: unknown): boolean => String(a) === String(b)-// `not` is exported so the template compiler keeps it referenced -export { not }Also applies to: 552-553
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/ember/cell-selection/app/templates/application.gts` around lines 161 - 162, Remove the unused not helper and its associated comment from the application template; retain the eq helper and all template usages unchanged.
209-252: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLayout-reset side effect hides in a getter.
resetOnLayoutChange()only runs because the template rendersselectedCellCount. Removing or reordering that summary silently disables the reset. Consider driving it from a real reactive hook/modifier (e.g. an element modifier on the grid or aresource) so the behavior does not depend on which getters happen to be consumed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/ember/cell-selection/app/templates/application.gts` around lines 209 - 252, Move the layout-change reset trigger out of the selectedCellCount getter and into an explicit reactive lifecycle mechanism, such as a grid element modifier or resource. Ensure resetOnLayoutChange runs whenever the tracked column order, pinning, visibility, or sorting state changes, regardless of which summary getters the template renders.examples/svelte/cell-selection/src/App.svelte (1)
320-330: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueEmpty
onkeydown+role="presentation"silences the lint without making sorting keyboard-reachable.If you want the example to be exemplary, a
<button>inside the<th>(orrole="button"with a real keydown handler) gets sorting for keyboard users at no extra cost.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/svelte/cell-selection/src/App.svelte` around lines 320 - 330, Update the sortable header element around FlexRender to use a native button inside the table header, preserving the existing click sorting handler and sort indicators while making sorting keyboard-accessible. Remove the empty onkeydown handler and role="presentation" workaround, and retain the sortable-header styling as needed.examples/lit/cell-selection/src/main.ts (1)
246-273: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove layout-change side effects out of
render().
resetOnLayoutChangewritesthis._lastLayoutKeyand schedulestable.resetCellSelection(true)duringrender(), which should remain pure in Lit. Move the comparison/write to an update lifecycle method and keep the microtask only for the state update.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/lit/cell-selection/src/main.ts` around lines 246 - 273, Move the resetOnLayoutChange(table) invocation out of render() and perform its layout-key comparison and this._lastLayoutKey write in an appropriate Lit update lifecycle method. Keep table.resetCellSelection(true) deferred to a microtask, with the lifecycle method handling only the state comparison/write and scheduling.examples/react/cell-selection/tests/e2e/smoke.spec.ts (1)
47-86: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise cell selection in the smoke suite.
These specs never initiate a selection. Add at least a pointer-drag test asserting selected cells and selection-edge classes; otherwise broken selection handlers can ship while both tests pass.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/react/cell-selection/tests/e2e/smoke.spec.ts` around lines 47 - 86, The smoke suite only verifies table rendering and regeneration, so it does not cover cell selection behavior. Add a pointer-drag test in the existing smoke specs that selects a range of cells, then assert the selected cells and their selection-edge classes using the table locators; preserve the existing server cleanup and error assertions.examples/solid/cell-selection/tests/e2e/smoke.spec.ts (1)
72-100: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd an interaction assertion for cell selection.
This smoke test only checks rendering and data regeneration; it never clicks, drags, or uses keyboard selection. Add one focused range-selection assertion, or add a separate framework integration test so broken selection wiring cannot pass unnoticed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/solid/cell-selection/tests/e2e/smoke.spec.ts` around lines 72 - 100, Add a focused cell-selection interaction assertion to the “renders the table without crashing” test, using the existing table and body-row helpers to select a range via click, drag, or keyboard and verify the expected selected-cell state. Keep the rendering and regeneration checks unchanged, and ensure the assertion exercises the actual selection wiring rather than only DOM visibility.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/framework/react/guide/cell-selection.md`:
- Around line 269-284: Update escapeTsvValue in
docs/framework/react/guide/cell-selection.md (lines 269-284) and
docs/framework/solid/guide/cell-selection.md (lines 276-291) to neutralize
formula-like values before TSV escaping by prefixing an apostrophe when the
value, including leading spaces or tabs, begins with =, +, -, or @. Preserve the
existing null conversion, quote doubling, and delimiter escaping behavior in
both sites.
In `@examples/angular/cell-selection/src/app/app.html`:
- Around line 81-103: Update the header markup around the sortable container so
sorting uses a dedicated keyboard-operable button, while the pin-actions
controls remain sibling buttons rather than nested within the sorting control.
Preserve the existing sort label, indicator, and pin behavior, and ensure
pin-button clicks cannot trigger the sorting handler.
- Line 73: Update the grid container identified by the `#grid` template reference
in app.html so its tabindex allows keyboard users to reach it through normal Tab
navigation. Preserve the existing keyboard selection behavior while replacing
the current programmatic-focus-only setting.
In `@examples/lit/cell-selection/vite.config.js`:
- Around line 10-15: Update the Rollup Replace configuration in vite.config.js
to derive __DEV__ and process.env.NODE_ENV from Vite’s current build command or
mode instead of hard-coding development values. Ensure vite build produces
production replacements while development serving retains development behavior.
In `@examples/react/cell-selection/index.html`:
- Line 7: Remove the unversioned react-scan script from the cell-selection
example’s HTML so shipped instrumentation is not fetched and executed at
runtime; do not replace it with another mutable external script, and only
reintroduce instrumentation through a pinned, reviewed build dependency if
required.
In `@examples/solid/cell-selection/tests/e2e/smoke.spec.ts`:
- Around line 24-39: Update openExample so failures during page.route or
page.goto close the server before propagating the error. Wrap the route and
navigation setup after startExampleServer in error handling that invokes the
server’s shutdown method, while preserving the successful return of errors and
server.
In `@examples/svelte/cell-selection/tests/e2e/smoke.spec.ts`:
- Line 6: Update the exampleDir initialization near the test setup to resolve
the examples/svelte/cell-selection directory explicitly rather than relying on
path.resolve()’s current working directory. Derive it from the test file
location or use the correct relative path so startExampleServer receives the
intended example root regardless of where the tests are launched.
In `@examples/svelte/cell-selection/vite.config.js`:
- Around line 11-18: Update the Vite configuration’s rollupReplace setup to
derive __DEV__ and process.env.NODE_ENV from the command, setting them to
development values only for serve mode and production values for build mode.
In
`@packages/table-core/src/features/cell-selection/cellSelectionFeature.utils.ts`:
- Around line 917-919: Guard the column lookup in the selectable-column counting
loop before accessing its properties. Update the logic around columnDef so
missing columns are skipped, matching the existing guards in the other loops,
while preserving the enableCellSelection check for present columns.
---
Minor comments:
In `@docs/framework/alpine/guide/cell-selection.md`:
- Line 319: Update the cell-selection guidance around the layout-change example
to remove the React-specific useEffect reference. Describe the solution using
framework-neutral wording, such as a userland layout-change handler or Alpine
reactive effect/subscription.
In `@docs/framework/angular/guide/cell-selection.md`:
- Around line 323-330: Update the cell-selection layout tracking examples so the
first effect run only records the initial layout key and preserves the existing
cellSelection; call resetCellSelection(true) only when a subsequent layout key
differs. Apply this behavior in docs/framework/angular/guide/cell-selection.md
(323-330), docs/framework/ember/guide/cell-selection.md (301-313),
docs/framework/lit/guide/cell-selection.md (301-313), and
docs/framework/preact/guide/cell-selection.md (298-305), adapting each example’s
existing layout-key state or comparison symbols.
In `@docs/framework/lit/guide/cell-selection.md`:
- Around line 341-346: Update the cell-selection example’s subscribe usage to
match the documented Lit directive API: import or reference the established
subscribe symbol as appropriate, and place its invocation inside the template
interpolation rather than as a standalone call. Preserve the existing atom,
range-count selector, and rendered “ranges” output.
In `@docs/framework/preact/guide/cell-selection.md`:
- Around line 338-343: Define the missing rowSelectionKey helper in the Preact
guide near the Subscribe selector, matching the equivalent implementation from
the React guide and its arguments (ranges, cell-selection bounds, display index,
and row ID). Keep the existing selector call unchanged so the example remains
copyable and type-checkable.
In `@docs/framework/svelte/guide/cell-selection.md`:
- Line 304: Update the layout-reset wording in
docs/framework/svelte/guide/cell-selection.md at lines 304-304 to refer to
Svelte’s $effect instead of useEffect. Also update
docs/framework/vue/guide/cell-selection.md at lines 302-302 to refer to a Vue
watcher or watch, with no other changes required.
In `@examples/alpine/cell-selection/src/index.css`:
- Line 114: Update the affected style rules in index.css, including the
declarations around the currentColor border and the additional occurrences near
lines 160-164: use the Stylelint-compliant lowercase currentcolor value and
insert the required blank line before box-shadow declarations.
In `@examples/angular/cell-selection/src/styles.css`:
- Around line 123-137: Fix the Stylelint violations in the listed sites: in
examples/angular/cell-selection/src/styles.css lines 123-137, 215-219, and
318-324, and examples/ember/cell-selection/app/app.css lines 213-223, change
currentColor to the configured currentcolor casing; in
examples/angular/cell-selection/src/styles.css lines 385-396 and
examples/ember/cell-selection/app/app.css lines 321-332, insert the required
blank line immediately before box-shadow.
In `@examples/preact/cell-selection/index.html`:
- Line 7: Update the document title in the cell-selection example page from the
generic Vite + Preact label to a title that identifies the TanStack Table
cell-selection example.
In `@examples/preact/cell-selection/src/index.css`:
- Line 142: Resolve the Stylelint violations in
examples/preact/cell-selection/src/index.css: replace currentColor with
currentcolor at lines 142, 223, and 328, and insert the required empty line
before box-shadow at lines 399-403.
In `@examples/preact/cell-selection/tests/e2e/smoke.spec.ts`:
- Line 6: Update the exampleDir initialization in smoke.spec.ts to derive the
example directory from the test module’s import.meta.url, resolving two levels
above tests/e2e. Remove the path.resolve() dependence on process.cwd() so
startExampleServer uses this example’s Vite configuration.
In `@examples/react/cell-selection/src/index.css`:
- Line 135: Update the border declarations in
examples/react/cell-selection/src/index.css at lines 135-135, 216-216, and
321-321, replacing currentColor with the required lowercase currentcolor keyword
to satisfy Stylelint.
- Around line 392-396: Add the required empty line immediately before the
box-shadow declaration in the cell-selection stylesheet, preserving the existing
shadow values and formatting.
In `@examples/react/cell-selection/src/main.tsx`:
- Around line 523-533: Update rowSelectionKey in
examples/react/cell-selection/src/main.tsx:523-533 to build the selection key
from active.focusRowId and active.focusColumnId rather than the anchor
identifiers. Apply the identical change in
examples/preact/cell-selection/src/main.tsx:523-533 so both examples invalidate
memoization when focus moves.
In
`@examples/react/mantine-react-table/src/mantine-react-table/hooks/useMRT_TableOptions.ts`:
- Around line 128-129: Update the aggregation-functions memo in
useMRT_TableOptions so it recomputes when the caller-provided aggregationFns
changes by adding aggregationFns to the dependency list, while preserving the
merge with MRT_RowAggregationFns.
In `@examples/solid/cell-selection/index.html`:
- Line 7: Update the title element in the Solid selection example so it
identifies the page as “Cell Selection” rather than “Solid Row Selection,”
ensuring the browser tab and bookmarks use the correct label.
In `@examples/solid/cell-selection/src/index.css`:
- Line 135: Update the three reported border declarations to use the
Stylelint-compliant lowercase currentcolor value, and add the required blank
line before box-shadow in the declaration block near the final reported
location. Preserve all other styling unchanged.
In `@examples/svelte/cell-selection/index.html`:
- Line 6: Update the title element in the cell selection example from “Svelte
Row Selection Example” to accurately identify it as the Svelte Cell Selection
Example.
In `@examples/svelte/cell-selection/src/index.css`:
- Line 135: Update the CSS declarations at the referenced border lines to use
the lowercase currentcolor keyword, and add the required blank line immediately
before the box-shadow declaration in the relevant style block. Preserve all
other styles unchanged.
In `@examples/vue/cell-selection/index.html`:
- Line 7: Update the HTML document title from the generic “Vite App” value to a
descriptive “Cell Selection” title in the page head, preserving the rest of the
document unchanged.
In `@examples/vue/cell-selection/src/index.css`:
- Line 135: Update the border declarations at the affected locations in
index.css to use the configured lowercase CSS keyword currentcolor instead of
currentColor, including all three occurrences.
- Around line 385-396: Insert an empty line between the final --cell-edge-left
custom property and the box-shadow declaration in the .cell-selectable rule to
satisfy the required CSS style separation.
In `@examples/vue/cell-selection/vite.config.ts`:
- Around line 5-9: Add the trusted CodeSandbox hostname to the Vite server
configuration’s server.allowedHosts in defineConfig, keeping the allow-list
explicit and avoiding the unrestricted true setting.
In `@packages/table-core/skills/cell-selection/SKILL.md`:
- Around line 136-138: Rewrite the performance guidance in the cell-selection
documentation to replace the sentence fragment with a complete, clear sentence
contrasting table-wide subscriptions with per-row subscriptions. Preserve the
existing recommendation to subscribe per row and the surrounding details about
cellSelection, getCellSelectionBounds(), and affected neighboring rows.
- Line 44: Update the cell-selection explanation to describe the range update as
a “two-corner write” or “four-field write,” replacing “two-field write.”
Preserve the existing explanation of the anchor and focus corners and the
CellSelectionRange state.
In
`@packages/table-core/src/features/cell-selection/cellSelectionFeature.utils.ts`:
- Around line 642-675: Update stepCoordinate so row navigation remains
functional when the current column is not selectable: resolve the row index
independently and handle the active column outside getSelectableColumns,
snapping to an appropriate selectable column when horizontal movement requires
one while preserving vertical movement in the current column where valid. Ensure
anchors on opted-out columns no longer make every arrow-key navigation return
null.
- Around line 1044-1059: Update the cell-drag initialization around
_isSelectingCells so the flag is set to true only when dragEnabled and
contextDocument are both available and the mouseup listener is attached;
otherwise leave it false. Preserve the mouseup cleanup that resets
table._isSelectingCells.
In
`@packages/table-core/tests/implementation/features/cell-selection/cellSelectionFeature.test.ts`:
- Around line 137-153: The test using makeTable does not feed updated cell
selection state back into the table, so it does not verify controlled-state
behavior. Update the onCellSelectionChange handler in “routes writes through
onCellSelectionChange when provided” to propagate the resolved state through
table.setOptions, or otherwise update table.atoms.cellSelection, and assert the
table’s state after the write while preserving the existing seen assertion.
---
Nitpick comments:
In `@examples/ember/cell-selection/app/templates/application.gts`:
- Around line 161-162: Remove the unused not helper and its associated comment
from the application template; retain the eq helper and all template usages
unchanged.
- Around line 209-252: Move the layout-change reset trigger out of the
selectedCellCount getter and into an explicit reactive lifecycle mechanism, such
as a grid element modifier or resource. Ensure resetOnLayoutChange runs whenever
the tracked column order, pinning, visibility, or sorting state changes,
regardless of which summary getters the template renders.
In `@examples/lit/cell-selection/src/main.ts`:
- Around line 246-273: Move the resetOnLayoutChange(table) invocation out of
render() and perform its layout-key comparison and this._lastLayoutKey write in
an appropriate Lit update lifecycle method. Keep table.resetCellSelection(true)
deferred to a microtask, with the lifecycle method handling only the state
comparison/write and scheduling.
In `@examples/react/cell-selection/tests/e2e/smoke.spec.ts`:
- Around line 47-86: The smoke suite only verifies table rendering and
regeneration, so it does not cover cell selection behavior. Add a pointer-drag
test in the existing smoke specs that selects a range of cells, then assert the
selected cells and their selection-edge classes using the table locators;
preserve the existing server cleanup and error assertions.
In `@examples/solid/cell-selection/tests/e2e/smoke.spec.ts`:
- Around line 72-100: Add a focused cell-selection interaction assertion to the
“renders the table without crashing” test, using the existing table and body-row
helpers to select a range via click, drag, or keyboard and verify the expected
selected-cell state. Keep the rendering and regeneration checks unchanged, and
ensure the assertion exercises the actual selection wiring rather than only DOM
visibility.
In `@examples/svelte/cell-selection/src/App.svelte`:
- Around line 320-330: Update the sortable header element around FlexRender to
use a native button inside the table header, preserving the existing click
sorting handler and sort indicators while making sorting keyboard-accessible.
Remove the empty onkeydown handler and role="presentation" workaround, and
retain the sortable-header styling as needed.
In
`@packages/table-core/src/features/cell-selection/cellSelectionFeature.utils.ts`:
- Around line 338-358: Move the “Checks whether this cell falls inside any
selected range” documentation block from above resolveCellPosition to
immediately above cell_getIsSelected, leaving the resolveCellPosition
documentation directly above its function and preserving both descriptions
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| {{ table.getCellSelectionColumnIds().length }} columns | ||
| </div> | ||
| <div class="spacer-sm"></div> | ||
| <div #grid tabindex="-1"> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make the keyboard target reachable.
tabindex="-1" leaves the grid unreachable by Tab while no cell is focused, so keyboard-only users cannot invoke the registered selection hotkeys without first using a mouse.
Proposed fix
- <div `#grid` tabindex="-1">
+ <div `#grid` [attr.tabindex]="table.getFocusedCell() ? -1 : 0">📝 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.
| <div #grid tabindex="-1"> | |
| <div `#grid` [attr.tabindex]="table.getFocusedCell() ? -1 : 0"> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/angular/cell-selection/src/app/app.html` at line 73, Update the grid
container identified by the `#grid` template reference in app.html so its tabindex
allows keyboard users to reach it through normal Tab navigation. Preserve the
existing keyboard selection behavior while replacing the current
programmatic-focus-only setting.
| rollupReplace({ | ||
| preventAssignment: true, | ||
| values: { | ||
| __DEV__: JSON.stringify(true), | ||
| 'process.env.NODE_ENV': JSON.stringify('development'), | ||
| }, |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate target and dependencies =="
fd -a 'vite.config.js|package.json|pnpm-lock.yaml|yarn.lock|package-lock.json' . | sed 's#^\./##' | head -200
echo
echo "== Target file =="
if [ -f examples/lit/cell-selection/vite.config.js ]; then
cat -n examples/lit/cell-selection/vite.config.js
fi
echo
echo "== Nearby package manifests =="
for f in examples/lit/cell-selection/package.json package.json; do
if [ -f "$f" ]; then
echo "--- $f ---"
cat -n "$f"
fi
done
echo
echo "== Search dev/build/source usages of flags =="
rg -n "__DEV__|process\.env\.NODE_ENV|rollupReplace|replace:" examples/lit/cell-selection -S || true
rg -n "__DEV__|process\.env\.NODE_ENV" -S . | sed -n '1,200p' || true
echo
echo "== Vite docs? local plugin docs? =="
fd -a 'README|vite.config.js|vite.*.js' examples | sed -n '1,100p'Repository: TanStack/table
Length of output: 12977
🌐 Web query:
Vite CLI ViteConfigFn command resolveConfig build replace process.env.NODE_ENV production flag
💡 Result:
In Vite, the handling of process.env.NODE_ENV is managed by the Vite CLI and the underlying resolveConfig internal utility [1][2]. Key points regarding your query: 1. CLI Command and NODE_ENV: By default, the vite build command sets process.env.NODE_ENV to production [3][4]. Setting it explicitly in the command (e.g., NODE_ENV=development vite build) overrides this default, allowing you to influence the process environment early—which is necessary if you need to access this value while your vite.config.ts is being evaluated [3][4]. 2. ViteConfigFn (Conditional Config): If you export a function from your vite.config.ts (a "ViteConfigFn"), you can receive the command and mode parameters [5]. However, because Vite deliberately defers loading.env files until after the config is resolved, any variables defined in those.env files are not automatically present in process.env during the config evaluation [6][5]. To use environment variables to influence your configuration (e.g., setting define replacements), you should use the loadEnv helper function within your exported function [6][5]. 3. Statically Replacing process.env.NODE_ENV: Vite does not automatically replace process.env.NODE_ENV in your application code with the string "production" during builds [7][8]. This is an intentional design choice, particularly to support library mode and other use cases where consumers might want to influence behavior dynamically [7][8]. If you want to force this replacement for optimization or to strip out development-only code, you must configure it explicitly in your vite.config.ts using the define option [7][8][9]: // vite.config.ts export default defineConfig({ define: { 'process.env.NODE_ENV': '"production"' } }) If you are building an application and rely on this for performance or code removal, ensure the value is double-quoted (e.g., '"production"') because Vite performs a direct text replacement [7][9].
Citations:
- 1: https://github.com/vitejs/vite/blob/2c4a0920ec7730e61508745fb8e4d49fc2945f00/packages/vite/src/node/config.ts
- 2: calling
createServersetsprocess.env.NODE_ENVvitejs/vite#18712 - 3: https://vite.dev/guide/env-and-mode
- 4: https://github.com/vitejs/vite/blob/130ef31b/docs/guide/env-and-mode.md
- 5: https://vite.dev/config/
- 6: https://github.com/vitejs/vite/blob/main/docs/config/index.md
- 7: https://vite.dev/guide/build
- 8: https://github.com/vitejs/vite/blob/main/docs/guide/build.md
- 9:
process.env.NODE_ENVshould be replace to "production" when library mode. vitejs/vite#11730
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Target config =="
cat -n examples/lit/cell-selection/vite.config.js
echo
echo "== package manifests for this example =="
for f in examples/lit/cell-selection/package.json package.json examples/lit/cell-selection/pnpm-lock.yaml examples/lit/pnpm-workspace.yaml pnpm-workspace.yaml; do
[ -f "$f" ] && { echo "--- $f ---"; sed -n '1,220p' "$f"; }
done
echo
echo "== Vite and rollupReplace dependency declarations =="
rg -n '"`@rollup/plugin-replace`"|"replace"|rollupReplace|vite|process\.env\.NODE_ENV|__DEV__' examples/lit/cell-selection examples/lit package.json pnpm-lock.yaml -S | sed -n '1,240p'
echo
echo "== Similar Vite configs in examples/lit =="
for f in examples/lit/kitchen-sink/vite.config.js examples/lit/virtualized-rows/vite.config.js examples/lit/filters/vite.config.js; do
[ -f "$f" ] && { echo "--- $f ---"; sed -n '1,120p' "$f"; }
doneRepository: TanStack/table
Length of output: 30452
Do not ship development-only replacements to production builds.
vite build reuses this Vite config, so the Rollup Replace plugin currently forces __DEV__ = true and NODE_ENV = "development" into production artifacts. Derive those values from Vite’s build command/mode instead.
Proposed fix
-export default defineConfig({
+export default defineConfig(({ command }) => {
+ const isDev = command === 'serve'
+
+ return {
server: {
port: 6565,
},
plugins: [
rollupReplace({
preventAssignment: true,
values: {
- __DEV__: JSON.stringify(true),
- 'process.env.NODE_ENV': JSON.stringify('development'),
+ __DEV__: JSON.stringify(isDev),
+ 'process.env.NODE_ENV': JSON.stringify(
+ isDev ? 'development' : 'production',
+ ),
},
}),
],
-})
+ }
+})📝 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.
| rollupReplace({ | |
| preventAssignment: true, | |
| values: { | |
| __DEV__: JSON.stringify(true), | |
| 'process.env.NODE_ENV': JSON.stringify('development'), | |
| }, | |
| export default defineConfig(({ command }) => { | |
| const isDev = command === 'serve' | |
| return { | |
| server: { | |
| port: 6565, | |
| }, | |
| plugins: [ | |
| rollupReplace({ | |
| preventAssignment: true, | |
| values: { | |
| __DEV__: JSON.stringify(isDev), | |
| 'process.env.NODE_ENV': JSON.stringify( | |
| isDev ? 'development' : 'production', | |
| ), | |
| }, | |
| }), | |
| ], | |
| } | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/lit/cell-selection/vite.config.js` around lines 10 - 15, Update the
Rollup Replace configuration in vite.config.js to derive __DEV__ and
process.env.NODE_ENV from Vite’s current build command or mode instead of
hard-coding development values. Ensure vite build produces production
replacements while development serving retains development behavior.
| <meta charset="UTF-8" /> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0" /> | ||
| <title>Vite App</title> | ||
| <script src="https://unpkg.com/react-scan/dist/auto.global.js"></script> |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Remove the mutable third-party script.
This unversioned URL executes whatever react-scan publishes in every visitor’s browser, outside this repository’s review and release controls. Keep instrumentation out of the shipped HTML, or bundle a pinned, reviewed dependency through the example build.
Proposed fix
- <script src="https://unpkg.com/react-scan/dist/auto.global.js"></script>📝 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.
| <script src="https://unpkg.com/react-scan/dist/auto.global.js"></script> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/react/cell-selection/index.html` at line 7, Remove the unversioned
react-scan script from the cell-selection example’s HTML so shipped
instrumentation is not fetched and executed at runtime; do not replace it with
another mutable external script, and only reintroduce instrumentation through a
pinned, reviewed build dependency if required.
| import path from 'node:path' | ||
| import { startExampleServer } from '../../../../../tests/e2e/helpers/startExampleServer' | ||
|
|
||
| const exampleDir = path.resolve() |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files matching smoke.spec.ts:"
fd -a 'smoke\.spec\.ts$' . | sed 's#^\./##' | sort
echo
echo "Relevant test file:"
cat -n examples/svelte/cell-selection/tests/e2e/smoke.spec.ts
echo
echo "Find startExampleServer definitions/usages:"
rg -n "function startExampleServer|const startExampleServer|startExampleServer|exampleDir|path\.resolve\(" -S .Repository: TanStack/table
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "startExampleServer helper:"
cat -n tests/e2e/helpers/startExampleServer.ts
echo
echo "Comparable adjacent test using relative path:"
cat -n examples/svelte/cell-selection/tests/e2e/smoke.spec.ts | head -12
echo
echo "Other smoke tests nearby that do NOT use path.resolve() and their exampleDir:"
for f in examples/svelte/*/tests/e2e/smoke.spec.ts; do
echo "--- $f"
sed -n '1,12p' "$f"
done | sortRepository: TanStack/table
Length of output: 24584
Resolve this test’s example directory explicitly.
path.resolve() resolves to the runner’s current directory, while startExampleServer uses exampleDir as the server root, Vite root, Angular/Ember cwd, etc. When tests are launched from the repo root, this serves/forks from ./ instead of examples/svelte/cell-selection, so derive the path from the test file location or use an explicit relative path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/svelte/cell-selection/tests/e2e/smoke.spec.ts` at line 6, Update the
exampleDir initialization near the test setup to resolve the
examples/svelte/cell-selection directory explicitly rather than relying on
path.resolve()’s current working directory. Derive it from the test file
location or use the correct relative path so startExampleServer receives the
intended example root regardless of where the tests are launched.
| rollupReplace({ | ||
| preventAssignment: true, | ||
| values: { | ||
| __DEV__: JSON.stringify(true), | ||
| 'process.env.NODE_ENV': JSON.stringify('development'), | ||
| }, | ||
| }), | ||
| svelte(), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
git ls-files | rg '(^|/)vite\.config\.js$|(^|/)package\.json$|rollupReplace|__DEV__|process\.env\.NODE_ENV' || true
echo
echo "== example config =="
if [ -f examples/svelte/cell-selection/vite.config.js ]; then
cat -n examples/svelte/cell-selection/vite.config.js
fi
echo
echo "== related occurrences =="
rg -n "__DEV__|process\.env\.NODE_ENV|build|serve" examples/svelte/cell-selection -S || true
echo
echo "== package deps =="
if [ -f examples/svelte/cell-selection/package.json ]; then
cat examples/svelte/cell-selection/package.json
fiRepository: TanStack/table
Length of output: 26329
Reserve __DEV__ replacement for the dev server.
This config runs during vite build, so any code gated by __DEV__ still sees a truthy dev flag after the build. Derive these replacements from command (serve vs build) or only set them for serve mode.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/svelte/cell-selection/vite.config.js` around lines 11 - 18, Update
the Vite configuration’s rollupReplace setup to derive __DEV__ and
process.env.NODE_ENV from the command, setting them to development values only
for serve mode and production values for build mode.
There was a problem hiding this comment.
Important
At least one additional CI pipeline execution has run since the conclusion below was written and it may no longer be applicable.
Nx Cloud is proposing a fix for your failed CI:
We removed the implicit global document fallback in cell_getSelectionStartHandler so that _isSelectingCells is only set when a document is explicitly provided. Without this fix, jsdom's globally available document was picked up automatically, causing the drag flag to be set true even when no document argument was passed. This aligns the implementation with the PR's own test contract: "does not open a drag without a document to close it."
Tip
✅ We verified this fix by re-running @tanstack/table-core:test:lib.
diff --git a/packages/table-core/src/features/cell-selection/cellSelectionFeature.utils.ts b/packages/table-core/src/features/cell-selection/cellSelectionFeature.utils.ts
index 41b2a0c8..c0039022 100644
--- a/packages/table-core/src/features/cell-selection/cellSelectionFeature.utils.ts
+++ b/packages/table-core/src/features/cell-selection/cellSelectionFeature.utils.ts
@@ -1077,8 +1077,7 @@ export function cell_getSelectionStartHandler<
const table = cell.table
const options = table.options
- const contextDocument =
- _contextDocument ?? (typeof document !== 'undefined' ? document : null)
+ const contextDocument = _contextDocument ?? null
const isRangeEvent =
options.enableCellRangeSelection !== false &&
Or Apply changes locally with:
npx nx-cloud apply-locally 0UYA-XoyX
Apply fix locally with your editor ↗ View interactive diff ↗
🎓 Learn more about Self-Healing CI on nx.dev
🎯 Changes
✅ Checklist
pnpm test:pr.Summary by CodeRabbit