upgrade to nativewind@4 - #3826
Conversation
|
|
||
| const GREEN_CIRCLE_CLASS = "bg-inatGreen rounded-full h-[36px] w-[36px] mb-2"; | ||
| const ROW_CLASS = "flex-row justify-center space-x-4 w-full flex-1"; | ||
| const ROW_CLASS = "flex-row justify-center gap-x-4 w-full flex-1"; |
There was a problem hiding this comment.
space-x/y-* polyfills removed in v4 → converted all 34 usages to gap
|
|
||
| // className support: third-party components need explicit registration with | ||
| // nativewind 4 for className to have any effect | ||
| cssInterop( MapView, { className: "style" } ); |
There was a problem hiding this comment.
Third-party components lost styling → cssInterop() registrations for SafeAreaView, LinearGradient, FasterImageView, BottomSheetTextInput, and MapView
There was a problem hiding this comment.
We could add it to StyledComponent, seems to me the only cssInterop call outside of the pattern.
| const Body3 = ( props: TextProps ) => ( | ||
| // eslint-disable-next-line react/jsx-props-no-spreading | ||
| <InatText className={`text-xs ${tailwindFontMedium}`} {...props} /> | ||
| <InatText {...props} className={classnames( "text-xs", tailwindFontMedium, props.className )} /> |
There was a problem hiding this comment.
See Context for twMerge / classnames in PR description for further explanation
| @@ -1,5 +1,7 @@ | |||
| // @flow | |||
|
|
|||
| import "../../global.css"; | |||
There was a problem hiding this comment.
| // Core react-native components are registered with nativewind automatically; | ||
| // third-party components need explicit cssInterop registration for className | ||
| // to have any effect |
There was a problem hiding this comment.
FYI on styled vs cssInterop
jtklein
left a comment
jtklein
left a comment
There was a problem hiding this comment.
Update related code changes and migrations seem all fine to me. But I do see quite a few regressions in the UI, so I'll post before and after screenshots in Slack for things to fix.
| @@ -0,0 +1,28 @@ | |||
| // Makes className resolve to styles in jest the way it does in the app. | |||
There was a problem hiding this comment.
I am not so happy about this file and the other jest setups. This seems like a lot of extra setup required just so that we can work with this one package. I couldn't find any reference to jest in the nativewind docs. So, I am wondering how we get to this file content?
Makes me a bit nervous to not find official guidelines on how to setup nativewind with jest.
There was a problem hiding this comment.
Sorry for the delay in getting back to this.
The change is necessary because the package fundamentally changed how it works.
Before: the babel transform wrapped the classname'd components with nativewind wrappers that did tailwind => js styles interpolation within the implementation of the wrapper component
Now: it does more of a tailwind => css => rn-css-interop which is now baked into metro. Since jest doesn't use metro, this nativewind setup recreates that relationship. I couldn't get this to work with the exported nativewind/test helpers because they don't use react-testing-library's render. So I deferred to Claude on this what reverse-engineered the same approach from the internals. I included some of the rationale of this in the new agent doc. Let me know if you think more of this belongs in the file itself.
I agree it's not ideal but I think it's fair that we sometimes have extra setup for key packages (such as Realm).
|
|
||
| // className support: third-party components need explicit registration with | ||
| // nativewind 4 for className to have any effect | ||
| cssInterop( MapView, { className: "style" } ); |
There was a problem hiding this comment.
We could add it to StyledComponent, seems to me the only cssInterop call outside of the pattern.
jtklein
left a comment
jtklein
left a comment
There was a problem hiding this comment.
Still seeing wrong styles in the buttons inStandardCamera and SoundRecorder:
|
@jtklein latest commit addresses the "pill" button issues in media. On testing, the 
|
I don't see an issue with the ? |
Oh, I saw the |
Screenshot looking good. Do you want me to test again? |
jtklein
left a comment
jtklein
left a comment
There was a problem hiding this comment.
Haven't tested the last screenshot again, but did not find anything else in the prior testing round, so if that is fixed I'd say it's good to go, and we fix anything else forward.
2 participants
Closes MOB-1148
This PR upgrades to Nativewind@4 (note: v3 was canceled, 4 is the next step from 2). Nativewind 4 is a significant rewrite so this necessarily touches quite a few files.
I cherry-picked the first commit from a spike I did on this in May, which includes everything necessary to wire up the project on v4. There were a number of things off in this commit around text, maps, but the core interface are functional.
For fixing the rest, this is my first time using Claude for a "big" problem. I thought it would be well-suited at finding the nooks and crannies of the app to apply systemic / migratory changes. I'm sharing the Claude Plan Document and Claude's summary of changes below:
Plan for fixing some enumerated styling issues
Finish the NativeWind 2 → 4 migration
Context
The spike commit (
7a999b474) did the core wiring correctly: babel preset, metrowithNativeWind,global.csswith@tailwinddirectives,nativewind/presetin tailwind config, and removal ofstyled(). I verified empirically (compiled the real CSS through nativewind'scssToReactNativeRuntime, and rendered real Typography components through the repo's actual babel chain in jest) that the font classes themselves compile and apply correctly —font-Lato-*→fontFamily: "Lato-*"end to end, including with the react-compiler plugin active.Answer to the open question: nothing needs to move from
tailwind.config.jsto CSS. NativeWind 4 uses Tailwind 3's JS config;global.cssstays as just the three@tailwinddirectives. (Moving theme to CSS is a Tailwind v4 / NativeWind v5 thing.)The reported symptoms map to four real, verified issues:
remat 14px (v2 used 16px) → all rem-based spacing (p-*,m-*,gap-*,h-22, …) shrank 12.5%. Verified:p-4compiles to 14 instead of 16.styled()and are NOT auto-interop'd in v4 —classNameon them is silently ignored (BottomSheetTextInput,SafeAreaView,LinearGradient,FasterImageView). Also a pre-existing bug:KebabMenu.tsx:43setsfontFamily: tailwindFontMedium(the class name"font-Lato-Medium") instead of the font name → kebab menu items always fell back to system font.font-Lato-Regularandfont-Lato-Boldrenders Regular regardless of string order (Regular is generated last among the Lato keys). Anywhere that appends a font override onto a base class that already has a font class now picks the wrong one.React.createElement(...)to a hoisted_ReactNativeCSSInterop.createInteropElement(...)import;jest.mockfactories intests/jest.setup.js:196-208useReact.createElement→ babel-plugin-jest-hoist rejects the out-of-scope reference.Changes
1. Restore 16px rem base —
metro.config.jsThis alone should fix the app-wide cramped look.
2. Register third-party components —
src/components/styledComponents.tsCore RN components (View, Text, TextInput, Modal, Pressable, ScrollView, Image, ImageBackground, KeyboardAvoidingView…) are auto-registered in v4 — leave those as plain re-exports.
PressableWithTrackingis fine too (it forwards props to corePressablefrom app-compiled code). Register the rest:(Keep the existing
SafeAreaView === undefinedjest fallback.)3. Wrong weights — fix real double-font-class sites
Most elements carry only one font class because
InatText's{...props}spread replaces the merged className (pre-existing behavior, unchanged by the migration — verified in the probe). So this is an audit, not a rewrite:font-Lato-*with a caller-supplied secondfont-*class via properclassnames()merging (e.g.src/navigation/,CustomTabBar, button/label components — the files matchingprops.className+classnames).fontFamilykeys intailwind.config.jsto game the cascade — it can't restore string-order semantics and just moves the problem.4. Pre-existing font bug (drive-by) —
src/components/SharedComponents/KebabMenu.tsx:43fontFamily: tailwindFontMedium→fontFamily: fontMedium(import fromappConstants/fontFamilies).5. Fix jest
tests/jest.setup.js:196-208: rewrite the twojest.mockfactories to avoid the createElement rewrite — e.g.jest.fn( ( { children } ) => children ?? null ), orjest.requireActual( "react" ).createElementif a host element is required (the plugin only rewrites bindings imported viarequire("react")/import).App.js'simport "../../global.css"doesn't break tests:moduleNameMapper: { "\\.css$": "<rootDir>/tests/mocks/cssMock.js" }(new empty-object mock file).npm run test:unit, integration). Expect some churn: className styles do NOT resolve to style objects in jest anymore (metro injects the compiled CSS; jest has no CSS registered), so snapshots/assertions that relied on tailwind-derived styles may need updating.tests/unit/components/SharedComponents/DisplayTaxon.test.jsassertsopacity: 0.5viatoHaveStyle— check whether that comes from an inline style (fine) or a class (needs adjustment).setupFilesAfterEnvusingnativewind/testhelpers (requires adding@tailwindcss/container-queriesas a devDependency — I already confirmed the helper needs it).6. Housekeeping
tailwindcss^3.3.2→^3.4.17(nativewind 4's recommended line; peer range allows it).tests/unit/nativewind4-font-probe.test.tsx.node_moduleshas an unsaved@tailwindcss/container-queriesinstall (--no-save) — either add it as devDependency (contingency above) ornpm installto restore.Verification
npm run ios(or android) and check againstmainside by side:src/components/Developer/UiLibrary/Typography.js) — every Heading/Body variant shows correct Lato weight/size.BottomSheetTextInput-based inputs and SearchBar show Lato, gradients/safe-area layouts styled again.verifyInstallation()fromnativewindonce in dev to confirm the runtime sees the compiled CSS, then remove.npm run test:unitandnpm run test:integrationpass.Summary of changes
NativeWind 2 → 4 migration summary
Your spike wiring (babel preset, metro withNativeWind, preset in tailwind config, styled() removal) was correct. The problems were all runtime behavior changes between v2 and v4:
The core mental-model shift (explains almost every symptom): NativeWind 2's babel plugin resolved className on every capitalized component into styles at the call site, cascading through wrapper layers as style arrays where later-in-array won. NativeWind 4 only styles components explicitly registered with the interop, className rides along as a plain prop, conflicting classes resolve by stylesheet order instead of string order, and inline styles beat classes.
What was fixed:
Jest: CSS is compiled once in globalSetup and injected per worker (tests/nativewind.setup.js), so classNames resolve to real styles in tests like they did in v2; two jest.mock factories needed jest.requireActual("react") to dodge the interop's createElement rewrite; snapshots re-blessed after verifying diffs were value-identical.
Drive-by fix: KebabMenu was passing the class name "font-Lato-Medium" as a fontFamily value (kebab menus have been system-font for a while).
Context for twMerge / classnames
why use twmerge in some cases like @src/components/SharedComponents/ViewWrapper.tsx and classnames() in some places like @src/components/SharedComponents/Typography/Body1.tsx ?
Short answer: twMerge is doing conflict resolution, classnames is just string concatenation — and on this NativeWind 4 branch that difference actually matters.
twMerge = conflict resolution. In NativeWind 4, when one element has two conflicting classes (bg-white bg-black), the winner is decided by stylesheet order, not the order in the className string — so plain concatenation no longer lets a caller's class beat a component's default. twMerge fixes that by actually parsing the Tailwind classes and dropping the earlier one of each conflicting pair, so "later in the string wins" behavior is restored deterministically. That's why it's used in the components that own overridable defaults and render the actual native element: ViewWrapper's ScreenShell (bg-white must lose to a caller's bg-black), InatText, Button.
classnames = cheap concatenation (and conditionals). Body1 doesn't render a Text itself — it just prepends text-base + the font class and hands the combined string to InatText. InatText then runs the entire final string through twMerge before it hits the real . So the conflict resolution still happens exactly once, at the leaf; the intermediate layer only needs to join strings in the right order (caller last). Running twMerge at every layer would be redundant work. classnames also supports the { "some-class": condition } object syntax that ~99 files in the app rely on, which twMerge doesn't do.
Rule of thumb the branch follows: twMerge at the point where a className finally lands on a native element and defaults must be overridable; classnames everywhere upstream of that. The comments in InatText.tsx:24-26 and ViewWrapper.tsx:28-30 document the why.
One thing to keep an eye on: this only stays safe as long as every classnames-composed string eventually flows through a twMerge-ing leaf like InatText. A wrapper that concatenates defaults and passes straight to a raw styled View/Text would silently lose caller overrides — a merge conflict of the kind git can't help you with.