fix(mantine-react-table): expose loading state slices on table.state - #6548
fix(mantine-react-table): expose loading state slices on table.state#6548naughton wants to merge 1 commit into
Conversation
The post-useTable patch that re-injects MRT-only state slices onto table.state omits the controlled-only loading slices (isLoading, isSaving, showLoadingOverlay, showProgressBars, showSkeletons). Those slices have no internal useState — they only arrive via options.state — so every component that destructures them from table.state reads undefined. Visible symptom: with state.isLoading true, the blank skeleton rows are generated, but MRT_TableBodyCell's skeleton branch never triggers, so column Cell renderers run against the null placeholder data (epoch dates, fallback strings) instead of rendering <Skeleton>s. The loading overlay and progress bars are similarly dead. showSkeletons is intentionally not defaulted to false: MRT_TableBodyCell checks `showSkeletons !== false` — false means 'suppress skeletons even while loading', undefined means 'auto'.
📝 WalkthroughWalkthroughThe table instance now reinjects controlled loading fields into ChangesControlled loading state
Estimated code review effort: 2 (Simple) | ~10 minutes Mergeability Score: 🟡 Moderate · up to The PR enables loading-state behavior, but its public TypeScript contract currently says showSkeletons is always boolean even though it can be undefined when uncontrolled. This can mislead consumers and should be corrected before merging. Suggested reviewers: 🚥 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@examples/react/mantine-react-table/src/mantine-react-table/hooks/useMRT_TableInstance.ts`:
- Around line 402-404: Update MRT_TableState.showSkeletons in types.ts to
boolean | undefined, then remove the non-null assertion from
statefulTableOptions.state.showSkeletons in useMRT_TableInstance. Preserve the
un-defaulted value so undefined remains distinct from explicit false.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5829b6ad-c373-47ab-ba1c-8253b6c86303
📒 Files selected for processing (1)
examples/react/mantine-react-table/src/mantine-react-table/hooks/useMRT_TableInstance.ts
| // not defaulted: `false` means "suppress skeletons even while loading" | ||
| // (MRT_TableBodyCell checks `showSkeletons !== false`), undefined means "auto" | ||
| showSkeletons: statefulTableOptions.state.showSkeletons!, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Align the exported showSkeletons type with the intentional undefined value.
examples/react/mantine-react-table/src/mantine-react-table/types.ts:424-446 declares MRT_TableState.showSkeletons as boolean, but Line 404 returns undefined when the option is not controlled. The non-null assertion only suppresses the TypeScript check; it does not change the runtime value. Update the type to boolean | undefined and remove !. Keep the value un-defaulted because MRT_TableBodyCell.tsx:202-209 distinguishes undefined from explicit false.
Suggested type alignment
--- a/examples/react/mantine-react-table/src/mantine-react-table/types.ts
+++ b/examples/react/mantine-react-table/src/mantine-react-table/types.ts
@@
- showSkeletons: boolean
+ showSkeletons: boolean | undefined
--- a/examples/react/mantine-react-table/src/mantine-react-table/hooks/useMRT_TableInstance.ts
+++ b/examples/react/mantine-react-table/src/mantine-react-table/hooks/useMRT_TableInstance.ts
@@
- showSkeletons: statefulTableOptions.state.showSkeletons!,
+ showSkeletons: statefulTableOptions.state.showSkeletons,📝 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.
| // not defaulted: `false` means "suppress skeletons even while loading" | |
| // (MRT_TableBodyCell checks `showSkeletons !== false`), undefined means "auto" | |
| showSkeletons: statefulTableOptions.state.showSkeletons!, | |
| // not defaulted: `false` means "suppress skeletons even while loading" | |
| // (MRT_TableBodyCell checks `showSkeletons !== false`), undefined means "auto" | |
| showSkeletons: statefulTableOptions.state.showSkeletons, |
🤖 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_TableInstance.ts`
around lines 402 - 404, Update MRT_TableState.showSkeletons in types.ts to
boolean | undefined, then remove the non-null assertion from
statefulTableOptions.state.showSkeletons in useMRT_TableInstance. Preserve the
un-defaulted value so undefined remains distinct from explicit false.
The post-useTable patch that re-injects MRT-only state slices onto table.state omits the controlled-only loading slices (isLoading, isSaving, showLoadingOverlay, showProgressBars, showSkeletons). Those slices have no internal useState — they only arrive via options.state — so every component that destructures them from table.state reads undefined.
Visible symptom: with state.isLoading true, the blank skeleton rows are generated, but MRT_TableBodyCell's skeleton branch never triggers, so column Cell renderers run against the null placeholder data (epoch dates, fallback strings) instead of rendering s. The loading overlay and progress bars are similarly dead.
showSkeletons is intentionally not defaulted to false: MRT_TableBodyCell checks
showSkeletons !== false— false means 'suppress skeletons even while loading', undefined means 'auto'.Summary by CodeRabbit