-
-
Notifications
You must be signed in to change notification settings - Fork 10.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgraded NX to version 20 #22258
base: main
Are you sure you want to change the base?
Upgraded NX to version 20 #22258
Conversation
WalkthroughThe pull request updates the project's configuration and environment scripts across multiple files. The 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
💤 Files with no reviewable changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (16)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
fb0e045
to
94dd09d
Compare
@@ -316,7 +316,6 @@ async function handleStripe() { | |||
debug('at least one command provided'); | |||
|
|||
debug('resetting nx'); | |||
process.env.NX_DISABLE_DB = "true"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this line, NX kept reverting to the "legacy" cache and logging a warning. I haven't seen any issue with removing this
@@ -17,7 +17,6 @@ env: | |||
~/.cache/ms-playwright/ | |||
CACHED_BUILD_PATHS: | | |||
${{ github.workspace }}/ghost/*/build | |||
NX_REJECT_UNKNOWN_LOCAL_CACHE: 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This option is no longer needed (or supported) with the database backed cache, see https://nx.dev/deprecated/legacy-cache
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
package.json (1)
121-123
: Lint-Staged Configuration:
Reintroducing lint-staged for*.js
files is a useful step to ensure code quality before commits. Consider later expanding this configuration to include additional file types (such as TypeScript) if your project grows in that direction.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (3)
.github/scripts/dev.js
(0 hunks).github/workflows/ci.yml
(0 hunks)package.json
(1 hunks)
💤 Files with no reviewable changes (2)
- .github/scripts/dev.js
- .github/workflows/ci.yml
🔇 Additional comments (10)
package.json (10)
1-8
: Metadata and Project Info:
The metadata section (lines 1–8) is clearly defined with proper project details. The use of"ghost-monorepo"
and version"0.0.0-private"
aligns with internal release conventions.
9-12
: Workspaces Configuration:
The workspaces array is maintained accurately, ensuring that the project structure remains well defined for package management.
13-18
: Monorepo Configuration:
The monorepo section is well structured with clear fields for public visibility, internal package settings, repository URL, and scope. This organization supports the overall monorepo setup effectively.
19-21
: ESLint Ignore Settings:
The ESLint ignore configuration is concise and covers the necessary patterns (e.g., ignoringnode_modules
). This setup should help maintain consistent linting outcomes.
22-57
: Scripts Section:
The scripts block has been comprehensively reorganized and continues to cover building, testing, and deployment workflows. Given the upgrade to NX v20, please verify that commands (such asnx run
andnx run-many
) work as expected under the new version. Running end-to-end tests in CI would help confirm compatibility with the enhanced cache features.
58-64
: Resolutions Section:
The resolutions are clearly stated and remain consistent. No issues are apparent here, and the block effectively locks dependency versions as needed.
65-83
: Renovate Configuration Enhancements:
The renovate section now includes a"rebaseWhen": "never"
property and an expanded"ignoreDeps"
list. These changes aim to refine automated dependency updates. It’s important to verify that these configuration updates align with your dependency management strategy and do not inadvertently skip critical updates.
84-87
: Renovate Ignore Paths:
The newly added"ignorePaths"
array excludes directories like"test"
and the specificpackage.json
inghost/admin/lib/koenig-editor
, which should reduce noise from unnecessary dependency scanning.
88-119
: Renovate Package Rules:
The introduction of detailed package rules—for example, grouping Ember-related and CSS-related packages—improves dependency management granularity. Please confirm that the rule disabling updates for some CSS packages is intentional, ensuring that it won’t block essential security or functionality updates.
124-138
: DevDependencies and NX Upgrade:
The critical upgrade here is the update of"nx"
from version"19.8.14"
to"20.4.6"
. Since NX v20 introduces a new database-backed cache and has improvements targeting Docker usage, please perform thorough validation to ensure that none of the existing workflows (both local and CI) are disrupted by this change.
e4dc0e5
to
17bf6ac
Compare
ref https://nx.dev/blog/announcing-nx-20