fix: bypass TanStack Table minification bug and migrate to scheduleOnRN#12
fix: bypass TanStack Table minification bug and migrate to scheduleOnRN#12
Conversation
## Minification Bug Fix
TanStack Table's `toggleExpanded()` uses computed property destructuring
with rest spread (`const { [key]: _, ...rest } = obj`) which is incorrectly
transpiled by some minifiers, causing the collapse functionality to fail
silently in production builds.
Fix: Bypass TanStack's updater function by using `lodash/omit` and a custom
`setRowExpanded` helper that directly manipulates the expanded state.
Affected files:
- products.tsx, pos/products/index.tsx - Added setRowExpanded helper
- attributes.tsx, variable-image.tsx, filters.tsx - Use setRowExpanded
- variable-product-row/context.tsx - Pass setRowExpanded through context
- dnd/native/context.tsx - Use inline delete (can't import in worklets)
## scheduleOnRN Migration
Migrated from `runOnJS` (react-native-reanimated) to `scheduleOnRN`
(react-native-worklets) as recommended by the deprecation warning.
Affected files:
- variable-product-row/index.tsx
- pulse-row.tsx
- splash/index.tsx
📝 WalkthroughWalkthroughAdds a table-driven row expansion API (setRowExpanded) to bypass a TanStack updater issue, migrates runOnJS scheduling to scheduleOnRN in RN worklets, and fixes a DnD context minification bug by replacing computed-destructuring with an explicit shallow-clone-and-delete for layout removal. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/screens/main/pos/products/index.tsx (1)
127-127: Missing import forExpandedStatetype.The
ExpandedStatetype is used at line 127 but is not imported. Add the import from@tanstack/react-table:Suggested fix
import { getExpandedRowModel } from '@tanstack/react-table'; +import type { ExpandedState } from '@tanstack/react-table';
🤖 Fix all issues with AI agents
In `@packages/core/src/screens/main/components/product/variable-image.tsx`:
- Around line 28-38: The code accesses table.options.meta.expanded$ without
guarding meta; change the usage so meta is checked consistently: read meta into
a local const (e.g., const meta = table.options.meta) and use optional chaining
or a fallback observable when calling useObservableEagerState (e.g.,
meta?.expanded$ ?? of({})), and keep setRowExpanded obtained from meta
(setRowExpanded) and handlePress unchanged but still using setRowExpanded?. This
ensures meta can be undefined without causing a runtime error while preserving
the existing setRowExpanded and isExpanded logic.
🧹 Nitpick comments (3)
packages/core/src/screens/splash/index.tsx (1)
30-34: Dependency on shared value's.valueproperty may not trigger re-runs as expected.The
useDerivedValuehook depends onprogress.valuein the dependency array, but shared value contents accessed via.valuedon't trigger React re-renders. The hook should depend onprogress(the shared value reference) instead, or simply rely on the reactive nature ofprogress.valueaccess inside the worklet.Suggested fix
useDerivedValue(() => { if (progress.value > 0) { scheduleOnRN(hideSplash); } - }, [progress.value, hideSplash]); + }, [progress, hideSplash]);packages/components/src/table/pulse-row.tsx (1)
17-22: Avoid usinganytype for generic parameters.Per coding guidelines, strict types should be used instead of
any. Consider using a generic type parameter forRowandTableto maintain type safety.Suggested fix
-type PulseTableRowProps = React.ComponentPropsWithoutRef<typeof Animated.View> & { +type PulseTableRowProps<TData = unknown> = React.ComponentPropsWithoutRef<typeof Animated.View> & { onRemove?: () => void; index?: number; - row: Row<any>; - table: Table<any>; + row: Row<TData>; + table: Table<TData>; };packages/core/src/screens/main/components/product/variable-product-row/context.tsx (1)
27-31: Replace newly introducedanytypes with stricter typing.This keeps the context and provider API safer and aligns with repo guidelines.
As per coding guidelines, avoid `any`.♻️ Suggested typing tightening
-interface VariationRowProviderProps { - row: any; - setRowExpanded?: (rowId: string, expanded: boolean) => void; - children: React.ReactNode; -} +interface VariationRowProviderProps { + row: { id: string }; + setRowExpanded?: (rowId: string, expanded: boolean) => void; + children: React.ReactNode; +} -const updateQueryParams = (key: string, value: any) => { +const updateQueryParams = (key: string, value: unknown) => {Also applies to: 36-39
Summary
runOnJS(reanimated) toscheduleOnRN(worklets)Problem
TanStack Table's
toggleExpanded()uses computed property destructuring with rest spread:This pattern is incorrectly transpiled by some minifiers in the Expo/Metro build pipeline, causing the temporary variable to be declared but never assigned. The result: collapse silently fails in production while working in development.
Solution
Bypass TanStack's updater - Added a
setRowExpanded(rowId, expanded)helper that useslodash/omitto safely remove keys without the problematic destructuring patternPass through context - Made
setRowExpandedavailable to child components viaVariationRowContextUpdated all toggle sites - Changed
row.toggleExpanded()calls to usesetRowExpanded()Fixed DnD context - Same pattern existed in drag-and-drop code; fixed with inline
deleteTest plan
Additional changes
Migrated
runOnJS→scheduleOnRNper deprecation warning:variable-product-row/index.tsxpulse-row.tsxsplash/index.tsxSummary by CodeRabbit
Bug Fixes
New Features
Performance
✏️ Tip: You can customize this high-level summary in your review settings.