Skip to content

fix(data-table): guard undefined defaultSort in hasActiveQuery#758

Merged
rohanchkrabrty merged 2 commits intoraystack:mainfrom
singh-pk:pk/fix-data-table-crashing
Apr 27, 2026
Merged

fix(data-table): guard undefined defaultSort in hasActiveQuery#758
rohanchkrabrty merged 2 commits intoraystack:mainfrom
singh-pk:pk/fix-data-table-crashing

Conversation

@singh-pk
Copy link
Copy Markdown
Contributor

Description

hasActiveQuery crashed with TypeError: Cannot destructure property 'name' of 'undefined' as it is undefined. when consumers passed undefined as defaultSort. The TypeScript type marks defaultSort as required, but at runtime callers can still pass undefined, which produced [undefined] and crashed generateSortMap during the destructure.

Stack trace observed:

TypeError: Cannot destructure property 'name' of 'undefined' as it is undefined.
    at index.tsx:119:30
    at Array.map (<anonymous>)
    at generateSortMap (index.tsx:119:23)
    at isSortChanged (index.tsx:142:22)
    at hasActiveQuery (index.tsx:175:23)
    at Content4 (content.tsx:217:5)

Two-layer defensive fix in packages/raystack/components/data-table/utils/index.tsx:

  • generateSortMap — filter falsy entries before destructure.
  • hasActiveQuery — pass [] instead of [undefined] when defaultSort is missing.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactor (no functional changes, no bug fixes just code improvements)
  • Chore (changes to the build process or auxiliary tools and libraries such as documentation generation)
  • Style (changes that do not affect the meaning of the code (white-space, formatting, etc))
  • Test (adding missing tests or correcting existing tests)
  • Improvement (Improvements to existing code)
  • Other (please specify)

How Has This Been Tested?

  • Reproduced the crash locally with a DataTable consumer passing defaultSort={undefined}.
  • After fix, the table renders without throwing and hasActiveQuery returns false for the default state.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (.mdx files)
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works

Screenshots (if appropriate):

N/A

Related Issues

Bug origin: #666 (fix: DataTable reset and zero state handling) — introduced hasActiveQuery and generateSortMap without guarding for an undefined defaultSort.

`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>
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 27, 2026

@singh-pk is attempting to deploy a commit to the Raystack Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 27, 2026

Warning

Rate limit exceeded

@singh-pk has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 46 minutes and 6 seconds before requesting another review.

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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3b3e1b20-b74f-4540-843a-9d0bd1e1e40a

📥 Commits

Reviewing files that changed from the base of the PR and between 53c8636 and 401ea15.

📒 Files selected for processing (2)
  • packages/raystack/components/data-table/data-table.types.tsx
  • packages/raystack/components/data-table/utils/index.tsx
📝 Walkthrough

Walkthrough

The changes enhance sort-change detection logic in the data-table utility by filtering out falsy elements when constructing the sort map in generateSortMap, preventing invalid entries. Additionally, hasActiveQuery is modified to conditionally provide defaultSort to isSortChanged, supplying an empty array when defaultSort is falsy rather than passing the falsy value itself. These modifications ensure accurate sort-state evaluation when default sort values are absent or undefined.

Suggested reviewers

  • paanSinghCoder
  • rohanchkrabrty
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main fix: guarding undefined defaultSort in hasActiveQuery, which directly matches the bug fix implemented in the changeset.
Description check ✅ Passed The description is directly related to the changeset, providing context about the bug (TypeError when defaultSort is undefined), the stack trace, and the two-layer defensive fix applied to the codebase.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@singh-pk singh-pk requested review from rohanchkrabrty and rsbh April 27, 2026 07:29
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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 defaultSort at 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 passing defaultSort={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 undefined defaultSort in getDefaultTableQuery for consistency with the hasActiveQuery fix.

getDefaultTableQuery unconditionally wraps defaultSort in an array at line 352. Although TypeScript marks defaultSort as required in DataTableProps, the recent fix to hasActiveQuery shows that runtime callers can still pass undefined despite the type signature. This could produce sort: [undefined], which may silently filter in generateSortMap but could surface as [undefined] to downstream consumers of InternalQuery.sort.

Applying the same guard—sort: defaultSort ? [defaultSort] : []—and making the parameter optional would maintain consistency with the pattern established in the hasActiveQuery fix. 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

📥 Commits

Reviewing files that changed from the base of the PR and between 08768b4 and 53c8636.

📒 Files selected for processing (1)
  • packages/raystack/components/data-table/utils/index.tsx

@singh-pk singh-pk added the bug Something isn't working label Apr 27, 2026
@rohanchkrabrty rohanchkrabrty merged commit 5905f2a into raystack:main Apr 27, 2026
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants