Conversation
|
Warning Review limit reached
More reviews will be available in 43 minutes and 46 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (34)
📝 WalkthroughWalkthroughNx 23 support updates workspace metadata, workflows, generator behavior, devkit compatibility imports, e2e command handling, and documentation. It also adds the new plugin dependency override guide and refreshes package and ignore settings. ChangesNx 23 support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/nx-forge/src/generators/application/lib/add-project-dependencies.ts (1)
15-26: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
getLatestPackageVersionthrows on network failure, defeating the'latest'fallback and adding a network call with no timeout.The function rethrows on any fetch/parse error, so a transient registry hiccup aborts the whole generator. Yet
addProjectDependencies(Lines 47-57) is written to warn and fall back to'latest'when a version is missing — that fallback path is unreachable for network errors because they throw first. Additionally,fetchhas no timeout, so a slow/unreachable registry can hang the generator indefinitely.Decide on one contract: if the intent is graceful degradation, return
undefinedon failure (and add a timeout); the existing warn-and-fallback in the caller will handle it.🛡️ Suggested change to align with fallback intent + add a timeout
async function getLatestPackageVersion( pkg: string ): Promise<string | undefined> { try { - const response = await fetch(`https://registry.npmjs.org/${pkg}`); + const response = await fetch(`https://registry.npmjs.org/${pkg}`, { + signal: AbortSignal.timeout(10_000), + }); + if (!response.ok) { + logger.error(`Failed to fetch latest version of ${pkg}: ${response.status}`); + return undefined; + } const json = await response.json(); return json?.['dist-tags']?.['latest']; } catch (error) { logger.error(`Failed to fetch latest version of ${pkg}: ${error}`); - throw new Error(`Failed to fetch latest version of ${pkg}`); + return 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 `@packages/nx-forge/src/generators/application/lib/add-project-dependencies.ts` around lines 15 - 26, getLatestPackageVersion currently throws on fetch/parse failures, which prevents addProjectDependencies from using its existing warn-and-fallback-to-'latest' path. Update getLatestPackageVersion to return undefined on network or JSON errors instead of rethrowing, and add a timeout to the fetch call so a slow npm registry does not hang the generator. Keep the logging in getLatestPackageVersion, and make sure the caller in addProjectDependencies continues to handle missing versions gracefully.
🤖 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/nx-forge/src/executors/tunnel/executor.ts`:
- Line 2: The tunnel executor currently imports combineAsyncIterables from the
internal `@nx/devkit/internal` entry point, which creates a brittle dependency on
Nx internals. Update the executor in executor.ts to stop using that internal
import and instead use a local helper or a documented public API equivalent,
keeping the change confined to the combineAsyncIterables usage in the tunnel
executor logic.
In `@packages/nx-forge/src/generators/application/generator.ts`:
- Line 19: The application generator currently imports logShowProjectCommand
from an unsupported internal Nx API, which breaks compatibility for nx >=23 <24.
Update the generator in application/generator.ts to stop using
`@nx/devkit/internal` and either replace logShowProjectCommand with a supported
public devkit API or remove the call entirely; keep the change localized to the
generator’s import and the place where the command is used.
In
`@packages/nx-forge/src/generators/application/lib/add-project-dependencies.ts`:
- Line 12: The import from the Nx internal subpath is broken in Nx 23 because
`@nx/js/internal` does not export `esbuildVersion`, `tsLibVersion`, or
`typesNodeVersion`. Update `add-project-dependencies` to stop relying on that
internal import and source those version constants from a supported export or
local definition, keeping the same symbols used by the dependency logic. Ensure
the references in this generator still resolve at runtime without importing from
`@nx/js/internal`.
In `@packages/nx-forge/src/generators/application/lib/add-project.ts`:
- Line 9: The import for addBuildTargetDefaults is using an unsupported internal
Nx entrypoint, which can break when the generator loads in Nx 23. Update
add-project.ts to import addBuildTargetDefaults from the supported public Nx
devkit export used elsewhere in the codebase, and keep the rest of the generator
logic unchanged so the symbol still resolves correctly.
In `@packages/nx-forge/src/generators/application/lib/normalize-options.ts`:
- Line 3: The import in normalize-options.ts is using an unsupported internal Nx
entrypoint that does not actually export determineProjectNameAndRootOptions.
Update the normalizeOptions-related code to import
determineProjectNameAndRootOptions from the supported public Nx devkit location
instead of `@nx/devkit/internal`, and make sure any references in the generator
still resolve through the existing normalize-options flow.
In
`@packages/nx-forge/src/migrations/update-3-0-0-webpack-config-setup/update-3-0-0-webpack-config-setup.ts`:
- Line 7: The migration script currently imports forEachExecutorOptions from the
internal Nx path, which should be switched to the public devkit API. Update the
import in update-3-0-0-webpack-config-setup.ts to use `@nx/devkit` instead of
`@nx/devkit/internal`, keeping the rest of the migration logic unchanged.
---
Outside diff comments:
In
`@packages/nx-forge/src/generators/application/lib/add-project-dependencies.ts`:
- Around line 15-26: getLatestPackageVersion currently throws on fetch/parse
failures, which prevents addProjectDependencies from using its existing
warn-and-fallback-to-'latest' path. Update getLatestPackageVersion to return
undefined on network or JSON errors instead of rethrowing, and add a timeout to
the fetch call so a slow npm registry does not hang the generator. Keep the
logging in getLatestPackageVersion, and make sure the caller in
addProjectDependencies continues to handle missing versions gracefully.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5e58f22c-e70b-4b1f-9e2d-dfea33f8314a
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
.gitignore.prettierignoree2e/nx-forge-e2e/src/utils/async-commands.tse2e/nx-forge-e2e/src/utils/test-workspace.tsnx.jsonpackage.jsonpackages/nx-forge/package.jsonpackages/nx-forge/src/executors/tunnel/executor.tspackages/nx-forge/src/generators/application/generator.spec.tspackages/nx-forge/src/generators/application/generator.tspackages/nx-forge/src/generators/application/lib/add-app-files.tspackages/nx-forge/src/generators/application/lib/add-project-dependencies.tspackages/nx-forge/src/generators/application/lib/add-project.tspackages/nx-forge/src/generators/application/lib/normalize-options.tspackages/nx-forge/src/generators/application/schema.d.tspackages/nx-forge/src/graph/create-nodes.tspackages/nx-forge/src/migrations/update-3-0-0-webpack-config-setup/update-3-0-0-webpack-config-setup.tspnpm-workspace.yaml
💤 Files with no reviewable changes (1)
- packages/nx-forge/src/generators/application/schema.d.ts
tbinna
force-pushed
the
nx-23
branch
4 times, most recently
from
97bcce0 to
41e3a1b
Compare
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
tbinna
force-pushed
the
nx-23
branch
4 times, most recently
from
d5ab3b9 to
8e4aaa6
Compare
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
tbinna
force-pushed
the
nx-23
branch
3 times, most recently
from
a42f6d2 to
51a4ef8
Compare
|
@coderabbitai we keep the internal imports because these imports are not part of the public API. |
|
Tip For best results, initiate chat on the files or code changes.
(・×・) ✏️ Learnings added
|
|
🎉 This PR is included in version 8.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
1 participant
BREAKING CHANGE: requires Nx 23
Closes #200 #163
Summary by CodeRabbit