Skip to content

Conversation

@nottuf
Copy link

@nottuf nottuf commented Dec 19, 2025

解决了@achrinza/node-ipc不兼容node23的问题
修复了webpack持久缓存

Summary by CodeRabbit

  • Documentation

    • Added guidance for a compatibility workaround in engine‑strict mode, showing npm/pnpm override usage.
  • Bug Fixes

    • Project creation now auto-injects a package‑manager-specific compatibility override during install without breaking the creation flow.
    • Changed which config file is watched for dependency caching to improve rebuild correctness.
  • Improvements

    • Build/serve output now respects per‑project stats settings and disables colored output in CI.
    • PostCSS setup conditionally omits autoprefixer for native targets.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 19, 2025

📝 Walkthrough

Walkthrough

Adds a creation-time install event handler to inject package-manager-specific overrides mapping @achrinza/node-ipcnpm:node-ipc-compat@1.0.0; adjusts vue-loader resolution and watched config file; threads plugin options into webpack result handling and makes stats rendering and color output configurable.

Changes

Cohort / File(s) Summary
Documentation
README.md
Adds compatibility workaround docs and JSON snippets showing @achrinza/node-ipcnpm:node-ipc-compat@1.0.0 overrides for pnpm and npm.
CLI: create & override injection
packages/mpx-cli/lib/create.js
Detects package manager (options.packageManager, loadOptions(), hasYarn, hasPnpm3OrLater) and registers an install-event handler that reads targetDir/package.json, injects a pnpm or npm override mapping @achrinza/node-ipcnpm:node-ipc-compat@1.0.0, and writes back inside try/catch; imports updated.
Config resolution & watched dependency
packages/vue-cli-plugin-mpx/config/base.js
Resolves vue-loader-v15 via the Vue CLI service path at runtime and changes the build cache watched dependency from vue.config.js to mpx.config.js.
Webpack/result handling & stats options
packages/vue-cli-plugin-mpx/utils/webpack.js, packages/vue-cli-plugin-mpx/utils/output.js
Expands handleWebpackDone(err, stats, watch)handleWebpackDone(err, stats, watch, options = {}); derives statsOptions from options.pluginOptions?.mpx?.stats and passes them to extractResultFromStats; extractResultFromStats/output now conditionally enables colors based on !process.env.CI.
Call-site and import updates
packages/vue-cli-plugin-mpx-ssr/commands/build/index.js, packages/vue-cli-plugin-mpx/commands/build/index.js, packages/vue-cli-plugin-mpx/commands/serve/*.js
Consolidates imports (e.g., getCurrentTarget) and updates call sites to pass the new options argument into handleWebpackDone; promise flows unchanged.
Generator template change
packages/vue-cli-plugin-mpx/generator/base/template/postcss.config.js
Makes autoprefixer conditional: excluded for RN targets (android, ios, harmony) via MPX_CURRENT_TARGET_MODE, and returns a filtered plugins array.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Dev as Developer
  participant CLI as mpx-cli (create)
  participant Events as Installer Events
  participant FS as targetDir/package.json
  participant PM as Package Manager

  Dev->>CLI: start project creation
  CLI->>CLI: resolve packageManager (options, loadOptions, hasYarn, hasPnpm3OrLater)
  CLI->>Events: register install-event handler
  Events->>CLI: install event fires (plugins-install / deps-install)
  CLI->>FS: read package.json
  CLI->>CLI: inject override `@achrinza/node-ipc` → `npm:node-ipc-compat@1.0.0` (pnpm or npm format)
  CLI->>FS: write updated package.json (try/catch)
  FS->>PM: package.json used during install
  PM->>FS: install respects override
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • mater1996

Poem

🐰 I hopped through JSON, neat and spry,

Swapped a package with a careful eye,
The creator whispered, then made the tweak,
Builds hummed softly, no need to peek,
I munched a carrot and waved goodbye 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title is partially related to the changeset, addressing @achrinza/node-ipc compatibility and webpack cache issues, but uses Chinese text and abbreviated phrasing that may not be immediately clear to all English-speaking reviewers.
✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 739c4da and 904acf7.

📒 Files selected for processing (2)
  • packages/vue-cli-plugin-mpx/commands/serve/web.js
  • packages/vue-cli-plugin-mpx/utils/webpack.js
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/vue-cli-plugin-mpx/utils/webpack.js
🧰 Additional context used
🧬 Code graph analysis (1)
packages/vue-cli-plugin-mpx/commands/serve/web.js (1)
packages/vue-cli-plugin-mpx/index.js (1)
  • options (9-9)
🔇 Additional comments (1)
packages/vue-cli-plugin-mpx/commands/serve/web.js (1)

275-278: Verify the property override order for assets configuration.

The spread operator ...(options.pluginOptions?.mpx?.stats || {}) comes after assets: false, which means user-provided configuration can override the assets: false setting. If a user sets options.pluginOptions.mpx.stats.assets = true, the assets will be displayed despite the default being false.

If the intention is to always keep assets: false regardless of user configuration, reverse the order:

-      result.push('', extractResultFromStats(stats, {
-        assets: false,
-        ...(options.pluginOptions?.mpx?.stats || {})
-      }))
+      result.push('', extractResultFromStats(stats, {
+        ...(options.pluginOptions?.mpx?.stats || {}),
+        assets: false
+      }))

If the current behavior (allowing user override) is intentional, then the code is correct as-is.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
packages/mpx-cli/lib/create.js (1)

207-207: Consider logging errors instead of silently swallowing them.

The empty catch block silently ignores any errors during package.json manipulation. While this prevents the creation flow from failing, it leaves users unaware if the Node 23 compatibility fix wasn't applied.

🔎 Proposed improvement
-      } catch (e) {}
+      } catch (e) {
+        // Non-fatal: log but don't fail project creation
+        console.warn('Warning: Failed to inject @achrinza/node-ipc override:', e.message)
+      }
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3059d58 and 643a141.

📒 Files selected for processing (3)
  • README.md (1 hunks)
  • packages/mpx-cli/lib/create.js (2 hunks)
  • packages/vue-cli-plugin-mpx/config/base.js (1 hunks)
🔇 Additional comments (4)
packages/mpx-cli/lib/create.js (2)

10-13: LGTM!

The additional imports for package manager detection are appropriate for the new functionality.


182-186: LGTM!

The package manager detection logic correctly follows the priority chain: explicit option → saved option → yarn → pnpm → npm fallback.

README.md (1)

82-93: LGTM!

The documentation clearly explains the Node 23 compatibility issue and the automatic workaround. The JSON examples for both pnpm and npm match the implementation in create.js.

packages/vue-cli-plugin-mpx/config/base.js (1)

475-475: The implementation properly handles the change from vue.config.js to mpx.config.js. The generator creates mpx.config.js in new projects and removes vue.config.js, while webpack cache invalidation is correctly configured using mpx.config.js in the buildDependencies array at line 475. No further verification is needed.

Likely an incorrect or invalid review comment.

Comment on lines +189 to +209
creator.on('creation', ({ event }) => {
if (event === 'plugins-install' || event === 'deps-install') {
try {
const pkgPath = path.resolve(targetDir, 'package.json')
const pkg = fs.readJsonSync(pkgPath)
// 根据包管理器类型写入对应的 overrides 配置,yarn无需处理
if (packageManager === 'pnpm') {
pkg.pnpm = {
overrides: {
'@achrinza/node-ipc': 'npm:node-ipc-compat@1.0.0'
}
}
} else if (packageManager === 'npm') {
pkg.overrides = {
'@achrinza/node-ipc': 'npm:node-ipc-compat@1.0.0'
}
}
fs.writeJsonSync(pkgPath, pkg, { spaces: 2 })
} catch (e) {}
}
})
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Risk of overwriting existing pnpm/npm configurations.

The code completely replaces pkg.pnpm and pkg.overrides objects, which could delete any existing configurations users may have. This is especially problematic if other CLI plugins or user customizations have already added pnpm or npm overrides.

🔎 Proposed fix to merge instead of replace
         // 根据包管理器类型写入对应的 overrides 配置,yarn无需处理
         if (packageManager === 'pnpm') {
-          pkg.pnpm = {
-            overrides: {
-              '@achrinza/node-ipc': 'npm:node-ipc-compat@1.0.0'
-            }
-          }
+          pkg.pnpm = pkg.pnpm || {}
+          pkg.pnpm.overrides = pkg.pnpm.overrides || {}
+          pkg.pnpm.overrides['@achrinza/node-ipc'] = 'npm:node-ipc-compat@1.0.0'
         } else if (packageManager === 'npm') {
-          pkg.overrides = {
-            '@achrinza/node-ipc': 'npm:node-ipc-compat@1.0.0'
-          }
+          pkg.overrides = pkg.overrides || {}
+          pkg.overrides['@achrinza/node-ipc'] = 'npm:node-ipc-compat@1.0.0'
         }
🤖 Prompt for AI Agents
In packages/mpx-cli/lib/create.js around lines 189 to 209, the code currently
replaces pkg.pnpm and pkg.overrides wholesale which can wipe out existing user
or plugin configurations; instead, read package.json (already done), ensure the
pnpm and overrides objects exist, and merge the new entry into the existing
objects (perform a shallow merge for pkg.pnpm and pkg.overrides, and
specifically merge into pkg.pnpm.overrides if using pnpm) rather than assigning
a new object; create the objects if missing, preserve existing keys, then write
the merged pkg back; also keep the try/catch but consider logging the error
instead of swallowing it.

if (packageManager === 'pnpm') {
pkg.pnpm = {
overrides: {
'@achrinza/node-ipc': 'npm:node-ipc-compat@1.0.0'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

grep -r "node-ipc-compat" --include="*.js" --include="*.json" .

Repository: mpx-ecology/mpx-cli

Length of output: 351


The package node-ipc-compat@1.0.0 does not exist on the NPM registry and will cause installation to fail.

The hardcoded alias to npm:node-ipc-compat@1.0.0 cannot be resolved. Web searches show no published package by this name or version. Similar alternatives like @node-ipc/compat (version 9.2.5) exist but not the referenced package. This dependency must be corrected before the code can be used in production, as any package installation will fail with a resolution error.

🤖 Prompt for AI Agents
In packages/mpx-cli/lib/create.js around line 198 the dependency alias
'@achrinza/node-ipc': 'npm:node-ipc-compat@1.0.0' points to a non-existent
package and will break installs; replace it with a valid package/version (for
example use the published @node-ipc/compat with a concrete version like 9.2.5 or
remove the alias and reference the correct package name directly), update the
lock/manifest if present, and verify by running an install to ensure the package
resolves from the NPM registry.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f7d0b0b and 6d9f5c3.

📒 Files selected for processing (6)
  • packages/vue-cli-plugin-mpx-ssr/commands/build/index.js
  • packages/vue-cli-plugin-mpx/commands/build/index.js
  • packages/vue-cli-plugin-mpx/commands/serve/mp.js
  • packages/vue-cli-plugin-mpx/commands/serve/web.js
  • packages/vue-cli-plugin-mpx/utils/output.js
  • packages/vue-cli-plugin-mpx/utils/webpack.js
🧰 Additional context used
🧬 Code graph analysis (4)
packages/vue-cli-plugin-mpx/commands/build/index.js (4)
packages/vue-cli-plugin-mpx/commands/inspect.js (1)
  • args (19-19)
packages/mpx-cli-service/bin/mpx-cli-service.js (1)
  • args (29-44)
scripts/test.js (1)
  • args (3-3)
packages/vue-cli-plugin-mpx/index.js (1)
  • options (9-9)
packages/vue-cli-plugin-mpx/commands/serve/mp.js (1)
packages/vue-cli-plugin-mpx/index.js (1)
  • options (9-9)
packages/vue-cli-plugin-mpx/utils/webpack.js (1)
packages/vue-cli-plugin-mpx/index.js (1)
  • options (9-9)
packages/vue-cli-plugin-mpx-ssr/commands/build/index.js (3)
packages/vue-cli-plugin-mpx-ssr/commands/serve/serveServer.js (6)
  • require (1-1)
  • require (2-2)
  • require (3-3)
  • require (4-4)
  • require (8-8)
  • target (13-13)
packages/vue-cli-plugin-mpx/commands/build/index.js (1)
  • target (39-39)
packages/vue-cli-plugin-mpx/commands/serve/mp.js (1)
  • target (9-9)
🔇 Additional comments (7)
packages/vue-cli-plugin-mpx/utils/output.js (1)

70-70: LGTM! Colors disabled in CI environments.

The conditional color output (colors: !process.env.CI) correctly disables colors in CI environments while keeping them enabled locally, which is a common best practice for CI/CD pipelines.

packages/vue-cli-plugin-mpx/commands/build/index.js (1)

55-55: LGTM! Consistent with the updated signature.

The addition of the options parameter correctly aligns with the extended handleWebpackDone signature and enables custom stats configuration.

packages/vue-cli-plugin-mpx/utils/webpack.js (2)

4-5: LGTM! Well-designed extension with backward compatibility.

The function signature extension with a default parameter value (options = {}) ensures backward compatibility, and the optional chaining safely extracts stats options from nested properties.


23-23: LGTM! Stats options properly propagated.

The statsOptions are correctly passed to extractResultFromStats, enabling custom stats rendering configuration.

packages/vue-cli-plugin-mpx/commands/serve/web.js (1)

275-278: LGTM! Stats customization properly merged.

The stats configuration correctly merges the default assets: false with plugin-provided options, allowing customization while preserving existing defaults.

packages/vue-cli-plugin-mpx/commands/serve/mp.js (1)

17-17: LGTM! Consistent with the updated signature.

The addition of the options parameter aligns with the extended handleWebpackDone signature and enables stats customization in serve mode.

packages/vue-cli-plugin-mpx-ssr/commands/build/index.js (1)

1-1: LGTM! Import consolidation.

Good refactoring to include getCurrentTarget in the destructured imports, removing the need for a separate require statement.

Comment on lines +44 to +45
// todo:此处target应该是一个布尔值,判断是否watch, 直接把target和api替换掉有问题嘛,当前只将api换为了options
handleWebpackDone(err, stats, target, options)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Incorrect parameter type passed to handleWebpackDone.

The third parameter of handleWebpackDone(err, stats, watch, options) expects a boolean indicating watch mode, but target (a platform identifier string like 'wx', 'ali', etc.) is being passed instead. This will cause incorrect behavior in the webpack completion handler.

For build commands, the watch parameter should typically be:

  • false (for standard builds), or
  • args.watch (if watch mode is supported via CLI args)
🔎 Proposed fix
-            // todo:此处target应该是一个布尔值,判断是否watch, 直接把target和api替换掉有问题嘛,当前只将api换为了options
-            handleWebpackDone(err, stats, target, options)
+            handleWebpackDone(err, stats, false, options)

Or if watch mode should be supported:

-            // todo:此处target应该是一个布尔值,判断是否watch, 直接把target和api替换掉有问题嘛,当前只将api换为了options
-            handleWebpackDone(err, stats, target, options)
+            handleWebpackDone(err, stats, args.watch || false, options)
📝 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
// todo:此处target应该是一个布尔值,判断是否watch, 直接把target和api替换掉有问题嘛,当前只将api换为了options
handleWebpackDone(err, stats, target, options)
handleWebpackDone(err, stats, false, options)
🤖 Prompt for AI Agents
In packages/vue-cli-plugin-mpx-ssr/commands/build/index.js around lines 44-45,
the third argument passed to handleWebpackDone is currently `target` (a platform
string) but the function expects a boolean `watch`; change the call so the third
parameter is a boolean: for standard builds pass `false`, or if CLI watch
support exists pass `Boolean(args.watch)` (or `args.watch` directly) instead of
`target`, and ensure the `options` argument remains the last parameter.

@mater1996 mater1996 merged commit e5d01bb into mpx-ecology:next Jan 5, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants