Conversation
|
Warning Review limit reached
More reviews will be available in 29 minutes and 33 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (12)
📒 Files selected for processing (41)
📝 WalkthroughWalkthroughThe PR adds React Native performance and upgrade documentation across two skill trees, plus map screen focus handling that suppresses camera updates while blurred. ChangesForge React Native skills
Opencode React Native skills
Map screen focus guard
Sequence Diagram(s)Skipped. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/maps/map-panel.tsx (1)
237-244:⚠️ Potential issue | 🟠 MajorFix camera-changed typing and gesture detection in
onCameraChanged
- Replace
event: anywith the@rnmapbox/mapsMapStatetype.- Update the condition from
event.properties?.isUserInteractiontostate.gestures.isGestureActive(user interaction isn’t exposed onpropertiesin theonCameraChangedMapState).🤖 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 `@src/components/maps/map-panel.tsx` around lines 237 - 244, In onCameraChanged, replace the loose event: any with the MapState type from `@rnmapbox/maps` (import MapState) and change the interaction check from event.properties?.isUserInteraction to using state.gestures.isGestureActive (e.g., (state: MapState) => { if (state.gestures?.isGestureActive && !isInteractionLocked) { setHasUserMovedMap(true); } }). Keep the useCallback and its dependency on isInteractionLocked unchanged.Source: Coding guidelines
🟠 Major comments (24)
.forge/skills/react-native-best-practices/references/bundle-barrel-exports.md-86-95 (1)
86-95:⚠️ Potential issue | 🟠 MajorFix direct-import syntax to match the barrel’s named exports
In this doc,
components/index.tsuses named exports (export { Button } from './Button'/export { Card } from './Card'), but “Solution 1: Direct Imports” switches to default imports (import Button from .../import Card from ...), which is inconsistent.Suggested fix
-import Button from './components/Button'; -import Card from './components/Card'; +import { Button } from './components/Button'; +import { Card } from './components/Card';🤖 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 @.forge/skills/react-native-best-practices/references/bundle-barrel-exports.md around lines 86 - 95, The direct-import examples in "Solution 1: Direct Imports" are using default-import syntax but the barrel (components/index.ts) exports Button and Card as named exports; change those examples to use named-imports (e.g., import { Button } from './components/Button') so the import syntax matches the barrel's export style, and update any explanatory text to reflect that these are named exports rather than default exports..forge/skills/react-native-best-practices/references/bundle-analyze-js.md-139-148 (1)
139-148:⚠️ Potential issue | 🟠 MajorFix stats-generation workflow (Metro
--jsonflag is invalid for this toolchain).In
bundle-analyze-js.md(lines 139-148),npx react-native bundle ... --json stats.jsonis not supported by the standard React Native CLI, so it can’t generate the Webpack/Rspack stats file. Generate the stats JSON via the Re.Pack/Webpack build (toolchain-specific), then pass the resulting stats file tonpx bundle-statsas its positional input (its--jsonflag controls report output format, not creatingstats.json).🤖 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 @.forge/skills/react-native-best-practices/references/bundle-analyze-js.md around lines 139 - 148, The current instructions use the React Native CLI flag "--json stats.json" which Metro/standard react-native bundle does not support; update the workflow in bundle-analyze-js.md so you remove the unsupported "--json stats.json" usage and instead generate the stats JSON with the toolchain that produces Webpack/Re.Pack stats (run your Re.Pack/Webpack build to emit its stats.json), then invoke bundle-stats by passing that generated stats file as the positional input (e.g., npx bundle-stats path/to/stats.json --html [--json] to control report output format), replacing the erroneous react-native bundle --json step..forge/skills/react-native-best-practices/references/js-animations-reanimated.md-118-123 (1)
118-123:⚠️ Potential issue | 🟠 MajorDon’t schedule JS from
useAnimatedStyle(.forge/skills/react-native-best-practices/references/js-animations-reanimated.mdlines 118-123):useAnimatedStylere-runs every frame, soif (progress.value === 1) scheduleOnRN(trackAnalytics, progress.value)can fire repeatedly. Move the side effect to awithTimingcompletion callback or auseAnimatedReactionand guard it to run once (e.g., via adidTrackshared value).🤖 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 @.forge/skills/react-native-best-practices/references/js-animations-reanimated.md around lines 118 - 123, The current useAnimatedStyle callback calls scheduleOnRN(trackAnalytics, progress.value) when progress.value === 1 but useAnimatedStyle runs every frame so that can fire repeatedly; move the side-effect out of useAnimatedStyle and into either the withTiming completion callback that drives progress or a useAnimatedReaction watching progress, and guard it with a shared boolean (e.g., didTrack) so scheduleOnRN(trackAnalytics, progress.value) is invoked only once; locate the progress/shared values and replace the call inside useAnimatedStyle with just return { opacity: progress.value } while implementing the single-run tracking logic in the withTiming onEnd or in useAnimatedReaction..forge/skills/react-native-best-practices/references/js-concurrent-react.md-112-119 (1)
112-119:⚠️ Potential issue | 🟠 MajorDefer the expensive work instead of calling
computeExpensiveData()insidestartTransition. In the Pattern 3 example,setHeavyData(computeExpensiveData())invokescomputeExpensiveData()synchronously when the transition scope runs, so it can still block before React applies the low-priority update. Use a lazy updater (e.g.,setHeavyData(() => computeExpensiveData())) or make the computation async.🤖 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 @.forge/skills/react-native-best-practices/references/js-concurrent-react.md around lines 112 - 119, The Pattern 3 example calls computeExpensiveData() synchronously inside startTransition which still blocks; update handleIncrement to defer the work by passing a lazy updater to setHeavyData (e.g., setHeavyData(() => computeExpensiveData())) or convert computeExpensiveData to an async function and await/handle its result outside the transition so the heavy computation runs asynchronously and the startTransition scope only schedules the low-priority state update..forge/skills/react-native-best-practices/references/bundle-r8-android.md-13-25 (1)
13-25:⚠️ Potential issue | 🟠 MajorWire
enableProguardInReleaseBuildsinto the release block (and fix the Step 1 wording).
enableProguardInReleaseBuildsis declared but never used;minifyEnabled/shrinkResourcesare hardcodedtrue.- The Step 1 text (“This sets
minifyEnabled = truefor release builds”) isn’t accurate with the current snippets.Either set
minifyEnabled enableProguardInReleaseBuilds(andshrinkResources enableProguardInReleaseBuilds) or remove the unused flag and keep the explicitminifyEnabled/shrinkResourcesvalues.🤖 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 @.forge/skills/react-native-best-practices/references/bundle-r8-android.md around lines 13 - 25, The variable enableProguardInReleaseBuilds is declared but never used; update the release build block in android/app/build.gradle to either use that flag (set minifyEnabled enableProguardInReleaseBuilds and shrinkResources enableProguardInReleaseBuilds) or remove the unused enableProguardInReleaseBuilds constant and keep explicit true/false values, and also update the Step 1 wording to accurately reflect the chosen approach (e.g., if using the flag, say “This sets minifyEnabled/shrinkResources based on enableProguardInReleaseBuilds”; if removing it, state that minifyEnabled/shrinkResources are explicitly enabled for release)..forge/skills/react-native-best-practices/references/native-android-16kb-alignment.md-62-68 (1)
62-68:⚠️ Potential issue | 🟠 Major | ⚡ Quick winScope this check to the artifact type you can actually validate.
zipalignonly validates APKs, so saying “APK or AAB” here overstates what this command covers. Either narrow the step to APKs or add the separate bundle/APK verification flow for AAB-based releases.🤖 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 @.forge/skills/react-native-best-practices/references/native-android-16kb-alignment.md around lines 62 - 68, Update the verification step to scope it correctly: change the text around "Run `zipalign` verification" so it only refers to APKs (e.g., "Run `zipalign` verification on the APK") or add a second, explicit flow describing how to validate AAB releases (build the AAB, generate the APKs with bundletool, then run `zipalign` or equivalent on the generated APKs); ensure the phrase "APK or AAB" is removed or replaced with clear, separate instructions for APK vs AAB verification so readers know `zipalign` applies to APKs only..forge/skills/react-native-best-practices/references/native-measure-tti.md-17-24 (1)
17-24:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMark TTI when the screen is actually interactive, not on mount.
useEffectfires after the first commit, so this example can under-report TTI for screens that still wait on data, transitions, or layout work. Use the real interactive signal for the screen instead of a generic mount effect.Also applies to: 168-180
🤖 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 @.forge/skills/react-native-best-practices/references/native-measure-tti.md around lines 17 - 24, The example marks TTI on mount using useEffect which can under-report TTI; change the implementation to call performance.mark('screenInteractive') from the actual interactive signal instead of the generic useEffect — for example invoke it after your data-ready callback (e.g., onDataLoaded/onSuccess), after layout completion (e.g., onLayout/onLayoutComplete), after navigation focus/transition completes, or wrap in InteractionManager.runAfterInteractions; update references to the existing useEffect example and replace it with calling performance.mark('screenInteractive') from the real readiness handlers (data fetch success handler, layout callback, transition end, or InteractionManager completion) so the mark reflects true interactivity..forge/skills/react-native-best-practices/references/js-memory-leaks.md-122-165 (1)
122-165:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake the example snippets copy/paste safe.
The timer sample references
setCountwithout declaring it, and the closure example uses TypeScriptprivatesyntax inside ajsxfence. As written, both snippets fail if copied into a React Native app.🤖 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 @.forge/skills/react-native-best-practices/references/js-memory-leaks.md around lines 122 - 165, The code examples are not copy/paste safe: update both timer examples (BadTimerComponent and GoodTimerComponent) to declare React state (e.g., add a useState hook for setCount) and import/use React properly so setCount is defined, and change the closure examples (BadClosureExample and GoodClosureExample) to plain JavaScript/valid React-friendly syntax (remove TypeScript-only `private` keyword or annotate that they're class fields) so largeData is a valid field; ensure createLeakyFunction/createEfficientFunction reference the correctly named field (largeData) and that the "GOOD" version extracts the primitive length into a local const before returning the closure..forge/skills/react-native-best-practices/references/native-view-flattening.md-24-29 (1)
24-29:⚠️ Potential issue | 🟠 MajorFix the wrapper example so
{...props}can’t overridecollapsable={false}.
In.forge/skills/react-native-best-practices/references/native-view-flattening.md, theNativeChildWrappersnippet spreads...propsaftercollapsable={false}(<View collapsable={false} {...props}>), so callers passingcollapsablevia props can override it—move{...props}beforecollapsable={false}or stripcollapsablefromprops.🤖 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 @.forge/skills/react-native-best-practices/references/native-view-flattening.md around lines 24 - 29, The NativeChildWrapper currently spreads props after collapsable={false} so callers can override it; update the wrapper (NativeChildWrapper) to either spread {...props} before specifying collapsable={false} or explicitly remove collapsable from props (e.g., const { collapsable, ...rest } = props) and then render <View {...rest} collapsable={false}> so collapsable cannot be overridden by consumers such as NativeTabBar / Tab1 / Tab2..forge/skills/react-native-best-practices/references/native-platform-setup.md-68-80 (1)
68-80:⚠️ Potential issue | 🟠 MajorKeep all Android Gradle commands in the same working directory.
In
.forge/skills/react-native-best-practices/references/native-platform-setup.md(lines 68-80), onlycd android && ./gradlew cleanis anchored;./gradlew tasksand./gradlew assembleReleasewill fail if copied/executed from the repo root.🤖 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 @.forge/skills/react-native-best-practices/references/native-platform-setup.md around lines 68 - 80, The Android Gradle commands in native-platform-setup.md are inconsistent: only the clean command is run from the android directory (cd android && ./gradlew clean) while ./gradlew tasks and ./gradlew assembleRelease will fail if executed from the repo root; update the document so all Android gradle commands are executed from the android working directory—either prefix ./gradlew tasks and ./gradlew assembleRelease with cd android && or add a single instruction to change into the android directory before running any ./gradlew command (referencing the existing "cd android && ./gradlew clean", "./gradlew tasks", and "./gradlew assembleRelease" lines)..forge/skills/react-native-best-practices/references/native-turbo-modules.md-199-216 (1)
199-216:⚠️ Potential issue | 🟠 MajorComplete the C++/Obj-C++ registration example before publishing it.
- The ObjC++ snippet uses
@implementation MyCppModuleRegistrationwithout showing an@interface(or superclass declaration) forMyCppModuleRegistration, so it won’t compile as written.- The registration references
facebook::react::MyCppModule::kModuleName, butkModuleNameisn’t declared in the provided example content, so it’s undefined.🤖 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 @.forge/skills/react-native-best-practices/references/native-turbo-modules.md around lines 199 - 216, The ObjC++ registration example is incomplete and won't compile: add a minimal Objective-C interface for MyCppModuleRegistration (e.g., `@interface` MyCppModuleRegistration : NSObject `@end`) or declare an appropriate superclass so the `@implementation` compiles, and ensure the C++ module exports a named module identifier by declaring kModuleName in the MyCppModule class/header (or reference the correct constant) and include the MyCppModule header before calling facebook::react::MyCppModule::kModuleName in registerCxxModuleToGlobalModuleMap so the symbol is defined and visible to the ObjC++ file..forge/skills/upgrading-react-native/references/expo-sdk-upgrade.md-28-29 (1)
28-29:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t blanket-skip
app.jsonedits.Expo bare/prebuild projects can still require
app.json/app.config.jschanges, so this instruction is too broad and may drop required config updates.♻️ Proposed fix
-- Important for this workflow: skip `app.json` changes, because this is not an Expo Managed project. +- Important for this workflow: only skip managed-app `app.json` changes; keep `app.json`/`app.config.js` edits when Expo config is part of the project.🤖 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 @.forge/skills/upgrading-react-native/references/expo-sdk-upgrade.md around lines 28 - 29, Replace the blanket instruction "skip `app.json` changes, because this is not an Expo Managed project" with a conditional guidance that tells the maintainer to detect whether the repo uses Expo Managed vs Bare/Prebuild (or whether `app.json` / `app.config.js` exists) and only skip edits when the project truly does not use/expose those config files; update the sentence in .forge/skills/upgrading-react-native/references/expo-sdk-upgrade.md (the line containing "skip `app.json` changes") to: instruct reviewers to check for presence of `app.json`/`app.config.js` and, if present or if the project is an Expo prebuild that requires config changes, apply the relevant config updates rather than always skipping them..forge/skills/upgrading-react-native/references/upgrade-helper-core.md-67-69 (1)
67-69:⚠️ Potential issue | 🟠 Major | ⚡ Quick winScope the install to
APP_DIR.Running the install from the repo root contradicts the monorepo-targeting rules and can regenerate the wrong lockfile when the app lives in a subpackage.
♻️ Proposed fix
-8. Apply dependency updates in one pass. - - Update `APP_DIR/package.json` (and lockfile) from the baseline plus approved migrations. - - Run exactly one install command with the repo's package manager (`npm install`, `yarn install`, `pnpm install`, or `bun install`). +8. Apply dependency updates in one pass. + - Update `APP_DIR/package.json` (and lockfile) from the baseline plus approved migrations. + - Run exactly one install command from `APP_DIR` with the repo's package manager (`cd "$APP_DIR" && npm install`, etc.).🤖 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 @.forge/skills/upgrading-react-native/references/upgrade-helper-core.md around lines 67 - 69, Update the guidance to explicitly run the install command from inside APP_DIR (not the repo root) so the install is scoped to the app package and its lockfile is updated correctly; change the bullets that mention "Run exactly one install command with the repo's package manager" and "Avoid piecemeal installs" to specify executing the package manager command (npm/yarn/pnpm/bun) within APP_DIR and warn against running installs from the repo root to prevent regenerating the wrong lockfile..opencode/skills/react-native-best-practices/references/js-concurrent-react.md-112-120 (1)
112-120:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
startTransitiondoesn't defer the expensive computation itself.
computeExpensiveData()still runs synchronously inside the transition callback, so input can stay blocked even though the state update is lower priority. Move the expensive work out of the transition or into a memoized/deferred render path.🤖 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 @.opencode/skills/react-native-best-practices/references/js-concurrent-react.md around lines 112 - 120, The transition callback currently calls computeExpensiveData() synchronously, so move the expensive computation out of startTransition and only call setHeavyData inside it; specifically, computeExpensiveData() before invoking startTransition in handleIncrement (or make it async/memoized or use a deferred value like useDeferredValue/useMemo) and pass the already-computed result into startTransition which then calls setHeavyData(result) to ensure the expensive work doesn't block the transition..opencode/skills/react-native-best-practices/references/bundle-r8-android.md-15-16 (1)
15-16:⚠️ Potential issue | 🟠 MajorFix Gradle wiring for
enableProguardInReleaseBuilds→minifyEnabledThe doc defines
def enableProguardInReleaseBuilds = true(lines 15-16 / 49-50) but never uses that variable in the Gradle snippets—minifyEnabled trueis set directly (lines 20-22 and 62-63). Also, the text claims “This setsminifyEnabled = truefor release builds” (line 52), which is misleading as written.Update the snippet to either remove the unused variable or wire it explicitly (e.g.,
minifyEnabled enableProguardInReleaseBuilds).🤖 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 @.opencode/skills/react-native-best-practices/references/bundle-r8-android.md around lines 15 - 16, The snippet defines the variable enableProguardInReleaseBuilds but doesn't use it; update the Gradle snippets to wire that variable into minifyEnabled (replace hard-coded minifyEnabled true/false with minifyEnabled enableProguardInReleaseBuilds or similar) OR remove the unused def enableProguardInReleaseBuilds altogether and adjust the explanatory text so it accurately reflects whether minifyEnabled is being set directly; ensure references to enableProguardInReleaseBuilds and minifyEnabled in the doc/snippets are consistent..opencode/skills/react-native-best-practices/references/js-react-compiler.md-257-285 (1)
257-285:⚠️ Potential issue | 🟠 MajorFix the
onChangeTextexample.
onChangeText={() => setValue(value)}writes the currentvalueback into state instead of using TextInput’s updated text argument; change it to something likeonChangeText={(text) => setValue(text)}and regenerate the “After (compiled output)” snippet to match.🤖 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 @.opencode/skills/react-native-best-practices/references/js-react-compiler.md around lines 257 - 285, Update the example in MyApp so onChangeText uses the new text argument instead of re-setting the old value: change the event handler from onChangeText={() => setValue(value)} to onChangeText={(text) => setValue(text)} and then regenerate the compiled output snippet (the transformed function using the $ cache, t0, and $[0]/$[1] variables) so it reflects the correct handler reference and cached JSX for the TextInput element..opencode/skills/react-native-best-practices/references/native-android-16kb-alignment.md-62-66 (1)
62-66:⚠️ Potential issue | 🟠 MajorFix the release-flow wording:
zipalignonly applies to APKs, not AABs.
Innative-android-16kb-alignment.md(Step 2 after “Build your release APK or AAB”), this can incorrectly implyzipalignvalidates an AAB. Scope the instruction to APK builds, or add an explicit AAB path (e.g., usebundletoolto generate APKs from the AAB, then runzipalignon those APKs).🤖 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 @.opencode/skills/react-native-best-practices/references/native-android-16kb-alignment.md around lines 62 - 66, Update the wording in Step 2 ("Run `zipalign` verification (see Quick Command)") to explicitly state that `zipalign` applies only to APKs and not AABs, and provide an alternative AAB path: instruct readers to use bundletool to generate APKs from the AAB (e.g., "bundletool build-apks" / "bundletool extract-apks") and then run `zipalign` on those generated APKs; keep the rest of the flow (trace misaligned libraries and update dependencies) unchanged..opencode/skills/react-native-best-practices/references/js-measure-fps.md-98-110 (1)
98-110:⚠️ Potential issue | 🟠 MajoriOS FPS example isn’t actually running a production build
npx react-native start --reset-cachejust clears Metro’s cache and starts the Metro dev server; it doesn’t switch the app/runtime to a Release/prod configuration, so the FPS results will still reflect dev-mode overhead. The snippet also stops short of providing an iOS Release run command (e.g.,npx react-native run-ios --mode release) despite claiming “Then build release variant”.🤖 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 @.opencode/skills/react-native-best-practices/references/js-measure-fps.md around lines 98 - 110, The iOS RN CLI snippet currently suggests running "npx react-native start --reset-cache" which only starts Metro in dev mode; replace that with instructions to build and run a Release app (e.g., run the release build command such as "npx react-native run-ios --configuration Release" or build/archive via Xcode) and note that you should not rely on the Metro dev server for FPS measurement, so run the Release binary directly; likewise update the Expo section to recommend using EAS Build (e.g., "eas build" with a production/release profile and installing that build on a device or simulator) instead of just "npx expo start --no-dev --minify" for accurate FPS results..opencode/skills/react-native-best-practices/references/native-turbo-modules.md-189-213 (1)
189-213:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDefine the module name constant before registering the C++ module.
MyCppModule::kModuleNameis referenced in the registration snippet but never declared in the class, so the example won't compile as written.🤖 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 @.opencode/skills/react-native-best-practices/references/native-turbo-modules.md around lines 189 - 213, The registration references MyCppModule::kModuleName but the class MyCppModule does not declare it; add a module name constant (e.g., a public static constexpr char kModuleName[] = "MyCppModule") to the MyCppModule class declaration so the call to registerCxxModuleToGlobalModuleMap can use MyCppModule::kModuleName, then rebuild and ensure the string matches the registered JS/native name used elsewhere..opencode/skills/react-native-best-practices/references/native-sdks-over-polyfills.md-57-90 (1)
57-90:⚠️ Potential issue | 🟠 MajorFix Hermes Intl support matrix for
Intl.supportedValuesOf().
In the table (lines 57-71),Intl.supportedValuesOf()is marked ✅, but Hermes Intl support sources indicate it’s not implemented (may be undefined/crash). Update the matrix + “Keep Polyfill?” guidance accordingly. TheIntl.NumberFormat.prototype.formatToParts()iOS gap and@formatjs/intl-numberformatadvice align with Hermes limitations, but still validate against the specific Hermes/RN versions you target.🤖 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 @.opencode/skills/react-native-best-practices/references/native-sdks-over-polyfills.md around lines 57 - 90, Update the Hermes Intl support matrix to reflect that Intl.supportedValuesOf() is not implemented: change the table row for `Intl.supportedValuesOf()` from ✅ to ❌ and set the "Keep Polyfill?" column to "Yes"; also update any surrounding guidance text that currently claims it’s supported so it instructs readers to include the appropriate polyfill (same treatment as other missing Intl features like `Intl.Locale`/`Intl.PluralRules`). Ensure the import examples and the sentence about validating against specific Hermes/RN versions remain accurate after this change..opencode/skills/react-native-best-practices/references/native-threading-model.md-13-21 (1)
13-21:⚠️ Potential issue | 🟠 MajorFix Yoga/shadow-tree threading claims (Fabric vs legacy)
native-threading-model.mdstates “Yoga layout | JS | JS” (Quick Reference) and “Layout (Yoga) | JS thread” / the “Yoga Layout” section says “JS Thread: Calculate Yoga tree → Shadow tree”, but Fabric’s Yoga/shadow-tree layout is handled in the native/C++ render pipeline (commit phase) rather than the JS thread—update the wording/table to reflect Fabric correctly or scope this content to the legacy renderer (also update the Summary Table row at lines ~219-229).- The fenced block under “Yoga Layout” (opening fence at line ~139) is missing a language tag.
🤖 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 @.opencode/skills/react-native-best-practices/references/native-threading-model.md around lines 13 - 21, Update the Quick Reference table row "Yoga layout" and the "Yoga Layout" section to distinguish legacy renderer vs Fabric: change the table/phrasing so Fabric is shown as native/C++ (commit phase) handling Yoga/shadow-tree layout while the JS thread path applies only to the legacy renderer or explicitly scope statements to "legacy renderer"; in the "Yoga Layout" section update the sentence that currently reads "JS Thread: Calculate Yoga tree → Shadow tree" to note Fabric's native/C++ render pipeline does this during the commit phase and keep the legacy JS flow as a separate note; also add a language tag to the fenced code block that opens around the Yoga Layout example so the block has an appropriate syntax highlighting marker..opencode/skills/react-native-best-practices/references/native-turbo-modules.md-83-98 (1)
83-98:⚠️ Potential issue | 🟠 MajorFix missing
MyCppModule::kModuleNamein the C++ TurboModule registration snippet.
The guide’s registration code referencesfacebook::react::MyCppModule::kModuleName, but the shownMyCppModuledefinition doesn’t declare that constant, so the snippet won’t compile if pasted. DefinekModuleName(or update the registration snippet to use the declared module name) in theMyCppModulesnippet.🤖 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 @.opencode/skills/react-native-best-practices/references/native-turbo-modules.md around lines 83 - 98, The C++ TurboModule snippet refers to facebook::react::MyCppModule::kModuleName but MyCppModule does not declare that constant; add a public static constexpr char kModuleName[] (or equivalent static const std::string) to the MyCppModule class definition with the module name used in the registration, or alternatively update the registration snippet to use the actual name provided by MyCppModule; ensure the symbol is named kModuleName on MyCppModule so the registration call facebook::react::MyCppModule::kModuleName compiles..opencode/skills/upgrading-react-native/references/expo-sdk-upgrade.md-28-28 (1)
28-28:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't blanket-skip Expo app config changes.
Bare Expo projects can still need
app.config.*/app.jsonreconciliation, so this should be conditional rather than an unconditional skip.♻️ Proposed fix
-- Important for this workflow: skip `app.json` changes, because this is not an Expo Managed project. +- Important for this workflow: skip `app.json` changes only when the project truly has no Expo config layer; otherwise reconcile Expo config changes per the target workflow.🤖 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 @.opencode/skills/upgrading-react-native/references/expo-sdk-upgrade.md at line 28, The line "Important for this workflow: skip `app.json` changes, because this is not an Expo Managed project." should be made conditional instead of unconditional: update the guidance to instruct maintainers to only skip app config reconciliation when the repository is truly a non-Expo-managed project (detect by absence of app.json or app.config.* and by project type), and to perform `app.json` / `app.config.*` reconciliation when those files exist in a bare project; replace the absolute "skip `app.json` changes" sentence with a conditional note referencing the original sentence text so readers check for app.json or app.config.* and reconcile if present..opencode/skills/upgrading-react-native/references/upgrading-dependencies.md-31-32 (1)
31-32:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGate the symlink-resolver cleanup on the target RN stack.
@rnx-kit/metro-resolver-symlinksshould only be removed once the target Metro version actually supports native symlinks; otherwise this rule can break older upgrade paths.♻️ Proposed fix
-- If `@rnx-kit/metro-resolver-symlinks` is present, remove it from deps and `metro.config.js` (Metro supports symlinks since 0.72) +- If `@rnx-kit/metro-resolver-symlinks` is present and the target RN/Metro stack supports native symlinks, remove it from deps and `metro.config.js`🤖 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 @.opencode/skills/upgrading-react-native/references/upgrading-dependencies.md around lines 31 - 32, The guidance to remove `@rnx-kit/metro-resolver-symlinks` must be gated by the target Metro/RN stack: update the doc so the step only applies when the upgrade target includes Metro >= 0.72 (or RN >= 0.72 that bundles that Metro), e.g., change the line mentioning removal from deps and metro.config.js to state "remove only if targeting Metro 0.72+ (or equivalent RN release)"; leave the package in place for older targets and add a short check/decision note so maintainers know to verify the target Metro version before deleting `@rnx-kit/metro-resolver-symlinks` from dependencies or metro.config.js.
🧹 Nitpick comments (3)
.opencode/skills/react-native-best-practices/POWER.md (1)
97-102: ⚡ Quick winKeep the state-management guidance consistent across both docs.
Both
POWER.mdandSKILL.mdstill recommendJotai/Zustand, but the repo rule isUse zustand for state management.Remove Jotai from both places so the skill docs match the codebase standard.As per coding guidelines, "Use zustand for state management."
🤖 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 @.opencode/skills/react-native-best-practices/POWER.md around lines 97 - 102, Update the state-management guidance in the docs to match the repo rule by removing Jotai everywhere it is referenced: change the bullet under the "Common fixes" (the line currently reading "Use atomic state (Jotai/Zustand) to reduce re-renders") to reference only zustand (e.g., "Use zustand for state management" or "Use atomic state (zustand) to reduce re-renders"), and make the same change in SKILL.md so both POWER.md and SKILL.md consistently recommend only zustand.Source: Coding guidelines
.opencode/skills/react-native-best-practices/references/bundle-code-splitting.md (1)
143-157: ⚡ Quick winMatch the example to the repo's JSX conventions.
The sample uses an inline event handler and
&&rendering, both of which conflict with the repo guidance. A named callback and a ternary keep the example aligned with the rest of the codebase.As per coding guidelines, "Avoid anonymous functions in renderItem or event handlers" and "Use conditional rendering with the ternary operator
? :and not&&."🤖 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 @.opencode/skills/react-native-best-practices/references/bundle-code-splitting.md around lines 143 - 157, The App component uses an inline event handler and && conditional rendering; update it to match repo JSX conventions by creating a named callback (e.g., handleLoadFeature) that calls setShowFeature(true) and replacing the "{showFeature && (...)}" with a ternary expression "{showFeature ? (<Suspense ...><HeavyFeature /></Suspense>) : null}". Ensure the Button's onPress references the named callback (handleLoadFeature) and keep Suspense and HeavyFeature unchanged.Source: Coding guidelines
.opencode/skills/react-native-best-practices/references/bundle-native-assets.md (1)
176-182: ⚡ Quick winClarify that FastImage is a runtime optimization, not a bundle-size optimization.
FastImage helps caching/decoding, but it doesn't reduce the shipped asset payload or store download size. In this section it reads like a size win, which is misleading.
🤖 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 @.opencode/skills/react-native-best-practices/references/bundle-native-assets.md around lines 176 - 182, Update the "3. Consider react-native-fast-image" section to clarify that react-native-fast-image is a runtime performance/cache/decoding optimization and does not reduce the shipped asset payload or app download size; modify the explanatory sentence under that heading (next to the npm install snippet) to explicitly state this distinction and warn readers not to expect bundle-size reductions from installing FastImage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bf69dae1-86b6-4aec-b1a0-776af92bd4bd
⛔ Files ignored due to path filters (24)
.forge/skills/react-native-best-practices/references/images/bundle-treemap-source-map-explorer.pngis excluded by!**/*.png.forge/skills/react-native-best-practices/references/images/controlled-textinput-pingpong.pngis excluded by!**/*.png.forge/skills/react-native-best-practices/references/images/devtools-flamegraph.pngis excluded by!**/*.png.forge/skills/react-native-best-practices/references/images/emerge-xray-ios.pngis excluded by!**/*.png.forge/skills/react-native-best-practices/references/images/expo-atlas-treemap.pngis excluded by!**/*.png.forge/skills/react-native-best-practices/references/images/flashlight-flatlist-vs-flashlist.pngis excluded by!**/*.png.forge/skills/react-native-best-practices/references/images/fps-drop-graph.pngis excluded by!**/*.png.forge/skills/react-native-best-practices/references/images/memory-heap-snapshot.pngis excluded by!**/*.png.forge/skills/react-native-best-practices/references/images/tti-warm-start-diagram.pngis excluded by!**/*.png.forge/skills/react-native-best-practices/references/images/view-hierarchy-flattening.pngis excluded by!**/*.png.forge/skills/react-native-best-practices/references/images/xcode-instruments-templates.pngis excluded by!**/*.png.forge/skills/react-native-best-practices/references/images/xcode-thread-view.pngis excluded by!**/*.png.opencode/skills/react-native-best-practices/references/images/bundle-treemap-source-map-explorer.pngis excluded by!**/*.png.opencode/skills/react-native-best-practices/references/images/controlled-textinput-pingpong.pngis excluded by!**/*.png.opencode/skills/react-native-best-practices/references/images/devtools-flamegraph.pngis excluded by!**/*.png.opencode/skills/react-native-best-practices/references/images/emerge-xray-ios.pngis excluded by!**/*.png.opencode/skills/react-native-best-practices/references/images/expo-atlas-treemap.pngis excluded by!**/*.png.opencode/skills/react-native-best-practices/references/images/flashlight-flatlist-vs-flashlist.pngis excluded by!**/*.png.opencode/skills/react-native-best-practices/references/images/fps-drop-graph.pngis excluded by!**/*.png.opencode/skills/react-native-best-practices/references/images/memory-heap-snapshot.pngis excluded by!**/*.png.opencode/skills/react-native-best-practices/references/images/tti-warm-start-diagram.pngis excluded by!**/*.png.opencode/skills/react-native-best-practices/references/images/view-hierarchy-flattening.pngis excluded by!**/*.png.opencode/skills/react-native-best-practices/references/images/xcode-instruments-templates.pngis excluded by!**/*.png.opencode/skills/react-native-best-practices/references/images/xcode-thread-view.pngis excluded by!**/*.png
📒 Files selected for processing (84)
.forge/skills/react-native-best-practices/POWER.md.forge/skills/react-native-best-practices/SKILL.md.forge/skills/react-native-best-practices/agents/openai.yaml.forge/skills/react-native-best-practices/references/bundle-analyze-app.md.forge/skills/react-native-best-practices/references/bundle-analyze-js.md.forge/skills/react-native-best-practices/references/bundle-barrel-exports.md.forge/skills/react-native-best-practices/references/bundle-code-splitting.md.forge/skills/react-native-best-practices/references/bundle-hermes-mmap.md.forge/skills/react-native-best-practices/references/bundle-library-size.md.forge/skills/react-native-best-practices/references/bundle-native-assets.md.forge/skills/react-native-best-practices/references/bundle-r8-android.md.forge/skills/react-native-best-practices/references/bundle-tree-shaking.md.forge/skills/react-native-best-practices/references/js-animations-reanimated.md.forge/skills/react-native-best-practices/references/js-atomic-state.md.forge/skills/react-native-best-practices/references/js-bottomsheet.md.forge/skills/react-native-best-practices/references/js-concurrent-react.md.forge/skills/react-native-best-practices/references/js-lists-flatlist-flashlist.md.forge/skills/react-native-best-practices/references/js-measure-fps.md.forge/skills/react-native-best-practices/references/js-memory-leaks.md.forge/skills/react-native-best-practices/references/js-profile-react.md.forge/skills/react-native-best-practices/references/js-react-compiler.md.forge/skills/react-native-best-practices/references/js-uncontrolled-components.md.forge/skills/react-native-best-practices/references/native-android-16kb-alignment.md.forge/skills/react-native-best-practices/references/native-measure-tti.md.forge/skills/react-native-best-practices/references/native-memory-leaks.md.forge/skills/react-native-best-practices/references/native-memory-patterns.md.forge/skills/react-native-best-practices/references/native-platform-setup.md.forge/skills/react-native-best-practices/references/native-profiling.md.forge/skills/react-native-best-practices/references/native-sdks-over-polyfills.md.forge/skills/react-native-best-practices/references/native-threading-model.md.forge/skills/react-native-best-practices/references/native-turbo-modules.md.forge/skills/react-native-best-practices/references/native-view-flattening.md.forge/skills/upgrading-react-native/SKILL.md.forge/skills/upgrading-react-native/agents/openai.yaml.forge/skills/upgrading-react-native/references/expo-sdk-upgrade.md.forge/skills/upgrading-react-native/references/monorepo-singlerepo-targeting.md.forge/skills/upgrading-react-native/references/react.md.forge/skills/upgrading-react-native/references/upgrade-helper-core.md.forge/skills/upgrading-react-native/references/upgrade-verification.md.forge/skills/upgrading-react-native/references/upgrading-dependencies.md.forge/skills/upgrading-react-native/references/upgrading-react-native.md.gitignore.opencode/skills/react-native-best-practices/POWER.md.opencode/skills/react-native-best-practices/SKILL.md.opencode/skills/react-native-best-practices/agents/openai.yaml.opencode/skills/react-native-best-practices/references/bundle-analyze-app.md.opencode/skills/react-native-best-practices/references/bundle-analyze-js.md.opencode/skills/react-native-best-practices/references/bundle-barrel-exports.md.opencode/skills/react-native-best-practices/references/bundle-code-splitting.md.opencode/skills/react-native-best-practices/references/bundle-hermes-mmap.md.opencode/skills/react-native-best-practices/references/bundle-library-size.md.opencode/skills/react-native-best-practices/references/bundle-native-assets.md.opencode/skills/react-native-best-practices/references/bundle-r8-android.md.opencode/skills/react-native-best-practices/references/bundle-tree-shaking.md.opencode/skills/react-native-best-practices/references/js-animations-reanimated.md.opencode/skills/react-native-best-practices/references/js-atomic-state.md.opencode/skills/react-native-best-practices/references/js-bottomsheet.md.opencode/skills/react-native-best-practices/references/js-concurrent-react.md.opencode/skills/react-native-best-practices/references/js-lists-flatlist-flashlist.md.opencode/skills/react-native-best-practices/references/js-measure-fps.md.opencode/skills/react-native-best-practices/references/js-memory-leaks.md.opencode/skills/react-native-best-practices/references/js-profile-react.md.opencode/skills/react-native-best-practices/references/js-react-compiler.md.opencode/skills/react-native-best-practices/references/js-uncontrolled-components.md.opencode/skills/react-native-best-practices/references/native-android-16kb-alignment.md.opencode/skills/react-native-best-practices/references/native-measure-tti.md.opencode/skills/react-native-best-practices/references/native-memory-leaks.md.opencode/skills/react-native-best-practices/references/native-memory-patterns.md.opencode/skills/react-native-best-practices/references/native-platform-setup.md.opencode/skills/react-native-best-practices/references/native-profiling.md.opencode/skills/react-native-best-practices/references/native-sdks-over-polyfills.md.opencode/skills/react-native-best-practices/references/native-threading-model.md.opencode/skills/react-native-best-practices/references/native-turbo-modules.md.opencode/skills/react-native-best-practices/references/native-view-flattening.md.opencode/skills/upgrading-react-native/SKILL.md.opencode/skills/upgrading-react-native/agents/openai.yaml.opencode/skills/upgrading-react-native/references/expo-sdk-upgrade.md.opencode/skills/upgrading-react-native/references/monorepo-singlerepo-targeting.md.opencode/skills/upgrading-react-native/references/react.md.opencode/skills/upgrading-react-native/references/upgrade-helper-core.md.opencode/skills/upgrading-react-native/references/upgrade-verification.md.opencode/skills/upgrading-react-native/references/upgrading-dependencies.md.opencode/skills/upgrading-react-native/references/upgrading-react-native.mdsrc/components/maps/map-panel.tsx
Member
Author
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
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.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes