fix(data-table): guard undefined defaultSort in hasActiveQuery#758
fix(data-table): guard undefined defaultSort in hasActiveQuery#758rohanchkrabrty merged 2 commits intoraystack:mainfrom
Conversation
`hasActiveQuery` crashed with "Cannot destructure property 'name' of 'undefined'" when consumers passed an undefined `defaultSort`. The TypeScript type marks it required, but runtime callers can still pass undefined, producing `[undefined]` and crashing `generateSortMap`. - Filter falsy entries inside `generateSortMap` before destructure. - Pass `[]` instead of `[undefined]` from `hasActiveQuery` when `defaultSort` is missing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@singh-pk is attempting to deploy a commit to the Raystack Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe changes enhance sort-change detection logic in the data-table utility by filtering out falsy elements when constructing the sort map in Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/raystack/components/data-table/utils/index.tsx (2)
167-185: Reflect runtime reality in the type signature.Since the function now intentionally handles a falsy
defaultSortat runtime (lines 176–179), the parameter should be typed as optional so TypeScript can enforce the same contract at call sites and prevent regressions. Otherwise, the code and the types disagree, and a future caller passingdefaultSort={undefined}will still type-check while relying on this implicit guard.🔧 Proposed signature update
export const hasActiveQuery = ( tableQuery: InternalQuery, - defaultSort: DataTableSort + defaultSort?: DataTableSort ): boolean => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/data-table/utils/index.tsx` around lines 167 - 185, The hasActiveQuery function accepts a parameter defaultSort but treats it as possibly falsy at runtime; update the function signature to make defaultSort optional (change parameter to defaultSort?: DataTableSort) so TypeScript reflects this runtime behavior, then fix any call sites that relied on it being required (or pass an explicit value) and ensure internal uses (the isSortChanged call using defaultSort ? [defaultSort] : []) remain correct; target the hasActiveQuery declaration and any callers that will now type-check differently.
344-356: Consider guarding undefineddefaultSortingetDefaultTableQueryfor consistency with thehasActiveQueryfix.
getDefaultTableQueryunconditionally wrapsdefaultSortin an array at line 352. Although TypeScript marksdefaultSortas required inDataTableProps, the recent fix tohasActiveQueryshows that runtime callers can still passundefineddespite the type signature. This could producesort: [undefined], which may silently filter ingenerateSortMapbut could surface as[undefined]to downstream consumers ofInternalQuery.sort.Applying the same guard—
sort: defaultSort ? [defaultSort] : []—and making the parameter optional would maintain consistency with the pattern established in thehasActiveQueryfix. Optional for this PR if scope is preferred tight.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/data-table/utils/index.tsx` around lines 344 - 356, getDefaultTableQuery currently always wraps defaultSort into sort: [defaultSort], which can produce [undefined] at runtime; change the signature to make defaultSort optional (DataTableSort | undefined) and guard the assignment so sort is defaultSort ? [defaultSort] : [], mirroring the hasActiveQuery fix; update any usages if needed and ensure InternalQuery.sort remains an empty array when defaultSort is not provided (this keeps behavior consistent with generateSortMap and avoids leaking undefined downstream while leaving group_by: [defaultGroupOption.id] unchanged).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/raystack/components/data-table/utils/index.tsx`:
- Around line 167-185: The hasActiveQuery function accepts a parameter
defaultSort but treats it as possibly falsy at runtime; update the function
signature to make defaultSort optional (change parameter to defaultSort?:
DataTableSort) so TypeScript reflects this runtime behavior, then fix any call
sites that relied on it being required (or pass an explicit value) and ensure
internal uses (the isSortChanged call using defaultSort ? [defaultSort] : [])
remain correct; target the hasActiveQuery declaration and any callers that will
now type-check differently.
- Around line 344-356: getDefaultTableQuery currently always wraps defaultSort
into sort: [defaultSort], which can produce [undefined] at runtime; change the
signature to make defaultSort optional (DataTableSort | undefined) and guard the
assignment so sort is defaultSort ? [defaultSort] : [], mirroring the
hasActiveQuery fix; update any usages if needed and ensure InternalQuery.sort
remains an empty array when defaultSort is not provided (this keeps behavior
consistent with generateSortMap and avoids leaking undefined downstream while
leaving group_by: [defaultGroupOption.id] unchanged).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fab658a6-e3ac-4304-b4c0-c63b1036d600
📒 Files selected for processing (1)
packages/raystack/components/data-table/utils/index.tsx
Description
hasActiveQuerycrashed withTypeError: Cannot destructure property 'name' of 'undefined' as it is undefined.when consumers passedundefinedasdefaultSort. The TypeScript type marksdefaultSortas required, but at runtime callers can still passundefined, which produced[undefined]and crashedgenerateSortMapduring the destructure.Stack trace observed:
Two-layer defensive fix in
packages/raystack/components/data-table/utils/index.tsx:generateSortMap— filter falsy entries before destructure.hasActiveQuery— pass[]instead of[undefined]whendefaultSortis missing.Type of Change
How Has This Been Tested?
DataTableconsumer passingdefaultSort={undefined}.hasActiveQueryreturnsfalsefor the default state.Checklist:
Screenshots (if appropriate):
N/A
Related Issues
Bug origin: #666 (
fix: DataTable reset and zero state handling) — introducedhasActiveQueryandgenerateSortMapwithout guarding for an undefineddefaultSort.