fix(metro): pin uniwind to a single copy in withUniwindConfig#564
Closed
ottob wants to merge 1 commit into
Closed
Conversation
In monorepos (bun/pnpm) uniwind can be installed as multiple peer-hash
copies. Mixing them has two failure modes:
1. Hermes crashes at startup with "Maximum call stack size exceeded
(native stack depth)" — a get NativeModules recursion. The
react-native shim redirects require('react-native') to
uniwind/components; when that resolves to a different copy than the
one seeding the isInternal guard, the shim redirects into itself
across copies and never terminates.
2. When it doesn't crash, the two copies split uniwind's theme/CSS
runtime across instances: components render from one copy while the
theme initialized in the other, so styling silently breaks and
colorKit reports "invalid" colors.
Wrap the resolver passed into the platform resolvers so every 'uniwind'
specifier resolves from a single origin (the project root). Wrapping the
inner resolver (not just resolveRequest) is required so the internal
react-native -> uniwind/components redirects are pinned too. This
collapses duplicate copies to one instance, fixing both failure modes.
Refs uni-stack#353
2 tasks
📝 WalkthroughWalkthroughThe Metro adapter is updated to resolve CSS sources as part of the module resolution pipeline and to pin all ChangesMetro CSS and module resolution
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@packages/uniwind/src/bundler/adapters/metro/metro.ts`:
- Around line 30-31: withUniwindConfig is currently spreading
config.resolver?.sourceExts and filtering assetExts which can drop Metro
defaults when resolver fields are missing; update withUniwindConfig to either
(a) obtain Metro defaults (e.g., call getDefaultConfig()/mergeConfig()) and
merge those defaults with config.resolver.sourceExts and
config.resolver.assetExts before appending 'css' and filtering, or (b) add a
runtime assertion that config.resolver.sourceExts and config.resolver.assetExts
are present and throw a clear error if they are not; reference the
withUniwindConfig function and the resolver fields (config.resolver.sourceExts,
config.resolver.assetExts) when implementing the merge or the fail-fast
assertion.
🪄 Autofix (Beta)
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
Run ID: 88d06987-41f4-4349-9f24-c3ec9e69a0a4
📒 Files selected for processing (1)
packages/uniwind/src/bundler/adapters/metro/metro.ts
Contributor
|
With this changes I'm getting a different error inside your repro 
I've just: |
Brentlok
reviewed
Jun 8, 2026
Comment on lines
+1
to
+11
| import { resolve } from 'node:path' | ||
|
|
||
| import { UniwindBundlerConfig } from '@/bundler/config' | ||
| import type { UniwindConfig } from '@/bundler/types' | ||
| import { Platform } from '@/common/consts' | ||
| import type { MetroConfig } from 'metro-config' | ||
|
|
||
| import { cacheStore, patchMetroGraphToSupportUncachedModules } from './patches' | ||
| import { nativeResolver, webResolver } from './resolvers' | ||
|
|
||
| import type { UniwindConfig } from '@/bundler/types' | ||
| import type { MetroConfig } from 'metro-config' | ||
| import type { CustomResolver } from 'metro-resolver' |
Contributor
There was a problem hiding this comment.
It's not really common in this repo to separate imports
Comment on lines
-25
to
+31
| sourceExts: [ | ||
| ...config.resolver?.sourceExts ?? [], | ||
| 'css', | ||
| ], | ||
| assetExts: config.resolver?.assetExts?.filter( | ||
| ext => ext !== 'css', | ||
| ), | ||
| sourceExts: [...(config.resolver?.sourceExts ?? []), 'css'], | ||
| assetExts: config.resolver?.assetExts?.filter((ext) => ext !== 'css'), |
Comment on lines
+34
to
+43
| // Pin every `uniwind` specifier to a single physical copy. In monorepos | ||
| // (bun/pnpm) uniwind can be installed as multiple peer-hash copies; mixing | ||
| // them both crashes Hermes ("Maximum call stack size exceeded" via | ||
| // get NativeModules recursion, when the react-native shim redirects across | ||
| // copies) and splits uniwind's theme runtime across instances (no styling, | ||
| // colorKit "invalid" color errors). Resolving every `uniwind` request from | ||
| // a single origin (the project root) collapses them to one instance. | ||
| // Wrapping `resolver` — not just this resolveRequest — ensures the platform | ||
| // resolvers' internal `react-native` -> `uniwind/components` redirects are | ||
| // pinned too. |
Contributor
There was a problem hiding this comment.
This comment seems to be too long
Contributor
|
fixed in #570 |
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
2 participants
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.
Problem
In monorepos using bun or pnpm,
uniwindcan be installed as multiple physical copies (different peer-hash variants in bun's store, or duplicate copies under pnpm). When more than one copy ends up reachable by Metro, there are two failure modes:Hermes crash at startup —
RangeError: Maximum call stack size exceeded (native stack depth), aget NativeModulesrecursion. uniwind's metro plugin rewritesimport 'react-native'→uniwind/components. TheisInternalguard (keyed torequire.resolve('uniwind/package.json')) is meant to let uniwind's ownrequire('react-native')reach real RN — but when Metro resolves a uniwind file to a different copy than the one that seeded the guard,isInternalisfalsefor code that genuinely is internal, so the shim redirects its ownrequire('react-native')back intouniwind/componentsand recurses forever.Silently broken styling — when it doesn't crash outright, the two copies split uniwind's theme/CSS runtime across instances: components render from one copy while the theme initialized in the other. Result is no styling plus
[colorKit.RGB] … "invalid" is not a valid color or brushwarnings.This is install-order dependent (the crash happens only when an app's canonical copy differs from the copy Metro's hierarchical lookup falls back to), which makes it hard to reproduce on demand.
Tracked in #353. Minimal deterministic repro: https://github.com/ottob/uniwind-353-repro (
bun install && bun run diagnoseshows the two copies and predicts the crash).Fix
Pin every
uniwindspecifier to a single physical copy by resolving it from one stable origin (the MetroprojectRoot). The key detail: wrap theresolverthat's passed intonativeResolver/webResolver, not just the outerresolveRequest— the platform resolvers perform the internalreact-native→uniwind/componentsredirect through that innerresolver, and that's exactly where the recursion forms.This collapses duplicate copies to one instance, fixing both the recursion and the split-runtime styling bug. Non-
uniwindspecifiers (includingreact-native) are untouched, so the existing react-native → components shim for app code behaves exactly as before.Notes / scope for discussion
uniwind/uniwind/*specifiers; everything else is passed through unchanged.config.projectRootfromgetDefaultConfigis relative ("."), sopath.resolve(notjoin) is used to produce an absolute origin.isInternalguard to recognize any…/uniwind/(dist|src)/…origin. That stops the crash but not the split-runtime styling bug, because the duplication is in how Metro resolvesimport { Text } from 'uniwind'per-origin — only single-copy resolution fixes both. Hence pinning here.Verification
check:typescript,lint,dprint fmt, andbuildall pass.get NativeModulesrecursion: with this change the app boots and styling renders correctly, with no per-appmetro.config.jsworkaround.Summary by CodeRabbit