feat(cli): pin nitro deps and allow '_' unused vars #339
Conversation
WalkthroughBroad reformatting across configs/workflows and docs. CI workflow was split into multiple jobs. Dependabot gained a “nitro” group (root and template). ESLint adds a no-unused-vars rule. Core feature: generator now resolves and pins nitro-codegen/react-native-nitro-modules versions at generation time via npm/pnpm. Minor code style tweak in NitroSpinner. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer
participant CLI as NitroModuleFactory
participant NPM as npm registry
participant PNPM as pnpm (fallback)
participant FS as Filesystem
Dev->>CLI: generate nitro package
activate CLI
CLI->>NPM: npm view react-native-nitro-modules version
alt npm success
NPM-->>CLI: latest version
else npm fails
CLI->>PNPM: pnpm view react-native-nitro-modules version
PNPM-->>CLI: latest or null
end
CLI->>NPM: npm view nitro-codegen version
alt npm success
NPM-->>CLI: latest version
else npm fails
CLI->>PNPM: pnpm view nitro-codegen version
PNPM-->>CLI: latest or null
end
note over CLI: Store resolved versions (may be null)
CLI->>FS: Write nitro.json (placeholders replaced)
CLI->>FS: Update workspace devDependencies with pinned versions (fallback to existing/template if null)
CLI->>FS: Update example app dependencies with pinned react-native-nitro-modules
deactivate CLI
CLI-->>Dev: Generation complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
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: 5
🧹 Nitpick comments (19)
docs/src/css/custom.css (1)
22-29: Indentation consistency between blocksThe dark theme block uses 4-space indentation while :root uses 2. Consider standardizing for readability.
Apply this diff to match the 2-space indentation style:
[data-theme='dark'] { - --ifm-color-primary: #25c2a0; - --ifm-color-primary-dark: #21af90; - --ifm-color-primary-darker: #1fa588; - --ifm-color-primary-darkest: #1a8870; - --ifm-color-primary-light: #29d5b0; - --ifm-color-primary-lighter: #32d8b4; - --ifm-color-primary-lightest: #4fddbf; - --docusaurus-highlighted-code-line-bg: rgba(0, 0, 0, 0.3); + --ifm-color-primary: #25c2a0; + --ifm-color-primary-dark: #21af90; + --ifm-color-primary-darker: #1fa588; + --ifm-color-primary-darkest: #1a8870; + --ifm-color-primary-light: #29d5b0; + --ifm-color-primary-lighter: #32d8b4; + --ifm-color-primary-lightest: #4fddbf; + --docusaurus-highlighted-code-line-bg: rgba(0, 0, 0, 0.3); }src/nitro-spinner.ts (1)
18-18: Duplicate initialization oftext
textis initialized here and again in the constructor (Line 51). Drop one to avoid churn.- this.text = ''.github/actions/install-deps/action.yml (1)
15-21: Use frozen/immutable installs per package manager for deterministic CIAligns with the PR goal (deterministic installs). Prefer lockfile-respecting commands instead of a generic “install”.
Apply this diff:
set -euo pipefail cd "${{ inputs.working-directory }}" case "${{ inputs.pm }}" in bun|yarn|pnpm|npm) ;; *) echo "Unsupported pm: ${{ inputs.pm }}" >&2; exit 1;; esac - ${{ inputs.pm }} install + case "${{ inputs.pm }}" in + bun) + # bun respects --frozen-lockfile; use if lockfile exists, otherwise still freeze + if [ -f bun.lockb ]; then bun install --frozen-lockfile; else bun install --frozen-lockfile; fi + ;; + yarn) + # Yarn Berry/3+: prefer immutable to ensure lockfile determinism + yarn install --immutable + ;; + pnpm) + pnpm install --frozen-lockfile + ;; + npm) + if [ -f package-lock.json ]; then npm ci; else npm install --no-audit --no-fund; fi + ;; + esacdocs/docs/usage/create-an-app-with-nitro-module-setup.md (1)
43-47: Fix nested list indentation and render tests literallyCurrent indentation triggers MD007 and “tests” renders as emphasis. Use backticks and 2-space nesting.
Apply this diff:
- - **my-nitro-module/**: Example of a nitro module. - - **\_**_tests_**\_/**: Contains test files. - - **android/**: Contains the Android native module implementation. - - **ios/**: Contains the iOS native module implementation. - - **src/**: Contains the native module specs. + - **my-nitro-module/**: Example of a nitro module. + - `__tests__/`: Contains test files. + - **android/**: Contains the Android native module implementation. + - **ios/**: Contains the iOS native module implementation. + - **src/**: Contains the native module specs.CHANGELOG.md (1)
61-63: Minor grammar fixes in entriesTighten wording for clarity.
Apply this diff:
-- bum up nitro to 0.27.2 - - bump up nitro and react-native +- bump nitro to 0.27.2 +- bump nitro and React Native.github/actions/android-gradle-build/action.yml (1)
13-21: Add --stacktrace to Gradle invocations for better CI diagnosticsHelps triage failures without re-running jobs.
- ./gradlew clean - ./gradlew generateCodegenArtifactsFromSchema - ./gradlew assemble${{ inputs.mode }} --no-daemon --build-cache + ./gradlew clean --stacktrace + ./gradlew generateCodegenArtifactsFromSchema --stacktrace + ./gradlew assemble${{ inputs.mode }} --no-daemon --build-cache --stacktraceeslint.config.js (1)
32-39: Avoid false positives with object rest patternsConsider enabling ignoreRestSiblings to prevent reports like const { used, ..._rest } = obj.
'@typescript-eslint/no-unused-vars': [ 'error', { argsIgnorePattern: '^_', varsIgnorePattern: '^_', caughtErrorsIgnorePattern: '^_', + ignoreRestSiblings: true, }, ],.github/actions/run-codegen-build/action.yml (1)
13-21: Quote pm invocation for safetyMinor robustness; avoids edge cases if pm ever carries spaces or flags via matrix inputs.
- ${{ inputs.pm }} run build + "${{ inputs.pm }}" run build.github/workflows/deploy.yml (1)
29-33: Pin Bun to a specific version for deterministic CI.Using
latestcan introduce silent CI drift. Consider pinning to an exact Bun release or a repo/org variable.- - name: Setup Bun.js - uses: oven-sh/setup-bun@v2 - with: - bun-version: latest + - name: Setup Bun.js + uses: oven-sh/setup-bun@v2 + with: + # Option A: exact pin + bun-version: '1.1.XX' + # Option B: manage via repo variable + # bun-version: ${{ vars.BUN_VERSION }}.github/actions/setup-yarn/action.yml (2)
4-6: Make Yarn version configurable via input (keeps default pin).Small DX improvement: allow callers to override the pinned version without editing the action.
inputs: - working-directory: + working-directory: description: Directory to run yarn config required: true + yarn-version: + description: Yarn version to set (defaults to pinned) + required: false + default: '4.6.0'
15-18: Use the new input for the version; rest looks solid.Quoting is fine;
node-moduleslinker and immutable installs toggle are correct for Yarn v4.- yarn set version 4.6.0 + yarn set version "${{ inputs.yarn-version }}".github/workflows/release.yml (2)
23-31: Pin Bun version for reproducible releasesUsing bun-version: latest can break the pipeline when Bun ships breaking changes. Pin to a known-good version and update intentionally.
- with: - bun-version: latest + with: + bun-version: '1.1.29'
26-33: Broaden cache key to include Bun’s default lockfile (.lockb)Bun defaults to bun.lockb; your key only hashes bun.lock. Include both so cache hits remain effective regardless of lockfile format.
- key: ${{ runner.os }}-bun-${{ hashFiles('**/bun.lock') }} + key: ${{ runner.os }}-bun-${{ hashFiles('**/bun.lock', '**/bun.lockb') }}docs/docusaurus.config.ts (1)
39-49: Optional: use require.resolve for file pathsDocusaurus often recommends require.resolve for robustness across cwd changes. Not required, but a small hardening.
- sidebarPath: './sidebars.ts', + sidebarPath: require.resolve('./sidebars.ts'), ... - customCss: './src/css/custom.css', + customCss: require.resolve('./src/css/custom.css'),lefthook.yml (1)
9-21: Ensure formatted files are re-stagedIf bun format writes changes, consider stage_fixed: true so Lefthook re-stages them and the commit succeeds without a second attempt.
pre-commit: parallel: true commands: lint: glob: - '*.json' - '*.ts' - '*.tsx' run: bun lint {staged_files} format: + stage_fixed: true glob: - '*.json' - '*.ts' - '*.tsx' run: bun format {staged_files}docs/src/components/HomepageFeatures/index.tsx (1)
63-66: Prefer stable keys over indexUsing array index as key can cause unnecessary re-renders if the list changes. Title is unique here and works as a stable key.
- {FeatureList.map((props, idx) => ( - <Feature key={idx} {...props} /> + {FeatureList.map((props) => ( + <Feature key={props.title} {...props} /> ))}.github/actions/ios-build-xcode/action.yml (1)
21-30: Avoid hard-coding simulator device; make it configurable.Hard-coded 'iPhone 16' can break when the runner’s simulators differ. Expose a 'device' input and use it in -destination.
inputs: ios-dir: description: Path to example/ios directory required: true scheme: description: Xcode scheme to build required: true mode: description: Build configuration (Debug|Release) required: true + device: + description: iOS Simulator device name (e.g., 'iPhone 16') + required: false + default: iPhone 16 @@ - -destination 'platform=iOS Simulator,name=iPhone 16' \ + -destination 'platform=iOS Simulator,name=${{ inputs.device }}' \.github/workflows/ci-packages.yml (1)
356-367: Add timeouts to long-running jobs to avoid stuck workflows.E2E and matrix builds can hang; set conservative timeouts per job.
e2e-android: - if: always() + if: always() + timeout-minutes: 60 @@ e2e-ios: - if: always() + if: always() + timeout-minutes: 75Also applies to: 445-455
src/generate-nitro-package.ts (1)
431-435: Duplicate write to settings.gradle; remove the first write.settings.gradle is written twice with the same content. Drop the earlier write.
- await writeFile( - androidSettingsGradlePath, - androidSettingsGradleCode(toPascalCase(this.config.packageName)), - { encoding: 'utf8' } - )Also applies to: 474-480
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (36)
.github/actions/android-gradle-build/action.yml(1 hunks).github/actions/install-deps/action.yml(1 hunks).github/actions/ios-build-xcode/action.yml(1 hunks).github/actions/run-codegen-build/action.yml(1 hunks).github/actions/setup-yarn/action.yml(1 hunks).github/dependabot.yml(1 hunks).github/workflows/ci-packages.yml(1 hunks).github/workflows/deploy.yml(1 hunks).github/workflows/release.yml(1 hunks)CHANGELOG.md(1 hunks)assets/react-native.config.js(1 hunks)assets/template/.github/dependabot.yml(1 hunks)assets/template/.github/workflows/release.yml(1 hunks)assets/template/.watchmanconfig(1 hunks)assets/template/nitro.json(1 hunks)assets/template/release.config.cjs(1 hunks)docs/docs/intro.md(1 hunks)docs/docs/troubleshooting.md(0 hunks)docs/docs/usage/_category_.json(1 hunks)docs/docs/usage/create-an-app-with-nitro-module-setup.md(1 hunks)docs/docusaurus.config.ts(1 hunks)docs/package.json(1 hunks)docs/sidebars.ts(3 hunks)docs/src/components/HomepageFeatures/index.tsx(1 hunks)docs/src/components/HomepageFeatures/styles.module.css(1 hunks)docs/src/css/custom.css(1 hunks)docs/src/pages/index.module.css(1 hunks)docs/src/pages/index.tsx(1 hunks)docs/tsconfig.json(1 hunks)e2e-tests/view.e2e.yaml(1 hunks)eslint.config.js(1 hunks)lefthook.yml(1 hunks)src/generate-nitro-package.ts(4 hunks)src/nitro-spinner.ts(1 hunks)tsconfig.json(1 hunks)tsup.config.ts(1 hunks)
💤 Files with no reviewable changes (1)
- docs/docs/troubleshooting.md
🧰 Additional context used
🧬 Code graph analysis (1)
docs/src/pages/index.tsx (1)
docs/src/components/HomepageFeatures/index.tsx (1)
HomepageFeatures(59-71)
🪛 markdownlint-cli2 (0.17.2)
docs/docs/usage/create-an-app-with-nitro-module-setup.md
43-43: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
44-44: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
45-45: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
46-46: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
47-47: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
🪛 LanguageTool
CHANGELOG.md
[grammar] ~5-~5: There might be a mistake here.
Context: ...75f6b2d97861ea845e26d547b4a906646d62f6)) - update Node.js engine requirement from >...
(QB_NEW_EN)
[grammar] ~14-~14: There might be a mistake here.
Context: ...503e983b4d0f612)) ### 🛠️ Other changes - android: enable RN_SERIALIZABLE_STATE ...
(QB_NEW_EN)
[grammar] ~44-~44: There might be a mistake here.
Context: ...838ef76fa91248cca152f2344d819c2bb6fd63)) - add support for --package-type which c...
(QB_NEW_EN)
[grammar] ~47-~47: There might be a mistake here.
Context: ...155654e4883d41cb7888)) ### 🐛 Bug Fixes - ci: update e2e workflow to use dynamic...
(QB_NEW_EN)
[grammar] ~55-~55: There might be a mistake here.
Context: ...c34ed109282efaf)) ### 🔄 Code Refactors - improve formatting and readability in cr...
(QB_NEW_EN)
[grammar] ~59-~59: There might be a mistake here.
Context: ...91d8eb4e7bd3909)) ### 🛠️ Other changes - add xcpretty installation step to CI wor...
(QB_NEW_EN)
[grammar] ~91-~91: There might be a mistake here.
Context: ...t.2) (2025-07-31) ### 🛠️ Other changes - update package.json dependencies and add...
(QB_NEW_EN)
[grammar] ~99-~99: There might be a mistake here.
Context: ...838ef76fa91248cca152f2344d819c2bb6fd63)) - add support for --package-type which c...
(QB_NEW_EN)
[grammar] ~102-~102: There might be a mistake here.
Context: ...155654e4883d41cb7888)) ### 🐛 Bug Fixes - ci: update e2e workflow to use dynamic...
(QB_NEW_EN)
[grammar] ~110-~110: There might be a mistake here.
Context: ...c34ed109282efaf)) ### 🔄 Code Refactors - improve formatting and readability in cr...
(QB_NEW_EN)
[grammar] ~114-~114: There might be a mistake here.
Context: ...91d8eb4e7bd3909)) ### 🛠️ Other changes - add xcpretty installation step to CI wor...
(QB_NEW_EN)
[grammar] ~142-~142: There might be a mistake here.
Context: ...0.3) (2025-07-06) ### 🛠️ Other changes - deps: downgrade react-native from 0.80...
(QB_NEW_EN)
[grammar] ~148-~148: There might be a mistake here.
Context: ...0.2) (2025-07-05) ### 🛠️ Other changes - deps-dev: bump @typescript-eslint/esli...
(QB_NEW_EN)
[grammar] ~150-~150: There might be a mistake here.
Context: ...c290e1ad47bb228446c89587ae8af77d094757)) - deps-dev: bump eslint-plugin-n from 17...
(QB_NEW_EN)
[grammar] ~151-~151: There might be a mistake here.
Context: ...aa1bb4e95cb32032090efb985572634cc448cc)) - deps-dev: bump semantic-release from 2...
(QB_NEW_EN)
[grammar] ~152-~152: There might be a mistake here.
Context: ...c99713f9b6a70e177b64c303eff79f0be381e5)) - deps-dev: bump semantic-release in /as...
(QB_NEW_EN)
[grammar] ~157-~157: There might be a mistake here.
Context: .....v3.0.1) (2025-07-01) ### 🐛 Bug Fixes - remove or overwrite existing folder only...
(QB_NEW_EN)
[grammar] ~161-~161: There might be a mistake here.
Context: ...d5cdb55cee2e822)) ### 🛠️ Other changes - add clean step to Android build workflow...
(QB_NEW_EN)
[grammar] ~180-~180: There might be a mistake here.
Context: ...n field. ### ✨ Features - add support for pnpm package manager ([a456841](https:/...
(QB_NEW_EN)
[grammar] ~182-~182: There might be a mistake here.
Context: ...e8e5b69d0984cfa95696)) ### 🐛 Bug Fixes - add .npmrc file for pnpm configuration a...
(QB_NEW_EN)
[grammar] ~187-~187: There might be a mistake here.
Context: ...cce87ae96a22bcd2)) ### 📚 Documentation - add pnpm support to README and documenta...
(QB_NEW_EN)
[grammar] ~191-~191: There might be a mistake here.
Context: ...751942b0d2f4b92)) ### 🛠️ Other changes - add pnpm to package manager selection in...
(QB_NEW_EN)
[grammar] ~193-~193: There might be a mistake here.
Context: ...561bec1ce12a98c911014bb0b799fff6f735f6)) - deps-dev: bump @typescript-eslint/pars...
(QB_NEW_EN)
[grammar] ~200-~200: There might be a mistake here.
Context: ...1.3) (2025-06-13) ### 🛠️ Other changes - deps-dev: bump @types/node from 22.15....
(QB_NEW_EN)
[grammar] ~206-~206: There might be a mistake here.
Context: ...1.2) (2025-06-07) ### 🛠️ Other changes - deps: update nitro-codegen and react-n...
(QB_NEW_EN)
[grammar] ~208-~208: There might be a mistake here.
Context: ...adf9a8aa1689cbf727aee39cd4544e58d6f857)) - deps: update nitro-codegen and react-n...
(QB_NEW_EN)
[grammar] ~213-~213: There might be a mistake here.
Context: ...1.1) (2025-06-05) ### 🛠️ Other changes - deps-dev: bump @typescript-eslint/esli...
(QB_NEW_EN)
[grammar] ~215-~215: There might be a mistake here.
Context: ...22c3e259aa0dc9231d59dfb1567d64db97fb74)) - deps-dev: bump eslint from 9.27.0 to 9...
(QB_NEW_EN)
[grammar] ~216-~216: There might be a mistake here.
Context: ...e7ace904dc65a92dcafed794dca46293fddd01)) - deps-dev: bump eslint-plugin-n from 17...
(QB_NEW_EN)
[grammar] ~227-~227: There might be a mistake here.
Context: ...08c4b7cdc1f2b0db49573417e5260ff9c7f092)) - package and readme description ([b033671...
(QB_NEW_EN)
[grammar] ~230-~230: There might be a mistake here.
Context: ...4c95327f10fd779)) ### 🔄 Code Refactors - improve code readability in installDepen...
(QB_NEW_EN)
[grammar] ~232-~232: There might be a mistake here.
Context: ...e Refactors - improve code readability in installDependenciesAndRunCodegen method...
(QB_NEW_EN)
[grammar] ~235-~235: There might be a mistake here.
Context: ...d05bc8de8817957)) ### 🛠️ Other changes - update ESLint configuration to use defin...
(QB_NEW_EN)
[grammar] ~256-~256: There might be a mistake here.
Context: ...81e01cff985870340524)) ### 🐛 Bug Fixes - Clean up code formatting and ensure newl...
(QB_NEW_EN)
[grammar] ~265-~265: There might be a mistake here.
Context: ...61d2bda8ddcaa0b)) ### 🔄 Code Refactors - standardize naming from moduleName to pa...
(QB_NEW_EN)
[grammar] ~270-~270: There might be a mistake here.
Context: ...87ef7880767216f2)) ### 📚 Documentation - add Semantic Release mention to README a...
(QB_NEW_EN)
[grammar] ~274-~274: There might be a mistake here.
Context: ...7e51efabb8be986)) ### 🛠️ Other changes - add Act configuration and update GitHub ...
(QB_NEW_EN)
[grammar] ~322-~322: There might be a mistake here.
Context: ...6ebef9d07997480fc486)) ### 🐛 Bug Fixes - correct syntax in package generation key...
(QB_NEW_EN)
[grammar] ~330-~330: There might be a mistake here.
Context: ...0-next.4) (2025-06-01) ### 🐛 Bug Fixes - workflow ([d35a0b6](https://github.com/p...
(QB_NEW_EN)
[grammar] ~334-~334: There might be a mistake here.
Context: ...61d2bda8ddcaa0b)) ### 🛠️ Other changes - enhance GitHub workflow for code generat...
(QB_NEW_EN)
[grammar] ~341-~341: There might be a mistake here.
Context: ...t.3) (2025-06-01) ### 🛠️ Other changes - update GitHub workflow to support both D...
(QB_NEW_EN)
[grammar] ~348-~348: There might be a mistake here.
Context: ...0-next.2) (2025-05-31) ### 🐛 Bug Fixes - ensure newline at end of package.json fi...
(QB_NEW_EN)
[grammar] ~352-~352: There might be a mistake here.
Context: ...c2e738d14fab589)) ### 🔄 Code Refactors - standardize naming from moduleName to pa...
(QB_NEW_EN)
[grammar] ~357-~357: There might be a mistake here.
Context: ...87ef7880767216f2)) ### 📚 Documentation - add Semantic Release mention to README a...
(QB_NEW_EN)
[grammar] ~361-~361: There might be a mistake here.
Context: ...7e51efabb8be986)) ### 🛠️ Other changes - add Act configuration and update GitHub ...
(QB_NEW_EN)
[grammar] ~396-~396: There might be a mistake here.
Context: ....10) (2025-05-29) ### 🛠️ Other changes - deps: bump commander from 13.1.0 to 14...
(QB_NEW_EN)
[grammar] ~398-~398: There might be a mistake here.
Context: ...fc5a7ffe2123745f91bb690e6ad6d8e5d89b69)) - deps: bump inquirer from 12.6.1 to 12....
(QB_NEW_EN)
[grammar] ~399-~399: There might be a mistake here.
Context: ...35eb696f69d3183d6f8625d8e0fe4776fb5aab)) - deps: bump tsup from 8.4.0 to 8.5.0 ([...
(QB_NEW_EN)
[grammar] ~404-~404: There might be a mistake here.
Context: ...8.9) (2025-05-29) ### 🛠️ Other changes - deps-dev: bump conventional-changelog-...
(QB_NEW_EN)
[grammar] ~410-~410: There might be a mistake here.
Context: ...8.8) (2025-05-25) ### 🛠️ Other changes - deps-dev: bump eslint from 9.26.0 to 9...
(QB_NEW_EN)
[grammar] ~412-~412: There might be a mistake here.
Context: ...49ca45b9539400306968ad6e4b63d69a991604)) - deps-dev: bump semantic-release from 2...
(QB_NEW_EN)
[grammar] ~417-~417: There might be a mistake here.
Context: ...8.7) (2025-05-24) ### 🛠️ Other changes - deps-dev: bump @types/node from 22.15....
(QB_NEW_EN)
[grammar] ~423-~423: There might be a mistake here.
Context: .....v1.8.6) (2025-05-19) ### 🐛 Bug Fixes - release workflow ([2f8ddb8](https://gith...
(QB_NEW_EN)
[grammar] ~425-~425: There might be a mistake here.
Context: ...8ddb83c3b556754d704c24c19e8653303c2c53)) - release workflow ([1d76a6f](https://gith...
(QB_NEW_EN)
[grammar] ~428-~428: There might be a mistake here.
Context: ...fe565941917d826)) ### 🛠️ Other changes - update semantic-release configuration an...
(QB_NEW_EN)
🔇 Additional comments (35)
docs/src/css/custom.css (1)
9-17: LGTM — formatting-only change with no styling impactNo functional differences; variables and values are unchanged.
tsconfig.json (1)
2-13: Formatting-only change; no semantic impact.Indentation update looks fine; compiler options remain unchanged.
docs/docs/intro.md (1)
43-43: Minor docs formatting tweak is OK.Trailing newline after the code fence improves consistency.
e2e-tests/view.e2e.yaml (1)
5-5: YAML formatting is acceptable.Indentation under
assertVisible:remains valid for a nested mapping.docs/docs/usage/_category_.json (1)
2-8: JSON reindent only; semantics unchanged.Looks good.
assets/template/.watchmanconfig (1)
1-1: EOF newline addition is fine.No behavioral change.
tsup.config.ts (1)
4-8: Formatting-only change — LGTMNo behavioral impact.
docs/src/pages/index.module.css (1)
7-11: Whitespace/indent updates — LGTMNo CSS semantics changed.
Also applies to: 14-16, 20-23
docs/src/components/HomepageFeatures/styles.module.css (1)
2-6: Re-indentation — LGTMPure formatting.
Also applies to: 9-10
docs/tsconfig.json (1)
2-7: Formatting-only change — LGTMNo semantic differences; safe to merge.
CHANGELOG.md (1)
5-6: Bullet style normalization — LGTMSwitching to dashes is consistent and harmless.
assets/template/.github/dependabot.yml (2)
53-56: Nice: added Nitro update groupKeeps nitro-codegen and react-native-nitro-modules aligned in a single PR. Looks good.
31-57: No action needed: root .github/dependabot.yml already defines the “nitro” group with bothnitro-codegenandreact-native-nitro-modulespatterns..github/actions/android-gradle-build/action.yml (1)
4-9: Formatting-only change — OKRe-indentation of inputs preserves semantics. No behavior change.
assets/react-native.config.js (1)
8-17: Formatting-only change — OKIndentation/whitespace tweaks only. No runtime or CLI behavior change.
eslint.config.js (1)
32-39: Underscore-prefixed unuseds: looks goodThis matches the PR goal and uses the canonical @typescript-eslint rule config.
assets/template/.github/workflows/release.yml (1)
52-55: Quoting change is safeSingle quotes around expressions keep GitHub expressions functional and do not alter behavior.
.github/actions/run-codegen-build/action.yml (1)
4-9: Formatting-only change — OKInputs block re-indented without semantic impact.
.github/workflows/deploy.yml (2)
55-58: LGTM: environment URL wiring matches Pages deploy pattern.Referencing
steps.deployment.outputs.page_urlis the standard pattern for Pages.
25-27: actions/checkout@v5 is published and stable (v5.0.0 on Aug 11, 2025); remove the v4 fallback.assets/template/release.config.cjs (2)
2-7: Formatting-only change—no functional impact.Rules/readability improvements look good.
18-59: Semantic-release config reads clearly and remains equivalent.No behavior change detected; sorting and grouping logic preserved.
.github/dependabot.yml (2)
3-9: LGTM: GitHub Actions ecosystem block.Daily cadence and label are fine.
10-34: Ignore conversion suggestion; original Dependabot config is valid
Dependabot supportspackage-ecosystem: bunand thedirectorieskey for multiple paths—no need to switch tonpmor split into separate entries.Likely an incorrect or invalid review comment.
docs/sidebars.ts (2)
1-1: LGTM: type-only import formatting.
16-18: LGTM: consistent sidebar config formatting; no behavior change..github/workflows/release.yml (1)
41-50: Release env looks soundGITHUB_TOKEN, NPM_TOKEN, and provenance/id-token are set correctly; author/committer vars are fine. No action needed.
docs/docusaurus.config.ts (1)
35-50: Config changes are formatting-only—LGTMContents and types (satisfies Preset.Options/ThemeConfig) remain equivalent. Safe.
lefthook.yml (1)
1-5: Commit-msg hook looks goodcommitlint via Bun is straightforward. No changes needed.
docs/src/components/HomepageFeatures/index.tsx (1)
14-42: Formatting-only—LGTMNo semantic changes to FeatureList or rendering. All good.
docs/src/pages/index.tsx (2)
12-30: HomepageHeader reformat—LGTMPure formatting. Behavior unchanged.
34-45: Home component reformat—LGTMNo functional changes. Safe to merge.
.github/actions/ios-build-xcode/action.yml (1)
24-30: Verify availability of “iPhone 16” simulator on the runner
Ensure that the macOS GitHub Actions image includes “iPhone 16”; otherwise,xcodebuildwill fail. On the runner, run:xcrun simctl list devices available | grep 'iPhone 16'and update the
-destinationflag if the device is missing.assets/template/nitro.json (1)
3-12: LGTM — formatting only.Inline arrays are fine and keep schema-compatible. No functional impact.
src/generate-nitro-package.ts (1)
187-205: Nice: pinning nitro-codegen and nitro-modules in the workspace.Parallel resolution and fallback to template devDependencies is solid.
| - name: Build CLI and Link Locally | ||
| run: | | ||
| bun run build | ||
| ${{matrix.pm}} link | ||
| - name: Generate ${{ matrix.package-type }} with ${{ matrix.pm }} | ||
| if: matrix.pm != 'pnpm' | ||
| run: ${{ matrix.pm }} create nitro-module test-${{ matrix.package-type }} --skip-install --ci --package-type ${{ matrix.package-type }} | ||
|
|
||
| - name: Generate ${{ matrix.package-type }} with ${{ matrix.pm }} (pnpm) | ||
| if: matrix.pm == 'pnpm' | ||
| run: create-nitro-module test-${{ matrix.package-type }} --skip-install --ci --package-type ${{ matrix.package-type }} | ||
|
|
There was a problem hiding this comment.
CI is invoking the published generator, not the local PR build.
Using “bun|yarn create nitro-module …” fetches from npm and bypasses your local changes, so the workflow doesn’t validate this PR. Call the built CLI directly.
- - name: Build CLI and Link Locally
- run: |
- bun run build
- ${{matrix.pm}} link
+ - name: Build CLI
+ run: bun run build
@@
- - name: Generate ${{ matrix.package-type }} with ${{ matrix.pm }}
- if: matrix.pm != 'pnpm'
- run: ${{ matrix.pm }} create nitro-module test-${{ matrix.package-type }} --skip-install --ci --package-type ${{ matrix.package-type }}
+ - name: Generate ${{ matrix.package-type }} with local CLI
+ run: node lib/cli/index.js create test-${{ matrix.package-type }} --skip-install --ci --package-type ${{ matrix.package-type }}
@@
- - name: Generate ${{ matrix.package-type }} with ${{ matrix.pm }} (pnpm)
- if: matrix.pm == 'pnpm'
- run: create-nitro-module test-${{ matrix.package-type }} --skip-install --ci --package-type ${{ matrix.package-type }}
+ # (pnpm path no longer needed when invoking CLI directly)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Build CLI and Link Locally | |
| run: | | |
| bun run build | |
| ${{matrix.pm}} link | |
| - name: Generate ${{ matrix.package-type }} with ${{ matrix.pm }} | |
| if: matrix.pm != 'pnpm' | |
| run: ${{ matrix.pm }} create nitro-module test-${{ matrix.package-type }} --skip-install --ci --package-type ${{ matrix.package-type }} | |
| - name: Generate ${{ matrix.package-type }} with ${{ matrix.pm }} (pnpm) | |
| if: matrix.pm == 'pnpm' | |
| run: create-nitro-module test-${{ matrix.package-type }} --skip-install --ci --package-type ${{ matrix.package-type }} | |
| - name: Build CLI | |
| run: bun run build | |
| - name: Generate ${{ matrix.package-type }} with local CLI | |
| run: node lib/cli/index.js create test-${{ matrix.package-type }} --skip-install --ci --package-type ${{ matrix.package-type }} | |
| # (pnpm path no longer needed when invoking CLI directly) |
🤖 Prompt for AI Agents
.github/workflows/ci-packages.yml lines 119-131: the workflow currently uses
package manager "create" commands which fetch the published generator from npm
(bypassing the local build) — replace those run steps so they invoke the locally
built CLI artifact produced by the preceding build step (call the built
binary/executable in the repo output directory for both the pnpm and non-pnpm
branches) instead of "bun|yarn create" or the package manager wrapper, ensuring
the local build is used to generate the test-${{ matrix.package-type }} modules.
| runs-on: macOS-latest | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: |
There was a problem hiding this comment.
Fix runner labels casing: macOS runners are “macos-*”.
“macOS-latest/macOS-15” are invalid and will not match any runner.
- runs-on: macOS-latest
+ runs-on: macos-latest
@@
- runs-on: macOS-15
+ runs-on: macos-15Also applies to: 449-450
🤖 Prompt for AI Agents
.github/workflows/ci-packages.yml around lines 183-186 (and also update lines
449-450): the workflow uses incorrect runner labels with upper-case "macOS-"
(e.g., "macOS-latest" / "macOS-15") which are invalid; change them to the
correct lowercase labels "macos-latest" and "macos-15" (or the appropriate
macos-* label used elsewhere) so the runner names match GitHub Actions' expected
casing, and ensure both occurrences at 183-186 and 449-450 are updated
consistently.
| "name": "docs", | ||
| "version": "0.0.0", | ||
| "private": true, | ||
| "scripts": { | ||
| "docusaurus": "docusaurus", | ||
| "start": "docusaurus start", | ||
| "build": "docusaurus build", | ||
| "swizzle": "docusaurus swizzle", | ||
| "deploy": "docusaurus deploy", | ||
| "clear": "docusaurus clear", | ||
| "serve": "docusaurus serve", | ||
| "write-translations": "docusaurus write-translations", | ||
| "write-heading-ids": "docusaurus write-heading-ids", | ||
| "typecheck": "tsc" | ||
| }, | ||
| "dependencies": { | ||
| "@docusaurus/core": "3.6.3", | ||
| "@docusaurus/preset-classic": "3.6.3", | ||
| "@mdx-js/react": "^3.0.0", | ||
| "clsx": "^2.0.0", | ||
| "prism-react-renderer": "^2.3.0", | ||
| "react": "^18.0.0", | ||
| "react-dom": "^18.0.0" | ||
| }, | ||
| "devDependencies": { | ||
| "@docusaurus/module-type-aliases": "3.6.3", | ||
| "@docusaurus/tsconfig": "3.6.3", | ||
| "@docusaurus/types": "3.6.3", | ||
| "typescript": "~5.6.2" | ||
| }, | ||
| "browserslist": { | ||
| "production": [ | ||
| ">0.5%", | ||
| "not dead", | ||
| "not op_mini all" | ||
| ], | ||
| "development": [ | ||
| "last 3 chrome version", | ||
| "last 3 firefox version", | ||
| "last 5 safari version" | ||
| ] | ||
| }, | ||
| "engines": { | ||
| "node": ">=18.0" | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Reformatting — LGTM; ensure Node engine stays consistent repo-wide
If root/package engines differ from ">=18.0", align or document exceptions.
Run to compare all package.json engine fields:
🏁 Script executed:
#!/bin/bash
fd -a package.json | xargs -I {} sh -c 'echo {}; jq -r ".engines.node // empty" "{}"; echo'Length of output: 227
Align Node engine versions across package.json files
The root package.json specifies "node": ">=22", docs/package.json defines "node": ">=18.0", and assets/template/package.json omits the engines field. Update docs/package.json and assets/template/package.json to match the root engines.node (>=22), or document valid exceptions.
🤖 Prompt for AI Agents
In docs/package.json (lines 2-46) and assets/template/package.json, the Node
engine requirement is inconsistent with the root package.json (which requires
"node": ">=22"); update both files' "engines.node" field to ">=22" to match the
root (or add the same "engines" block if missing in
assets/template/package.json), ensure JSON remains valid, and run a quick
lint/typecheck to confirm no other mismatches.
| private async getLatestVersion(pkg: string): Promise<string | null> { | ||
| try { | ||
| const { stdout } = await execAsync(`npm view ${pkg} version`) | ||
| const v = stdout?.toString().trim() | ||
| return v?.length ? v : null | ||
| } catch (_) { | ||
| try { | ||
| const { stdout } = await execAsync(`pnpm view ${pkg} version`) | ||
| const v = stdout?.toString().trim() | ||
| return v?.length ? v : null | ||
| } catch { | ||
| return null | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden version resolution: add timeouts and pin registry.
External commands can hang or be influenced by custom registries. Add short timeouts and use the npmjs registry explicitly.
- private async getLatestVersion(pkg: string): Promise<string | null> {
+ private async getLatestVersion(pkg: string): Promise<string | null> {
try {
- const { stdout } = await execAsync(`npm view ${pkg} version`)
+ const { stdout } = await execAsync(
+ `npm view ${pkg} version --registry=https://registry.npmjs.org`,
+ { timeout: 15000 }
+ )
const v = stdout?.toString().trim()
return v?.length ? v : null
} catch (_) {
try {
- const { stdout } = await execAsync(`pnpm view ${pkg} version`)
+ const { stdout } = await execAsync(
+ `pnpm view ${pkg} version --registry=https://registry.npmjs.org`,
+ { timeout: 15000 }
+ )
const v = stdout?.toString().trim()
return v?.length ? v : null
} catch {
return null
}
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private async getLatestVersion(pkg: string): Promise<string | null> { | |
| try { | |
| const { stdout } = await execAsync(`npm view ${pkg} version`) | |
| const v = stdout?.toString().trim() | |
| return v?.length ? v : null | |
| } catch (_) { | |
| try { | |
| const { stdout } = await execAsync(`pnpm view ${pkg} version`) | |
| const v = stdout?.toString().trim() | |
| return v?.length ? v : null | |
| } catch { | |
| return null | |
| } | |
| } | |
| } | |
| private async getLatestVersion(pkg: string): Promise<string | null> { | |
| try { | |
| const { stdout } = await execAsync( | |
| `npm view ${pkg} version --registry=https://registry.npmjs.org`, | |
| { timeout: 15000 } | |
| ) | |
| const v = stdout?.toString().trim() | |
| return v?.length ? v : null | |
| } catch (_) { | |
| try { | |
| const { stdout } = await execAsync( | |
| `pnpm view ${pkg} version --registry=https://registry.npmjs.org`, | |
| { timeout: 15000 } | |
| ) | |
| const v = stdout?.toString().trim() | |
| return v?.length ? v : null | |
| } catch { | |
| return null | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/generate-nitro-package.ts around lines 109 to 123, the version lookup
uses external npm/pnpm commands without timeouts or a pinned registry; update
the two execAsync calls to include a short timeout (e.g. 5s) and pass the
registry flag (--registry=https://registry.npmjs.org) to the command strings so
the lookup cannot hang or be redirected by custom registries, and keep the
existing stdout trimming/length checks and catch blocks intact.
| const nitroKey = `react-native-nitro-modules` | ||
| exampleAppPackageJson.dependencies = { | ||
| ...exampleAppPackageJson.dependencies, | ||
| [nitroKey]: templatePackageJson.devDependencies[nitroKey] ?? '*', | ||
| [nitroKey]: this.nitroModulesVersion ?? '*', | ||
| } |
There was a problem hiding this comment.
Avoid '*' fallback in example app; use the workspace-pinned version.
Falling back to '*' breaks determinism if resolution fails. Prefer the version you pinned in the workspace or template.
- const nitroKey = `react-native-nitro-modules`
+ const nitroKey = `react-native-nitro-modules`
+ // Read the workspace's pinned version as a deterministic fallback
+ const workspacePkg = JSON.parse(
+ await readFile(path.join(this.config.cwd, 'package.json'), { encoding: 'utf8' })
+ )
+ const pinnedNitroFromWorkspace =
+ workspacePkg?.devDependencies?.[nitroKey] ??
+ templatePackageJson.devDependencies[nitroKey]
@@
exampleAppPackageJson.dependencies = {
...exampleAppPackageJson.dependencies,
- [nitroKey]: this.nitroModulesVersion ?? '*',
+ [nitroKey]: this.nitroModulesVersion ?? pinnedNitroFromWorkspace,
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const nitroKey = `react-native-nitro-modules` | |
| exampleAppPackageJson.dependencies = { | |
| ...exampleAppPackageJson.dependencies, | |
| [nitroKey]: templatePackageJson.devDependencies[nitroKey] ?? '*', | |
| [nitroKey]: this.nitroModulesVersion ?? '*', | |
| } | |
| const nitroKey = `react-native-nitro-modules` | |
| // Read the workspace's pinned version as a deterministic fallback | |
| const workspacePkg = JSON.parse( | |
| await readFile(path.join(this.config.cwd, 'package.json'), { encoding: 'utf8' }) | |
| ) | |
| const pinnedNitroFromWorkspace = | |
| workspacePkg?.devDependencies?.[nitroKey] ?? | |
| templatePackageJson.devDependencies[nitroKey] | |
| exampleAppPackageJson.dependencies = { | |
| ...exampleAppPackageJson.dependencies, | |
| [nitroKey]: this.nitroModulesVersion ?? pinnedNitroFromWorkspace, | |
| } |
🤖 Prompt for AI Agents
In src/generate-nitro-package.ts around lines 360 to 364, the code currently
falls back to '*' for react-native-nitro-modules when this.nitroModulesVersion
is undefined; replace that fallback with the workspace-pinned version from the
workspace/template (e.g., read it from the root/template package.json or the
generator's pinned versions map) so the dependency becomes [nitroKey]:
this.nitroModulesVersion ?? workspacePinnedVersion; if the workspace-pinned
version is also missing, surface an explicit error or fail the build rather than
using '*'.
patrickkabwe
deleted the
feat/pin-nitro-versions-and-eslint-underscore
branch
Summary by CodeRabbit