feat(ui): migrate packages/ui off @packrat-ai/nativewindui to @expo/ui - #2652
Open
mikib0 wants to merge 79 commits into
Open
feat(ui): migrate packages/ui off @packrat-ai/nativewindui to @expo/ui#2652mikib0 wants to merge 79 commits into
mikib0 wants to merge 79 commits into
Conversation
…ndui Wraps @expo/ui Universal Text/Button through Host, with a className parser that splits typography (font-weight/size/color/align) into native textStyle and leaves layout classes on Host. Two real device bugs found and fixed: matchContents fighting flex-1, and Host collapsing to zero height without an explicit size hint. Avatar/SegmentedControl wrap @rn-primitives/avatar and @expo/ui's community SegmentedControl directly. Converts the first batch of call sites (weather-alerts, settings, messages, packs).
Codemod-driven import swap across ~140 files, verified with a full typecheck + lint pass (0 errors). Widens Button's wrapper to accept the legacy variant names (primary/secondary/tonal/plain) and a size prop (sm/md/lg/icon/none mapped to fixed padding via style), since @expo/ui's Button only exposes filled/outlined/text with no size prop — this let the codemod land without rewriting ~90 call sites' variant/size props by hand. Adds a textStyle escape hatch to Text for arbitrary native styling (fontWeight, letterSpacing, etc) beyond variant/color/className. Three files kept on the old package where @expo/ui has no equivalent: GapSuggestionRow.tsx (Text as a MaskedView mask element — untested compatibility), demo/index.tsx (dev showcase using uiTextView/selectable), ChatBubble.tsx (one selectable Text for a copy/select bottom sheet). Typecheck-verified but not individually eyeballed on-device beyond the already-verified weather-alerts/settings screens — a visual QA pass across converted screens is still owed, since the matchContents-collapse bug found earlier is a silent, type-safe failure mode.
On-device spot-check of the codemod'd screens (shopping-list.tsx) found a real bug the typecheck couldn't catch: Text defaulting to vertical-only matchContents (to fix paragraph-text overflow) collapsed badge/label Text like "High"/"Medium" priority pills and the segmented-control-style toggle to zero-width slivers, since those need both-axes shrink-wrap. Adds an explicit `wrap` prop to Text (default false = shrink-wrap both axes, matching old NativeWindUI Text's default). Paragraph/note/description call sites across ~19 files now opt in explicitly. Verified on-device: badges, segmented toggle, and wrapped note text all render correctly simultaneously.
ActivityIndicator now wraps @expo/ui's SwiftUI ProgressView (iOS, controlSize+tint modifiers) and Jetpack Compose LoadingIndicator (Android, fixed Host dimensions since it has no size prop). Widens both to accept className/style via a locally-typed Host intersection, documenting the same swift-ui/jetpack-compose Host-doesn't-extend-ViewProps gap Universal's Host doesn't have. On-device sweep of the codemod'd Text call sites found the wrap default audit was incomplete: two more real overflow bugs (trail-conditions disclaimer, season-suggestions subtitle) were plain translated strings with no notes/description-style field name, so the earlier field-name scan missed them. A structural follow-up scan (note/callout boxes, disclaimer/hint/error copy, long strings) found 39 more across 30 files.
Checkbox wraps @rn-primitives/checkbox directly (already RN-native, same
zero-Host-risk pattern as Avatar) — no @expo/ui Universal Checkbox needed.
Found and fixed a third Text sizing bug on-device: wrap:true only stops
Host from shrink-wrapping (matchContents:{vertical:true}), it doesn't give
SwiftUI/Compose a width to actually wrap text against. A parent using
items-center (cross-axis center, not Yoga's default stretch) never hands
Host a width, so wrapped text with no explicit sizing class rendered one
word per line at ~0 available width instead of wrapping at the visual
container's width. Text now falls back to style={{ width: '100%' }} when
wrap is set and no explicit w-*/flex-1 class is present.
Two independent Host native-bridge boundaries can't correctly report
intrinsic size across each other: <Button><Text>Label</Text></Button> —
the shape left everywhere by the earlier codemod, since Button.label was
never used — collapsed the button to a near-zero-size blob with its label
overflowing outside it. This affected most already-migrated Button call
sites, not just size="lg" ones; the root cause is structural, not a
size/variant mismatch.
Button now auto-extracts a plain string when children is exactly one Text
(or raw string) with only string content, and passes it via @expo/ui's
label prop instead of nesting — zero call-site rewrites needed for the
dominant plain-text-label case. Icon+text or multi-child Button content
still nests a Host-bridged child and remains an unverified known gap.
Also documents a fourth Text sizing bug found on the same screen: wrap's
matchContents:{vertical:true} alone doesn't give SwiftUI/Compose a width
to wrap against when the parent uses items-center instead of stretch —
text rendered one word per line. Already fixed in the prior commit
(Text falls back to width:'100%' when wrap is set with no sizing class);
this documents the on-device repro that caught it.
Ports Card/CardContent/CardTitle/CardSubtitle/CardDescription/CardFooter as plain RN composition (View/BlurView/Text) rather than routing through @expo/ui — the original component never used a native-bridged view, only NativeWind-classed View/BlurView/Image/LinearGradient, so there's no Host risk to introduce. CardBadge/CardImage dropped (zero real call sites). CardDescription infers wrap from numberOfLines (undefined or >1 wraps, ===1 doesn't), matching the Text wrap findings from the last two commits. Not verified on-device (session was signed out mid-session, screens using Card require auth or live AI tool-call responses) — typecheck/lint clean, and Card carries the same risk profile as Avatar/Checkbox (no Host), both already verified clean on-device.
Toggle wraps react-native's Switch directly (already native on both platforms via RN core) rather than @expo/ui's Universal Switch, which is Host-bridged for no benefit over the RN component already used here. Verified on-device (weather-alert-preferences) alongside the earlier Text wrap fixes.
Ports as plain RN composition (FlashList + View/Pressable + Text) rather than routing through @expo/ui — ListItem specifically uses Pressable instead of the migrated Button, since it renders multiple Text children (title + optional subtitle): nesting a Host-bridged Button around Host-bridged Text children reproduces the Button-collapse bug fixed earlier. Press-state opacity (previously free via Button's plain variant) is now applied manually via Pressable's style callback. Narrows ListItemProps' titleStyle/subTitleStyle from TextStyle to ViewStyle — the only real call site (conversations.tsx's paddingRight timestamp-reservation hack) was layout, not typography, and Text's style prop routes to its Host box, not the native text itself. Verified on-device (messages/conversations: names, timestamps, previews, dividers, avatars all correct).
Ported as two platform files (text-field.tsx Material/Android, text-field.ios.tsx simple) matching the old package's genuine platform split — no @expo/ui Host bridge needed since the original was already plain RN (TextInput/Pressable/View/Reanimated). Verified on-device (iOS): auth credentials and login screens render correctly.
Ported directly — the old package's Sheet was already a thin wrapper around @gorhom/bottom-sheet's BottomSheetModal, not an @expo/ui component. Zero API changes, 17 call sites updated.
…e Phase 3 Ported directly — no @expo/ui Host bridge needed, the old package's Form was already plain RN View composition. FormSection's materialIconProps now uses this app's own Icon component (name string) instead of the old package's sfSymbol/materialCommunityIcon object shape; all real call sites already passed a plain name string so no rewrites needed. This completes Phase 3: Text, Button, List, Toggle, TextField, Sheet, and Form are all off nativewindui. Remaining: Phase 4 (Alert, ContextMenu/DropdownMenu, Toolbar) and Phase 2's SearchInput.
Neither platform needed an @expo/ui Host bridge — the old package's Android/default Alert was already built on @rn-primitives/alert-dialog (an RN primitive), not @expo/ui, resolving the AlertDialog 2-button/no-prompt-slot limitation that previously blocked this. iOS now uses RN core's Alert.alert/Alert.prompt directly instead of routing through @expo/ui's SwiftUI Alert and its unverified Trigger mechanism. materialIcon now takes this app's own Icon name string instead of the old package's sfSymbol/materialCommunityIcon object shape; real call sites already matched the new shape. 18 call sites updated.
Neither needed an @expo/ui Host bridge on either platform — Android/default used @rn-primitives/context-menu and @rn-primitives/dropdown-menu (unstyled RN primitives), iOS used react-native-ios-context-menu (a separate third-party native library). The previous "unverified RNHostView/Trigger mechanism" blocker was based on a wrong assumption that these used @expo/ui, same as the resolved Alert blocker. icon/materialIcon fields now use this app's own Icon (name string + color) instead of the old package's sfSymbol/materialCommunityIcon object shape — no real call site used icon/image so this is a type-only change. Widened Button's props to accept accessibilityHint/onLayout, needed by menu item rows and submenu trigger positioning. 11 call sites updated.
…ose out Phase 4 No @expo/ui Host bridge needed — the old package's Toolbar already wrapped expo-blur's BlurView, an already-installed dependency. Same pattern as Alert/ContextMenu/DropdownMenu: the "high-risk, paused" Phase 4 assessment was based on an unverified assumption these used @expo/ui, when the old package's actual source was plain RN composition throughout. This closes the entire component migration except Phase 2's SearchInput. 2 call sites updated.
…racker Kept as a component (two platform files, like TextField) rather than migrating to headerSearchBarOptions per the original plan — all 6 real call sites render it inline in a modal/screen body, not as native nav-bar search, so headerSearchBarOptions doesn't fit. No @expo/ui Host bridge needed, plain RN composition on both platforms. Widened Button with accessibilityLabel (SearchInput's pill-button trigger needs it). This closes packages/ui/nativewindui/index.ts entirely — every originally-exported component is now ported to packages/ui/src/. Final removal phase (drop the @packrat-ai/nativewindui dependency) not started yet.
…tion tracker Both were unused after the Alert and other migrations landed. Documented the 3 remaining Text/Button call sites that intentionally stay on nativewindui (MaskedView, selectable text, demo screen) so the tracker doesn't read as "still pending".
…ui to selectable-text only GapSuggestionRow.tsx: migrated all 12 Text uses except the 2 inside MaskedView's maskElement/masked content (ShimmerFindingText) — those two also migrated, converting typography style props to textStyle. Dropped fontStyle/italic (no @expo/ui equivalent, cosmetic only). MaskedView-as-maskElement compatibility with a Host-bridged Text is architecturally reasoned to work but unverified on-device — flagged in the migration doc for a manual check, since reaching this screen needs a real pack + gap-analysis API call. demo/index.tsx: migrated every Text/Button except the one SelectableTextExample. Replaced an inline onPress-on-Text hyperlink pattern (not representable — @expo/ui Text has no onPress and string-only children) with a Pressable-wrapped Text. @packrat-ai/nativewindui now has exactly 2 remaining call sites (demo/index.tsx, ChatBubble.tsx), both for the same reason: selectable/uiTextView text-selection has no @expo/ui equivalent on any platform. This is a permanent, justified exception, not a pending migration step — the dependency cannot be fully removed without dropping the long-press-to-copy text-selection feature.
…ation The last blocker (selectable/uiTextView text-selection, no @expo/ui equivalent on any platform) didn't require keeping the package after all — its Text component's selectable support was itself just a wrapper around react-native-uitextview, already a direct apps/expo dependency. Wrapped it directly as packages/ui/src/selectable-text.tsx. Removed everywhere: apps/expo's direct dependency, the root overrides version pin, the bunfig.toml GitHub Packages scope, the now-unnecessary configure-deps.ts preinstall token-gate script, and the corresponding CLAUDE.md documentation section. bun install now succeeds without PACKRAT_NATIVEWIND_UI_GITHUB_TOKEN set at all. Full typecheck and lint clean. packages/ui/nativewindui/index.ts kept as a completed-migration changelog rather than deleted, since it documents real architectural decisions. This completes the NativeWindUI → Expo UI migration.
@expo/ui's Host-wrapped Text reports a narrower intrinsic size to Yoga than a plain RN Text did, so measure(animatedRef) under-reserved the paddingRight/translateX space for the Cancel button, closing the gap between it and the search box. Use the same fixed character-width estimate both animated styles already had as a fallback instead of the live-measured width. Also adds defensive .gitignore entries for /ios/ and /android/ at repo root — bun expo <subcommand> (e.g. prebuild) resolves to the expo binary directly instead of the root "expo" script when extra args are passed, generating a stray Xcode/Gradle project at repo root instead of apps/expo.
button.tsx, loading-indicator.ios/android.tsx, lib/text-class-parser.ts, context-menu.tsx, dropdown-menu.tsx, alert.tsx, and list.tsx used raw typeof primitive checks introduced during the nativewindui -> @expo/ui migration, tripping the no-raw-typeof lint rule (raw typeof is only allowed inside packages/guards, packages/utils, and for global availability checks). Swapped each for isString/isNumber/isObject from @packrat/guards, which packages/ui now declares as a dependency.
# Conflicts: # apps/expo/app/(app)/pack-stats/[id].tsx # apps/expo/app/(app)/settings/index.tsx # apps/expo/app/(app)/trail-conditions.tsx # apps/expo/app/auth/index.tsx # apps/expo/features/trips/screens/TripDetailScreen.tsx
…asts:strict
Resolved the pre-existing check:casts:strict backlog in packages/ui/src
(16 unsafe `as X` casts flagged, all predating this branch's own changes):
- search-input.{tsx,ios.tsx}, text-field.{tsx,ios.tsx}: the useAugmentedRef
`ref as XRef` casts were masking a real ref?/required-Ref mismatch, not
narrowing a disagreement — replaced with `ref ?? null`.
- dropdown-menu/dropdown-menu.tsx, context-menu/context-menu.tsx: replaced
`(item as Partial<X>)?.items` + `item as X` chains with `'items' in item`,
which TS already narrows correctly for these discriminated unions.
- context-menu/context-menu.tsx: `rootRef.current as unknown as View` was
redundant — ContextMenuMethods already extends View.
- dropdown-menu/utils.ts, context-menu/utils.ts: kept the Object.assign
casts (TS can't express the merged-object union through Object.assign's
signature) but annotated with // safe-cast: instead of leaving them bare.
- lib/text-class-parser.ts: dropped the redundant `colors as TailwindPalette`
identity cast; annotated the two casts that survive (regex-derived string
widened to a palette key, and the resulting record indexed by shade).
- button.tsx: kept the `variant as LegacyButtonVariant` cast (`in` doesn't
narrow string-literal unions by key membership) with a safe-cast note.
…ntry The nativewindui->@expo/ui migration removed the root override in a prior commit but left the registry entry (and its rationale paragraph) in docs/dependency-policy.md, tripping check:overrides (every root override must have a matching registry entry, and vice versa).
CI's lint:custom (no-owned-max-params.ts, max 1 param for owned functions, not covered by the local pre-push hook) flagged createContextSubMenu, createDropdownSubMenu, getPreviewConfig, and textMatchContents for taking 2 positional params. Collapsed each into a single object param and updated call sites; no behavior change.
CI's root tsc caught these three files, added by unrelated upstream work
during the merge with origin/development, still importing Text/Button/
ActivityIndicator from the old @packrat/ui/nativewindui path (which no
longer re-exports them post-migration):
- app/(app)/dev/paywall-state.tsx
- features/purchases/components/CustomerCenter.tsx
- features/purchases/components/EarlyAccessGate.tsx
Also fixes settings/index.tsx: the new @expo/ui-backed Text takes color
via `textColor`, not a `style={{ color }}` prop (Host's style only reaches
the box, never the native-bridged text).
CI's per-package check-types (packages/ui's own tsc --noEmit, stricter than the root program) flagged both files' `nativeEvent` destructure as implicitly any. react-native-ios-context-menu ships no .d.ts files at all (the existing @ts-expect-error on its import documents this upstream bug: dominicstop/react-native-ios-context-menu#129), so OnPressMenuItemEvent's own param type carries no usable shape. Declared a local ContextMenuNativeEvent type matching the properties actually read.
A/B'd the migrated build against the pre-migration NativeWindUI build on a physical Android device and an iOS simulator. The prod APK (v2.1.0, tag a1b4362) still ships @packrat-ai/nativewindui and none of the migration commits are ancestors of main, so it works as a real baseline. Root cause behind most of the defects: @expo/ui's `Host` has no RN-side intrinsic size. Any axis it doesn't `matchContents` is sized by Yoga, which sees no content, so it collapses. Real text does min(content, parent); Host can only shrink-to-content (overflows) or stretch-to-parent (needs a stretch context). No default is correct for both row and column parents. Text and Button are now plain RN Text/Pressable. Only ActivityIndicator and SegmentedControl still use @expo/ui. Fixed (all reproduced on-device against the baseline): - tailwind.config.js never included packages/ui/src and still listed the deleted nativewindui path, so ~76 classes used only there compiled to nothing: invisible ListItem separators, zero-size checkboxes, Form sections with no spacing, a full-bleed alert dialog. - Android DropdownMenu never opened. Button dropped the ref that @rn-primitives measures to place its portal, and @expo/ui hands children to a Compose composable where the hosted RN view swallows the touch so onClick never fires. Verified by instrumenting onPress. - Buttons with non-text children collapsed ("Continue with Google" rendered its label outside an empty pill); buttons without a sizing class lost full-width; filled buttons lost the brand color. - Text.flattenToString replaced element children with '', deleting the consent screen's Terms/Privacy links, the OTP email and chat timestamps. - text-center was a no-op (127 sites); numberOfLines>=2 could never wrap; 214 typography classes were silently dropped (text-white, dark:/ios: prefixes, tracking/leading, arbitrary values, opacity modifiers, text-5xl+). - ListItem leftView/rightView stretched to full row height, top-pinning tile icons instead of centering them against the text. - A variant's lineHeight is no longer kept when className overrides the font size (clipped the iOS auth headline). - alert.ios.tsx show() was a no-op, so three tile alerts did nothing on iOS. - bottom-sheet.tsx destructured `index` then hard-coded index={0}. `wrap` is now inert and deprecated; kept so existing call sites compile.
…style prop
Button's `size` prop did nothing. Sizes and press feedback were delivered as an
inline `style={({ pressed }) => [SIZE_STYLE[size], pressed && PRESSED_STYLE, style]}`,
but NativeWind's cssInterop owns the `style` prop on a `className`'d component and
drops the function form that Pressable needs, so the whole array was discarded.
Measured on-device (TECNO KL4, 2x density) before the fix:
- `size="icon"` buttons rendered at the glyph's intrinsic ~20dp instead of 40dp
(pack card overflow menu: 40x42px = 20x21dp).
- The default `md` size contributed no padding at all, leaving only the 1dp
border between label and edge — "Add Item" on the pack detail screen measured
142px wide around a 138px label, so the rounded border cut into the glyphs.
"Ask AI" next to it only looked right because `flex-1` sizes it and centring
hides the missing padding.
As classes they go through the same pipeline as the variants, and since `cn` is
`twMerge` a call site's own `px-*`/`h-*` still overrides them. 97 call sites pass
`size`; none passes `style`, so nothing else relied on the dropped prop.
After the fix the pack card's overflow menu is 80x80px (40x40dp) with its glyph
centred at (616, 744) — the exact position the pre-migration build renders it,
confirming the "cosmetic" offset was a symptom of this bug rather than a separate
quirk. A held button now measures 0.70x the released frame's mean brightness.
ListItem carried the same pattern and the same silent no-op; fixed alongside.
…platform palette SegmentedControl is one of the two components still backed by @expo/ui, so it paints from the *platform* palette rather than the app's. No call site passed `tintColor`, so on Android it picked up Material You's dynamic colour: on a device themed brown, the Settings screen's unit switches rendered a brown selected segment in an app whose accent is blue everywhere else on the same screen (Sign In, "Upgrade to Pro", the category chips). Default `tintColor` to the theme's primary; call sites can still override. Verified on-device against the pre-migration build. The selected segment is now the app blue. Two native Material 3 traits remain and are not tintable — the checkmark on the selected segment and the outline colour of the unselected one.
… not sizing
Attempted the migration and hit a harder constraint than the one everybody
remembered. Established from @expo/ui's source rather than by probing:
Host is `export function Host(props: HostProps)` — a plain function component
with no forwardRef. HostProps is a closed list (matchContents, onLayoutContent,
useViewportSizeMeasurement, colorScheme, seedColor, layoutDirection,
ignoreSafeAreaKeyboardInsets, children, style, pointerEvents) and does NOT extend
RN's ViewProps, since PrimitiveBaseProps is just { modifiers? }. So no testID, no
accessibilityRole, no aria-*, and no way to attach a ref.
button.tsx needs both, and its own comments say why. The @rn-primitives menu
primitives inject a ref through Slot and call .measure() to position their
portal — dropping it is why the Android category DropdownMenu never opened. And
asChild primitives inject role/accessibilityState/nativeID, without which screen
readers announce menu rows as bare buttons with no checked/disabled state. Four
call sites wrap Button in Link asChild, which clones the child to inject onPress.
So Button stays RN across 196 call sites, and Text across 147 for the same reason
(numberOfLines, selectable, onLayout, a11y props). Not sizing: that's fixed, and
the doc keeps the measured table plus the Host sizing rule that caused three
false conclusions earlier in this migration.
Revisit if Host gains forwardRef and ViewProps — that single upstream change is
what blocks both.
…id's are thin
Looked up ref/accessibility for Host properly instead of relying on the installed
type definitions.
Ref: confirmed against the upstream changelog at every version, not just 57.0.9 —
forwardRef/ref support has never been added to Host at any release. That half of
the earlier note holds.
Accessibility: my earlier "no accessibility props" was too broad. A11y IS
available through modifiers, but the two platforms are very unequal. iOS has
accessibilityLabel/Hint/Value/Identifier/Hidden/Element/AddTraits/RemoveTraits/
InputLabels. Android has only semantics, testID, selectable, selectableGroup and
toggleable — and Android's `semantics` takes just { contentType?: string } with no
label and no role, while selectable/toggleable roles are limited to
radioButton|checkbox|switch|tab with no `button`.
So the conclusion is unchanged but the reasoning is now accurate: an @expo/ui
Button on Android cannot be given the label/role that asChild primitives inject
and screen readers announce, and Host still cannot receive the ref that
@rn-primitives menu triggers call .measure() on.
docs/BETA_TESTER_HANDOFF.md was in the wrong place with a shouty name that matched nothing else in the repo. docs/qa/ already holds exactly this kind of document, so it now sits alongside revenuecat-beta-test-plan.md and follows the same conventions: - path: docs/qa/native-android-controls-beta-test-plan.md - filename: kebab-case <feature>-beta-test-plan.md, matching its sibling - title: "Native Android Controls — Beta Tester Guide & Test Plan", matching the sibling's "PackRat Pro — Beta Tester Guide & Test Plan" Named for the feature rather than the branch, so it stays accurate after merge. Content unchanged; no other file referenced the old path.
Checked the draft against Wikipedia's "Signs of AI writing" and it was carrying a
cluster of them. Fourteen em dashes was the loudest one, but the rest mattered
too:
- Inline-header bullets ("**Where:**", "**Known and expected:**") in every
section, which is the vertical-list-with-bolded-headers pattern
- Boldface used mechanically for emphasis, roughly thirty instances
- Title case in the heading ("Beta Tester Guide & Test Plan")
- A negative parallelism ("it is not new") and a rule-of-three ("look, animate
and feel")
- A warm generic closer that thanked the reader before restating the ask
Rewrote it as prose a person would write: locations as plain sentences rather
than labelled fields, varied sentence length, and emphasis carried by word order
instead of bold. Sentences like "Start here" and "Skip reporting it" say the same
thing the bolded versions did with less ceremony.
Content is unchanged. Same six areas, same priority on bottom sheets, same two
known-and-expected items, same escalation list. Verified zero em dashes, zero en
dashes, zero curly quotes, zero inline-header bullets, and no hits on the AI
vocabulary list.
Trims the guide to the areas we most want covered and drops the sections that
were doing more explaining than directing.
- Removed the overflow menus section along with the `?` icon note, so nothing
points testers at a known cosmetic gap
- Removed the bottom sheets priority note; the checks stand on their own
- Removed the Notes section
- Asks testers to cover as many screens as possible rather than only the ones
they already use
- Switch and setting checks now read as instructions ("confirm that it
persists", "verify that the setting actually takes effect") instead of
questions
…xpo/ui The pre-push hook runs check:all, which caught two things the earlier per-package checks missed. expo-sqlite was still on 56.0.4 while everything else moved to SDK 57. This was mine: the root catalog pinned ~56.0.4, which overrode apps/expo's ~57.0.1, so `expo install expo-sqlite` kept reinstalling the old version. Aligned the root pin. @expo/ui was installed twice at the same version (once hoisted, once under apps/expo), which expo-doctor flags because a native build can only contain one copy of a native module. Removed the nested copy. apps/expo now reports 20/20 expo-doctor checks passing. check:all still fails on socket/low-supply-chain-score for apps/guides dependencies. That one is pre-existing and reproduces on origin/development, so it is not from this branch.
check:casts:strict blocked the push on `variant as ResolvedVariant` in resolveVariant. The cast was avoidable rather than a genuine type-system gap. VARIANT_MAP only covered the four legacy names, so resolving a variant needed an `in` check plus two casts to convince TypeScript that the remaining three members of ButtonVariant were already ResolvedVariant members. Widening it to Record<ButtonVariant, ResolvedVariant> with the three current names mapping to themselves makes resolveVariant a one-line lookup with no cast at all. Better than silencing it: the Record type now fails the build if a variant is added without a resolution, which the in-check could not catch. CLAUDE.md's rule about each cast being a place where two types disagree applied literally here.
Contributor
|
Important Review skippedToo many files! This PR contains 226 files, which is 126 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (226)
You can disable this status message by setting the 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 |
github-actions
Bot
added
documentation
Contributor
Coverage Report for packages/units (./packages/units)
File CoverageNo changed files found. |
Contributor
Coverage Report for apps/expo (./apps/expo)
File CoverageNo changed files found. |
Contributor
Coverage Report for packages/utils (./packages/utils)
File CoverageNo changed files found. |
Contributor
Coverage Report for packages/mcp (./packages/mcp)
File CoverageNo changed files found. |
Contributor
Coverage Report for packages/overpass (./packages/overpass)
File CoverageNo changed files found. |
Contributor
Coverage Report for packages/api (./packages/api)
File CoverageNo changed files found. |
Contributor
Coverage Report for packages/analytics (./packages/analytics)
File CoverageNo changed files found. |
Contributor
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
packrat-admin | 388c142 | Commit Preview URL Branch Preview URL |
Aug 07 2026, 08:33 PM |
Contributor
Deploying packrat-landing with
|
| Latest commit: |
388c142
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://17bef7b2.packrat-landing.pages.dev |
| Branch Preview URL: | https://feat-expo-ui-migration-sdk57.packrat-landing.pages.dev |
Contributor
Deploying packrat-guides with
|
| Latest commit: |
11498ef
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c58ff697.packrat-guides-6gq.pages.dev |
| Branch Preview URL: | https://feat-expo-ui-migration-sdk57.packrat-guides-6gq.pages.dev |
The guide said "Android only", which was wrong. iOS gets real changes in this build: sheets across 17 call sites, switches, loading spinners, segmented tabs, and the date, picker and masked-view drop-ins. Renamed off "native-android-controls" since the title no longer holds. Each section now says which platforms it applies to, because the split is uneven: checkboxes, dialogs, cards and overflow menus are Android only, while sheets, switches, spinners and the date picker changed on both. Added a "where to spend your time" section pointing testers at Android, since that is where most of the build landed, and told them a control looking unchanged on iOS is expected rather than a bug. They test both devices, so reports now need to say which one they came from. Also restored the bottom sheets priority note and an overflow menus section. The sheet dismiss bug is the one real regression this branch introduced, so it is worth naming, and the menus changed on Android without the guide mentioning them.
The doc said "Host gains forwardRef and ViewProps" then called it "that single upstream change". They are two independent changes and they block different call sites, so the section now separates them. forwardRef fixes the eight sites that sit inside a slot which clones the child: four @rn-primitives menu triggers, where the primitive calls .measure() on the ref to set triggerPosition and also writes node.open/node.close onto it, and four Link asChild buttons where expo-router injects onPress. Listed with file and line, plus the note that the two .android.tsx menu triggers no longer go through the primitive at all now that dropdown-menu.android.tsx drives expanded from its own Pressable. HostProps extending ViewProps is separate and unaffected by the ref. It blocks the three packages/ui components that render a Button themselves and pass a11y props through it: alert.rn.tsx, toolbar.tsx, search-input.tsx. alert.rn.tsx matters most since it is still the live path for iOS prompts, web, and any alert with more than two buttons. Also records that the other ~188 Button uses are ordinary and would migrate without trouble, and drops an em dash I had left in the sizing rule paragraph.
…confirmed Verified on iPhone 17 Pro sim (iOS 26.4) from a fresh prebuild that brought pods from ExpoUI 56.0.16 up to the 57.0.9 JS: - DateTimePicker: native picker opens, selection writes back, validation fires - Sheet: presents correctly and BOTH dismiss gestures work, so the Android dismiss regression does not reproduce; sheet-to-sheet handoff also works - SegmentedControl: native picker, onIndexChange fires, value survives a cold launch - Toggle: 9 instances with correct per-row state, onValueChange fires - ActivityIndicator: ProgressView animates, small/large map to distinct controlSizes (temporary probe, reverted) - Button: filled/outlined/text plus icon+text children all render correctly, which closes the multi-child nested-Host gap on iOS - Alert: AlertAnchor presents a real UIAlertController (was previously unverified on an iOS device) Also records two simulator-driving gotchas that cost time: Maestro can't read this app's RN hierarchy on iOS (coordinate taps only, integers only, re-measure after layout shifts) and the URL scheme is exp+packrat://.
… Alert claim AlertDialog was the last @expo/ui-backed component with no device result. Verified on the TECNO KL4 via /admin/ai-packs -> Generate Packs (a two-button confirm, so it takes the Compose path, not the RN fallback): - renders as a real M3 dialog with title and message slots - both button slots fire (onClick is wired; onPress would silently no-op) - labels resolve, so the Compose <Text>-child requirement is satisfied - onDismissRequest fires on hardware back and is consumed by the dialog, not the navigator - uiautomator dump confirms real TextViews per slot and a Button per action; Material orders confirm-right/cancel-left despite array order Also verified Android's LoadingIndicator in the same pass (Settings -> AI Models during a download) — a different component from iOS's ProgressView, and Android-only since the row is !isApple gated. Correction: an earlier commit recorded iOS Alert as verified because a failed login showed a UIAlertController. That call site calls RN core's Alert.alert directly, not the migrated AlertAnchor, so it proved nothing about alert.ios.tsx. Reverted that claim in both places. Both fallback branches (prompt, >2 buttons) stay unobserved by construction: their only call site is auth-gated and they are unconditional early-returns with no Compose branch.
…tionale Two spots, both aimed at readers who don't know what Expo or a component is. Intro: "the real native components" was accurate but abstract for a tester. Reframed as imitations vs the real ones, anchored to something they can picture — the controls in their own phone settings. Where to spend your time: finished the unfinished sentence about prioritizing Android. Split it into the small reason (more of this build landed there) and the real one (the iPhone app is moving to Apple's own tools, so this shared code is becoming the Android foundation rather than something both phones borrow — an Android bug found now gets fixed in code we keep, an iPhone one lands in code we're replacing). No mention of Expo or Swift, which testers wouldn't parse. Also made the platform vocabulary consistent (iPhone, not iOS, in tester-facing copy) and fixed "as much screens" -> "as many screens".
…ck to one line "and in every other app" was simply false — plenty of apps ship custom controls rather than the platform's. Narrowed to the phone's own settings and its built-in apps, which is both true and still something a tester can picture. Also trimmed the Android-priority explanation back to the original three lines. The three-paragraph version explained more than the point was worth.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Moves PackRat's shared UI components off the private
@packrat-ai/nativewinduipackage and onto@expo/ui, so the app uses real platform controls instead of hand-built lookalikes. Also upgrades to Expo SDK 57, which several of these components need.The private-package dependency is the main thing this retires: it was installed from GitHub Packages behind a token that no longer exists, so it could not be reinstalled from scratch. There are now zero imports of it.
11 components are now native
ToggleToggle/ Material 3SwitchActivityIndicatorLoadingIndicatorCheckboxCheckboxAlertAlertDialogCardCard+RNHostViewDropdownMenuDropdownMenuSheetcommunity/bottom-sheet, 17 call sitesSegmentedControlcommunity/segmented-controlDateTimePickercommunity/datetime-pickerPickercommunity/pickerMaskedViewcommunity/masked-viewFour community packages were removed outright, replaced by
@expo/ui's API-compatible versions:@gorhom/bottom-sheet,@react-native-community/datetimepicker,@react-native-picker/picker,@react-native-masked-view/masked-view.Eight components deliberately stay React Native, each for a measured reason recorded in
docs/migrations/nativewindui-to-expo-ui.md. The short version:ButtonandTextbecauseHosthas noforwardRefand Android's accessibility modifiers have nobuttonrole, which breaks the ref-and-.measure()contract that@rn-primitivesmenu triggers depend on;TextFieldbecause the native one needsuseNativeState, incompatible with TanStack Form;list/ListItembecause aHostper row stops aFlashListscrolling;Form,Toolbar,Avatar,SelectableTextandContextMenubecause no equivalent native surface exists or nothing renders on Android.Two bug fixes found along the way
@gorhomswap, leaving two sheets impossible to close. Caught in review and fixed before it shipped anywhere.Type of change
Area(s) affected
apps/expo)Testing
Every migrated component was verified on Android hardware (TECNO KL4) rather than from types alone, using accessibility-tree dumps plus screenshots. Highlights:
Sheets: back button and scrim tap both dismiss, and
onDismissfires.Alert: real Material dialog, both button callbacks fire, back button dismisses.
Card: all four variants render with correct elevation and typography, buttons inside stay tappable.
DropdownMenu: verified on the real Messages screen, where selecting "Go Home" navigated to the dashboard.
DateTimePicker: full round trip in the trip form, including the date persisting back into TanStack Form and validation firing.
Auth wall: Sign In now present in the accessibility tree and navigates.
Added / updated unit tests (392 pass; no new tests, these are UI wrappers)
Manually tested on iOS
Manually tested on Android
Manually tested on Web
iOS is not manually tested. It bundles cleanly and only
ToggleandActivityIndicatorchange there, but it needs a pass on a device before release.docs/qa/native-android-controls-beta-test-plan.mdscopes beta testers to Android for the same reason.Screenshots / recordings
Captured throughout development and available on request. The clearest before/after is the auth wall: Sign In was absent from the view tree entirely, and now renders and navigates.
Pre-merge checklist
bun format && bun lintpasses with no errorsbun check-typespasses with no errorsbun check:allreports one remaining failure,socket/low-supply-chain-scoreforapps/guidesdependencies. It reproduces onorigin/development, so it is not from this branch.Notes for review
Two things worth knowing about the commit history. It documents several wrong turns rather than hiding them, because the reasoning is load-bearing:
Cardtook four attempts and the first three conclusions were wrong, and the doc records what was ruled out so nobody retries them. Similarly, theDropdownMenuentry records a real upstream defect (enabled={false}onDropdownMenuItemis presentation-only and does not block the press) along with the five workarounds that failed, plus why it is unreachable in this app: no call site setsdisabled.The single rule most worth carrying forward is about
Hostsizing, since getting it wrong caused three separate false conclusions here.matchContentsis for intrinsic sizing; usestylewhen you need an explicit size; and never forward aclassNameto aHost, becausecssInteropturns it into astylethat fightsmatchContents.