Conversation
BREAKING CHANGE: requires Nx 22, removed build and install executors Closes #181
feat(nx): update to Nx 22
📝 WalkthroughWalkthroughThe pull request upgrades the nx-forge plugin to be compatible with Nx 22, removing deprecated Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
e2e/nx-forge-e2e/tests/utils/async-commands.ts (1)
100-106:⚠️ Potential issue | 🟡 MinorInconsistent package manager detection when
cwdis provided.
runNxCommandAsyncdetects the package manager based onopts.cwd, butrunForgeCommandAsynccallsgetPackageManagerCommand()without arguments, using the default detection path. If a caller provides a customcwd(as seen in Context snippet 4), the Forge command may run with a different package manager than expected.🔧 Proposed fix to detect package manager based on cwd
export const runForgeCommandAsync = ( command: string, opts: { silenceError?: boolean; env?: NodeJS.ProcessEnv; cwd?: string } = { silenceError: false, } ): Promise<{ stdout: string; stderr: string }> => { - const pmc = getPackageManagerCommand(); + const cwd = opts.cwd ?? tmpProjPath(); + const pmc = getPackageManagerCommand(detectPackageManager(cwd)); return new Promise((resolve, reject) => { exec( `${pmc.exec} forge ${command}`, { - cwd: opts.cwd ?? tmpProjPath(), + cwd, env: getCommandEnv(opts.env), },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/nx-forge-e2e/tests/utils/async-commands.ts` around lines 100 - 106, The package manager detection in runForgeCommandAsync is inconsistent because pmc is obtained via getPackageManagerCommand() without considering the caller-provided opts.cwd; update runForgeCommandAsync to compute the package manager using the same cwd logic as runNxCommandAsync (i.e., call getPackageManagerCommand(opts.cwd ?? tmpProjPath())), so pmc reflects the correct project directory before building the exec string that uses getCommandEnv(opts.env) and opts.cwd.
🧹 Nitpick comments (12)
docs/.vitepress/grammars/jsonata.tmLanguage.json (1)
75-84: Inconsistent scope name for single-line comments.The scope name uses
.jssuffix instead of.jsonatafor consistency with the rest of the grammar.🔧 Proposed fix
{ "captures": { "1": { "name": "punctuation.definition.comment.jsonata" } }, "match": "(//).*$\\n?", - "name": "comment.line.double-slash.js" + "name": "comment.line.double-slash.jsonata" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/.vitepress/grammars/jsonata.tmLanguage.json` around lines 75 - 84, The grammar's single-line comment token currently uses the scope "comment.line.double-slash.js" which is inconsistent with the rest of the JSONata grammar; change the scope name to use the .jsonata suffix (e.g., "comment.line.double-slash.jsonata") so it matches the existing "punctuation.definition.comment.jsonata" capture and the rest of the grammar; update the "name" value in the rule that contains the "match": "(//).*$\\n?" and the "captures" entry to use the .jsonata scope.docs/.vitepress/config.mts (1)
1-3: Unused import:resolvefromnode:path.The
resolvefunction is imported but never used in this file. Consider removing it to keep imports clean.🔧 Proposed fix
import { readFileSync } from 'node:fs'; import { fileURLToPath } from 'node:url'; -import { resolve } from 'node:path';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/.vitepress/config.mts` around lines 1 - 3, The import list includes an unused symbol `resolve` from 'node:path'—remove `resolve` from the import declaration (the line importing readFileSync, fileURLToPath, resolve) so only the used symbols (readFileSync and fileURLToPath) are imported to clean up unused imports.tools/docs/eslint.config.mjs (2)
33-44: Consider extracting the shared parser import.The
jsonc-eslint-parseris dynamically imported twice (lines 34 and 43). You could extract it to a shared constant to avoid duplicate imports.🔧 Proposed optimization
import baseConfig from '../../eslint.config.mjs'; + +const jsoncParser = await import('jsonc-eslint-parser'); export default [ ...baseConfig, // ... other configs ... { files: ['**/*.json'], rules: { '@nx/dependency-checks': [ 'error', { ignoredFiles: ['{projectRoot}/eslint.config.{js,cjs,mjs}'], }, ], }, languageOptions: { - parser: await import('jsonc-eslint-parser'), + parser: jsoncParser, }, }, { files: ['./package.json'], rules: { '@nx/nx-plugin-checks': 'error', }, languageOptions: { - parser: await import('jsonc-eslint-parser'), + parser: jsoncParser, }, }, ];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/docs/eslint.config.mjs` around lines 33 - 44, Extract the repeated dynamic import of 'jsonc-eslint-parser' into a shared constant (e.g., jsoncParser) and reuse it in both languageOptions.parser entries to avoid duplicate imports; update the two locations that currently set languageOptions.parser: await import('jsonc-eslint-parser') so they reference the shared constant instead.
4-7: Duplicate ignores pattern.The ignores for
**/distand**/out-tscon lines 4-6 duplicate what's already defined inbaseConfig(from rooteslint.config.mjsline 14). SincebaseConfigis spread after, its ignores will also apply. Consider removing the local ignores block unless there's a specific ordering requirement.🔧 Proposed simplification
export default [ - { - ignores: ['**/dist', '**/out-tsc'], - }, ...baseConfig,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/docs/eslint.config.mjs` around lines 4 - 7, The local ignores block in tools/docs/eslint.config.mjs duplicates patterns already present in baseConfig; remove the local ignores: ['**/dist', '**/out-tsc'] (or the entire ignores object) so you only spread ...baseConfig, or if you need to keep it for ordering, merge uniqued patterns into baseConfig instead—update the configuration around the ignores property and the spread of baseConfig to eliminate the duplicate patterns while keeping the effective ignore list identical.eslint.config.mjs (1)
1-10:FlatCompatis imported but not used in the configuration.The
compatinstance created fromFlatCompatis never used in the exported configuration array. If this is intentional scaffolding for future compatibility shims, consider adding a comment; otherwise, remove the unused import and instantiation.🔧 Proposed fix to remove unused code
-import { FlatCompat } from '@eslint/eslintrc'; -import { dirname } from 'path'; -import { fileURLToPath } from 'url'; import js from '@eslint/js'; import nx from '@nx/eslint-plugin'; - -const compat = new FlatCompat({ - baseDirectory: dirname(fileURLToPath(import.meta.url)), - recommendedConfig: js.configs.recommended, -});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eslint.config.mjs` around lines 1 - 10, The file creates a FlatCompat instance named compat but never uses it; either remove the unused import and the const compat = new FlatCompat(...) line (and the FlatCompat import) or wire compat into the exported ESLint configuration depending on intent; locate the FlatCompat import and the compat variable in eslint.config.mjs and either delete both or replace the unused instantiation with code that applies compat to the exported config so the symbol FlatCompat/compat is actually used.docs/reference/executors.md (1)
85-89: Consider updating tunnel command syntax for consistency.The tunnel executor documentation on line 88 still uses the shorthand syntax
nx tunnel <nx-forge-app-name>, while all other executors in this document were updated to usenx run <project>:<target>. For consistency, consider updating to:nx run <nx-forge-app-name>:tunnel🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/reference/executors.md` around lines 85 - 89, Update the Tunnel executor example to use the consistent "nx run" syntax: replace the shorthand example command `nx tunnel <nx-forge-app-name>` with `nx run <nx-forge-app-name>:tunnel` in the docs; ensure the code block under the "Tunnel" heading and any references to the `tunnel` executor reflect this `nx run <project>:<target>` pattern.nx.json (2)
26-31: Consider removing legacy.eslintrc.jsonreference after migration.The lint inputs include both
{workspaceRoot}/.eslintrc.jsonand{workspaceRoot}/eslint.config.mjs. Since the PR removes the legacy.eslintrc.jsonfiles in favor of flat configs, the old reference on line 29 can be removed to clean up the configuration.🔧 Proposed cleanup
"@nx/eslint:lint": { - "inputs": [ - "default", - "{workspaceRoot}/.eslintrc.json", - "{workspaceRoot}/eslint.config.mjs" - ], + "inputs": ["default", "{workspaceRoot}/eslint.config.mjs"], "cache": true },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nx.json` around lines 26 - 31, Remove the legacy `.eslintrc.json` reference from the "@nx/eslint:lint" inputs array: delete the "{workspaceRoot}/.eslintrc.json" entry so the inputs only reference the flat config "{workspaceRoot}/eslint.config.mjs" (update the inputs array in the "@nx/eslint:lint" block accordingly).
48-51: Same cleanup opportunity for production inputs.Similar to the lint inputs, the production exclusion still references
{projectRoot}/.eslintrc.jsonon line 48, which can be removed if all legacy ESLint configs have been deleted.🔧 Proposed cleanup
"production": [ "default", "!{projectRoot}/**/?(*.)+(spec|test).[jt]s?(x)?(.snap)", "!{projectRoot}/tsconfig.spec.json", "!{projectRoot}/jest.config.[jt]s", - "!{projectRoot}/.eslintrc.json", "!{projectRoot}/src/test-setup.[jt]s", "!{projectRoot}/eslint.config.mjs" ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nx.json` around lines 48 - 51, Remove the stale exclusion entry "!{projectRoot}/.eslintrc.json" from the production inputs array in nx.json (the same cleanup applied to lint inputs); locate the production inputs list and delete the string "!{projectRoot}/.eslintrc.json" so the config no longer references the legacy ESLint file.packages/nx-forge/eslint.config.mjs (2)
19-28: Consider removing empty placeholder rule blocks.These override blocks with empty
rules: {}don't modify any behavior. If they're intentional extension points for future use, consider adding a more descriptive comment; otherwise, they can be removed to reduce noise.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nx-forge/eslint.config.mjs` around lines 19 - 28, The two override blocks that target files ['**/*.ts','**/*.tsx'] and ['**/*.js','**/*.jsx'] only contain empty rules objects and thus have no effect; either remove these override entries entirely from the ESLint config (the objects containing files: [...] and rules: {}) or replace the empty rules with a short explanatory comment indicating they are intentional placeholders for future rule customizations so reviewers understand they’re deliberate.
39-55: Consider extracting the duplicated parser import.The
await import('jsonc-eslint-parser')is called twice (lines 40 and 54). Extract it to a single variable to avoid redundancy.♻️ Proposed refactor
import baseConfig from '../../eslint.config.mjs'; +const jsoncParser = await import('jsonc-eslint-parser'); + export default [ { ignores: ['**/dist', '**/out-tsc'], }, ...baseConfig, // ... other configs ... { files: [ './package.json', './generators.json', './executors.json', './migrations.json', ], rules: { '@nx/nx-plugin-checks': 'error', }, languageOptions: { - parser: await import('jsonc-eslint-parser'), + parser: jsoncParser, }, }, { files: ['./package.json'], rules: { '@nx/dependency-checks': [ 'error', { ignoredDependencies: ['nx'], }, ], }, languageOptions: { - parser: await import('jsonc-eslint-parser'), + parser: jsoncParser, }, }, ];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nx-forge/eslint.config.mjs` around lines 39 - 55, There are two identical dynamic imports for jsonc-eslint-parser in the languageOptions blocks; extract the import into a single variable (e.g., jsoncParser) and reuse that variable in both languageOptions.parser entries instead of calling await import('jsonc-eslint-parser') twice, updating the languageOptions.parser references accordingly so both blocks reference the same imported parser.packages/nx-forge/src/generators/application/generator.spec.ts (2)
342-346: Redundant assertion on line 346.The
hasJsonValueassertion on lines 331-339 already verifies thatexcludecontains'jest.config.cts'. Line 346 duplicates this check.♻️ Proposed fix to remove redundant assertion
const nestedTsconfigApp = readJson( tree, 'my-dir/my-forge-app/tsconfig.app.json' ); - expect(nestedTsconfigApp.exclude).toContain('jest.config.cts'); expect(tree.exists(eslintConfigPath('my-dir/my-forge-app'))).toBeTruthy();Note: If you want to keep the explicit read for clarity, consider removing it from the
hasJsonValuelist instead to avoid testing the same thing twice.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nx-forge/src/generators/application/generator.spec.ts` around lines 342 - 346, The test contains a redundant assertion: remove the duplicate expect that checks nestedTsconfigApp.exclude contains 'jest.config.cts'. Specifically, drop or comment out the expect(...) line that reads the JSON into nestedTsconfigApp and asserts .exclude includes 'jest.config.cts' (the explicit expect using nestedTsconfigApp), since hasJsonValue already verifies this; keep the hasJsonValue check and, if you prefer keeping the readJson for clarity, remove 'tsconfig.app.json' related entry from the hasJsonValue assertions instead to avoid duplicate verification.
161-169: Redundant assertion on line 169.The
toContain('jest.config.cts')check is redundant since lines 161-168 already assert the exact contents of theexcludearray, which includes'jest.config.cts'.♻️ Proposed fix to remove redundant assertion
expect(tsconfigApp.exclude.sort()).toEqual( [ 'jest.config.ts', 'jest.config.cts', 'src/**/*.spec.ts', 'src/**/*.test.ts', ].sort() ); - expect(tsconfigApp.exclude).toContain('jest.config.cts');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nx-forge/src/generators/application/generator.spec.ts` around lines 161 - 169, The test contains a redundant assertion: remove the extra expect that checks for 'jest.config.cts' since tsconfigApp.exclude is already asserted exactly in the preceding expect; specifically delete the line with expect(tsconfigApp.exclude).toContain('jest.config.cts') in generator.spec.ts so the test only verifies the full exclude array (the expect(...).toEqual(...) assertion remains).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.codex/config.toml:
- Around line 6-8: The config entry writable_roots = ["~/.npmrc"] uses an
unexpanded tilde and will be treated literally by Codex; replace it with a
workspace-local .npmrc (e.g., writable_roots = ["./.npmrc"]) so Nx's
startLocalRegistry helper writes auth tokens to the project root rather than the
user home, or if you intentionally must target the user home, change it to an
absolute home path (e.g., /Users/USERNAME/.npmrc) instead of "~/.npmrc"; update
the writable_roots value accordingly and ensure any CI/dev docs reflect the
chosen pattern.
In `@package.json`:
- Around line 47-50: The package.json has a version mismatch: update the
dependency "jest-environment-node" to match Jest's version (set it to "30.0.5"
to align with "jest" and "jest-environment-jsdom"); after changing
"jest-environment-node" update the lockfile (npm install or yarn install) and
run the test suite to verify no environment-related failures. Refer to the
manifest entries "jest", "jest-environment-jsdom", and "jest-environment-node"
when making the change.
---
Outside diff comments:
In `@e2e/nx-forge-e2e/tests/utils/async-commands.ts`:
- Around line 100-106: The package manager detection in runForgeCommandAsync is
inconsistent because pmc is obtained via getPackageManagerCommand() without
considering the caller-provided opts.cwd; update runForgeCommandAsync to compute
the package manager using the same cwd logic as runNxCommandAsync (i.e., call
getPackageManagerCommand(opts.cwd ?? tmpProjPath())), so pmc reflects the
correct project directory before building the exec string that uses
getCommandEnv(opts.env) and opts.cwd.
---
Nitpick comments:
In `@docs/.vitepress/config.mts`:
- Around line 1-3: The import list includes an unused symbol `resolve` from
'node:path'—remove `resolve` from the import declaration (the line importing
readFileSync, fileURLToPath, resolve) so only the used symbols (readFileSync and
fileURLToPath) are imported to clean up unused imports.
In `@docs/.vitepress/grammars/jsonata.tmLanguage.json`:
- Around line 75-84: The grammar's single-line comment token currently uses the
scope "comment.line.double-slash.js" which is inconsistent with the rest of the
JSONata grammar; change the scope name to use the .jsonata suffix (e.g.,
"comment.line.double-slash.jsonata") so it matches the existing
"punctuation.definition.comment.jsonata" capture and the rest of the grammar;
update the "name" value in the rule that contains the "match": "(//).*$\\n?" and
the "captures" entry to use the .jsonata scope.
In `@docs/reference/executors.md`:
- Around line 85-89: Update the Tunnel executor example to use the consistent
"nx run" syntax: replace the shorthand example command `nx tunnel
<nx-forge-app-name>` with `nx run <nx-forge-app-name>:tunnel` in the docs;
ensure the code block under the "Tunnel" heading and any references to the
`tunnel` executor reflect this `nx run <project>:<target>` pattern.
In `@eslint.config.mjs`:
- Around line 1-10: The file creates a FlatCompat instance named compat but
never uses it; either remove the unused import and the const compat = new
FlatCompat(...) line (and the FlatCompat import) or wire compat into the
exported ESLint configuration depending on intent; locate the FlatCompat import
and the compat variable in eslint.config.mjs and either delete both or replace
the unused instantiation with code that applies compat to the exported config so
the symbol FlatCompat/compat is actually used.
In `@nx.json`:
- Around line 26-31: Remove the legacy `.eslintrc.json` reference from the
"@nx/eslint:lint" inputs array: delete the "{workspaceRoot}/.eslintrc.json"
entry so the inputs only reference the flat config
"{workspaceRoot}/eslint.config.mjs" (update the inputs array in the
"@nx/eslint:lint" block accordingly).
- Around line 48-51: Remove the stale exclusion entry
"!{projectRoot}/.eslintrc.json" from the production inputs array in nx.json (the
same cleanup applied to lint inputs); locate the production inputs list and
delete the string "!{projectRoot}/.eslintrc.json" so the config no longer
references the legacy ESLint file.
In `@packages/nx-forge/eslint.config.mjs`:
- Around line 19-28: The two override blocks that target files
['**/*.ts','**/*.tsx'] and ['**/*.js','**/*.jsx'] only contain empty rules
objects and thus have no effect; either remove these override entries entirely
from the ESLint config (the objects containing files: [...] and rules: {}) or
replace the empty rules with a short explanatory comment indicating they are
intentional placeholders for future rule customizations so reviewers understand
they’re deliberate.
- Around line 39-55: There are two identical dynamic imports for
jsonc-eslint-parser in the languageOptions blocks; extract the import into a
single variable (e.g., jsoncParser) and reuse that variable in both
languageOptions.parser entries instead of calling await
import('jsonc-eslint-parser') twice, updating the languageOptions.parser
references accordingly so both blocks reference the same imported parser.
In `@packages/nx-forge/src/generators/application/generator.spec.ts`:
- Around line 342-346: The test contains a redundant assertion: remove the
duplicate expect that checks nestedTsconfigApp.exclude contains
'jest.config.cts'. Specifically, drop or comment out the expect(...) line that
reads the JSON into nestedTsconfigApp and asserts .exclude includes
'jest.config.cts' (the explicit expect using nestedTsconfigApp), since
hasJsonValue already verifies this; keep the hasJsonValue check and, if you
prefer keeping the readJson for clarity, remove 'tsconfig.app.json' related
entry from the hasJsonValue assertions instead to avoid duplicate verification.
- Around line 161-169: The test contains a redundant assertion: remove the extra
expect that checks for 'jest.config.cts' since tsconfigApp.exclude is already
asserted exactly in the preceding expect; specifically delete the line with
expect(tsconfigApp.exclude).toContain('jest.config.cts') in generator.spec.ts so
the test only verifies the full exclude array (the expect(...).toEqual(...)
assertion remains).
In `@tools/docs/eslint.config.mjs`:
- Around line 33-44: Extract the repeated dynamic import of
'jsonc-eslint-parser' into a shared constant (e.g., jsoncParser) and reuse it in
both languageOptions.parser entries to avoid duplicate imports; update the two
locations that currently set languageOptions.parser: await
import('jsonc-eslint-parser') so they reference the shared constant instead.
- Around line 4-7: The local ignores block in tools/docs/eslint.config.mjs
duplicates patterns already present in baseConfig; remove the local ignores:
['**/dist', '**/out-tsc'] (or the entire ignores object) so you only spread
...baseConfig, or if you need to keep it for ordering, merge uniqued patterns
into baseConfig instead—update the configuration around the ignores property and
the spread of baseConfig to eliminate the duplicate patterns while keeping the
effective ignore list identical.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cef6d0fe-983a-4bd4-b5bb-590cdf1de712
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (56)
.codex/config.toml.eslintignore.eslintrc.json.gitignoreAGENTS.mddocs/.vitepress/config.mtsdocs/.vitepress/grammars/jsonata.tmLanguage.jsondocs/concepts/plugin-concepts.mddocs/guides/adding-a-ui-kit-module.mddocs/guides/generating-a-forge-app.mddocs/guides/getting-started.mddocs/guides/migrating-to-inferred-tasks.mddocs/guides/migrating-to-package-executor.mddocs/guides/transforming-the-manifest.mddocs/reference/executors.mde2e/nx-forge-e2e/tests/application.generator.spec.tse2e/nx-forge-e2e/tests/basic-setup.spec.tse2e/nx-forge-e2e/tests/build.executor.spec.tse2e/nx-forge-e2e/tests/utils/async-commands.tse2e/nx-forge-e2e/tests/utils/generate-forge-app.tseslint.config.mjsnx.jsonpackage.jsonpackages/nx-forge/.eslintrc.jsonpackages/nx-forge/eslint.config.mjspackages/nx-forge/executors.jsonpackages/nx-forge/jest.config.tspackages/nx-forge/package.jsonpackages/nx-forge/project.jsonpackages/nx-forge/src/executors/build/executor.tspackages/nx-forge/src/executors/build/lib/compile-webpack.tspackages/nx-forge/src/executors/build/lib/generate-package-json.tspackages/nx-forge/src/executors/build/lib/normalize-options.tspackages/nx-forge/src/executors/build/schema.d.tspackages/nx-forge/src/executors/build/schema.jsonpackages/nx-forge/src/executors/install/executor.tspackages/nx-forge/src/executors/install/schema.d.tspackages/nx-forge/src/executors/install/schema.jsonpackages/nx-forge/src/executors/package/executor.tspackages/nx-forge/src/executors/package/lib/copy-forge-app-assets.tspackages/nx-forge/src/executors/package/lib/create-package-json.tspackages/nx-forge/src/executors/package/lib/patch-manifest-yml.tspackages/nx-forge/src/executors/package/lib/process-resource-dependencies.tspackages/nx-forge/src/generators/application/files/README.mdpackages/nx-forge/src/generators/application/files/manifest.ymlpackages/nx-forge/src/generators/application/files/tsconfig.app.jsonpackages/nx-forge/src/generators/application/generator.spec.tspackages/nx-forge/src/graph/create-nodes.tspackages/nx-forge/src/migrations/update-2-3-0/add-nx-forge-plugin.tspackages/nx-forge/src/migrations/update-2-3-0/remove-implicit-custom-ui-dependencies.tspackages/nx-forge/src/migrations/update-3-0-0-webpack-config-setup/update-3-0-0-webpack-config-setup.tspackages/nx-forge/src/shared/targets/get-install-config.tstools/docs/.eslintrc.jsontools/docs/eslint.config.mjstools/docs/src/lib/reference-docs.spec.tstools/scripts/start-local-registry.ts
💤 Files with no reviewable changes (22)
- .eslintignore
- packages/nx-forge/src/executors/package/lib/copy-forge-app-assets.ts
- packages/nx-forge/jest.config.ts
- packages/nx-forge/src/graph/create-nodes.ts
- packages/nx-forge/src/migrations/update-2-3-0/add-nx-forge-plugin.ts
- packages/nx-forge/src/executors/package/lib/create-package-json.ts
- e2e/nx-forge-e2e/tests/build.executor.spec.ts
- packages/nx-forge/src/shared/targets/get-install-config.ts
- .eslintrc.json
- packages/nx-forge/src/migrations/update-2-3-0/remove-implicit-custom-ui-dependencies.ts
- packages/nx-forge/executors.json
- tools/docs/.eslintrc.json
- packages/nx-forge/src/executors/install/executor.ts
- packages/nx-forge/src/executors/build/schema.json
- packages/nx-forge/src/executors/build/lib/compile-webpack.ts
- packages/nx-forge/src/executors/build/lib/normalize-options.ts
- packages/nx-forge/src/executors/install/schema.json
- packages/nx-forge/src/executors/build/executor.ts
- packages/nx-forge/.eslintrc.json
- packages/nx-forge/src/executors/build/schema.d.ts
- packages/nx-forge/src/executors/install/schema.d.ts
- packages/nx-forge/src/executors/build/lib/generate-package-json.ts
Comment on lines
+6
to
+8
| # Nx's startLocalRegistry helper shells out to `npm config set/delete ... --ws=false`, | ||
| # which temporarily writes the local registry auth token to the user npm config at ~/.npmrc. | ||
| writable_roots = ["~/.npmrc"] |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For Codex .codex/config.toml, does writable_roots = ["~/.npmrc"]support tilde expansion, or must paths be absolute? Also, what is the recommended secure pattern to allow temporary npm auth for Nx local registry helpers without writing to user-home.npmrc?
💡 Result:
For Codex .codex/config.toml, writable_roots = ["~/.npmrc"] does not support tilde expansion; paths must be absolute. Official docs and examples consistently use absolute paths like ["/Users/YOU/.pyenv/shims"] or ["/additional/writable/path"]. A related GitHub issue confirms that Codex tools like Edit do not expand ~, treating it literally, which aligns with config path handling requiring explicit expansion or absolute paths. The recommended secure pattern for temporary npm auth with Nx local registry helpers, without writing to user-home .npmrc, is to use project-level .npmrc in the workspace root. Nx docs state this configures custom registries and auth tokens for publishing/installing, scoped by npm scopes (e.g., @my-scope:registry=... //my-custom-registry.com/:_authToken=). This avoids global user config, keeping auth local to the project. Environment variables like NPM_TOKEN can also inject auth temporarily via CI or shell_environment_policy in Codex config, but project .npmrc is the explicit Nx recommendation.
Citations:
- 1: https://developers.openai.com/codex/config-advanced/
- 2: https://developers.openai.com/codex/config-reference/
- 3: https://gist.github.com/Zochory/d3df095db0c6f02643b83266efabaf5e
- 4: https://developers.openai.com/codex/config-sample/
- 5: https://developers.openai.com/codex/local-config
- 6: https://developers.openai.com/codex/concepts/sandboxing/
- 7: https://www.oneida-nsn.gov/education/vocational-rehabilitation
- 8: https://openai-codex.mintlify.app/concepts/sandboxing
- 9: https://developers.openai.com/codex/config-advanced
- 10: https://developers.openai.com/codex/config-sample
- 11: https://nx.dev/docs/guides/nx-release/configure-custom-registries
Replace user-home .npmrc reference with workspace-local .npmrc and use absolute path if keeping this approach.
The writable_roots = ["~/.npmrc"] path will not work as intended: Codex does not expand tilde in writable_roots, so it will treat ~/.npmrc literally rather than resolving to the user's home directory. This is both a config bug and a security concern.
The recommended approach is to use a workspace-local .npmrc in the project root instead. This allows Nx's local registry helper to configure auth tokens scoped to the project without writing to user-home config, eliminating cross-run/cross-user persistence risks. If you must write to user-home ~/.npmrc, use an absolute path like /Users/USERNAME/.npmrc—but the workspace-local .npmrc pattern is safer and aligns with Nx documentation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.codex/config.toml around lines 6 - 8, The config entry writable_roots =
["~/.npmrc"] uses an unexpanded tilde and will be treated literally by Codex;
replace it with a workspace-local .npmrc (e.g., writable_roots = ["./.npmrc"])
so Nx's startLocalRegistry helper writes auth tokens to the project root rather
than the user home, or if you intentionally must target the user home, change it
to an absolute home path (e.g., /Users/USERNAME/.npmrc) instead of "~/.npmrc";
update the writable_roots value accordingly and ensure any CI/dev docs reflect
the chosen pattern.
Comment on lines
+47
to
+50
| "jest": "30.0.5", | ||
| "jest-environment-jsdom": "30.0.5", | ||
| "jest-environment-node": "^29.4.1", | ||
| "nx": "20.3.1", | ||
| "jest-util": "30.0.5", |
There was a problem hiding this comment.
Version mismatch: jest-environment-node is at ^29.4.1 while Jest is at 30.0.5.
The jest-environment-jsdom is correctly updated to 30.0.5, but jest-environment-node remains at ^29.4.1. This version mismatch may cause test failures or unexpected behavior when tests use the Node.js environment.
Proposed fix
"jest": "30.0.5",
"jest-environment-jsdom": "30.0.5",
- "jest-environment-node": "^29.4.1",
+ "jest-environment-node": "30.0.5",
"jest-util": "30.0.5",📝 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.
Suggested change
| "jest": "30.0.5", | |
| "jest-environment-jsdom": "30.0.5", | |
| "jest-environment-node": "^29.4.1", | |
| "nx": "20.3.1", | |
| "jest-util": "30.0.5", | |
| "jest": "30.0.5", | |
| "jest-environment-jsdom": "30.0.5", | |
| "jest-environment-node": "30.0.5", | |
| "jest-util": "30.0.5", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 47 - 50, The package.json has a version mismatch:
update the dependency "jest-environment-node" to match Jest's version (set it to
"30.0.5" to align with "jest" and "jest-environment-jsdom"); after changing
"jest-environment-node" update the lockfile (npm install or yarn install) and
run the test suite to verify no environment-related failures. Refer to the
manifest entries "jest", "jest-environment-jsdom", and "jest-environment-node"
when making the change.
Contributor
|
🎉 This PR is included in version 7.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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.
feat(nx): update to Nx 22
BREAKING CHANGE: requires Nx 22, removed build and install executors
Closes #181
Summary by CodeRabbit
New Features
Deprecated/Removed
buildexecutorinstallexecutorDocumentation
Dependencies
Tests