fix: devkit build error, add external deps #950
Conversation
WalkthroughThe changes introduce workspace-aware dependency version resolution by adding a new public utility Changes
Sequence DiagramsequenceDiagram
participant Build as PluginPackage.build()
participant Write as writeExternalPackageVersion()
participant Dep as getDepVersion()
participant WS as Workspace Utils
participant YAML as js-yaml
participant NM as node_modules
Build->>Write: await writeExternalPackageVersion(cwd, log)
activate Write
loop for each excludePackage
Write->>Dep: getDepVersion(packageName, cwd)
activate Dep
alt workspace:* protocol
Dep->>WS: findWorkspacePackages()
Dep->>WS: getWorkspacePackagePath(packageName)
WS-->>Dep: version from package.json
else catalog: protocol
Dep->>YAML: load pnpm-workspace.yaml
Dep->>Dep: getCatalogVersion(packageName, catalogRef)
YAML-->>Dep: version from catalog
else normal protocol or fallback
Dep->>NM: read from node_modules
NM-->>Dep: version or null
end
Dep-->>Write: version | null
deactivate Dep
alt version resolved
Write->>Write: data[packageName] = version
else no version
Write->>Write: log error per package
end
end
Write-->>Build: (async completion)
deactivate Write
Build->>Build: continue build
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
|
处理插件依赖检查失败问题，现状是读取解析旧版本包。导致期望的版本号是旧版本号，但实际是更新后的版本。插件依赖检查失败，阻止开启动作。更改为：
|
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/devkit/src/builder/build/utils/getDepsConfig.ts (1)
117-177: Consider extracting the repeated node_modules fallback logic.The pattern of reading a version from node_modules (lines 123-125, 138-140, 152-154, 164-166, 170-172) is duplicated 5 times. This reduces maintainability.
Consider extracting this into a helper function:
+function getVersionFromNodeModules(packageName: string, cwd: string): string { + const depPkgPath = getDepPkgPath(packageName, cwd); + const depPkg = require(depPkgPath); + return depPkg.version; +} + export async function getDepVersion(packageName: string, cwd: string): Promise<string | null> { try { // Get current package.json to check dependency protocol const currentPkgPath = path.join(cwd, 'package.json'); if (!fs.existsSync(currentPkgPath)) { // Fallback to node_modules if package.json not found - const depPkgPath = getDepPkgPath(packageName, cwd); - const depPkg = require(depPkgPath); - return depPkg.version; + return getVersionFromNodeModules(packageName, cwd); } const currentPkg = fs.readJsonSync(currentPkgPath); const allDeps = { ...currentPkg.dependencies, ...currentPkg.devDependencies, ...currentPkg.peerDependencies, }; const depVersion = allDeps[packageName]; if (!depVersion) { // Not in dependencies, try to get from node_modules - const depPkgPath = getDepPkgPath(packageName, cwd); - const depPkg = require(depPkgPath); - return depPkg.version; + return getVersionFromNodeModules(packageName, cwd); } // Check protocol type if (depVersion === 'workspace:*' || depVersion.startsWith('workspace:')) { // For workspace:* protocol, read from workspace package.json const workspacePkgPath = await getWorkspacePackagePath(packageName); if (workspacePkgPath && fs.existsSync(workspacePkgPath)) { const workspacePkg = fs.readJsonSync(workspacePkgPath); return workspacePkg.version; } // Fallback to node_modules if workspace package not found - const depPkgPath = getDepPkgPath(packageName, cwd); - const depPkg = require(depPkgPath); - return depPkg.version; + return getVersionFromNodeModules(packageName, cwd); } if (depVersion.startsWith('catalog:')) { // For catalog: protocol, read from pnpm-workspace.yaml catalog const catalogVersion = getCatalogVersion(packageName, depVersion); if (catalogVersion) { return catalogVersion; } // Fallback to node_modules if catalog not found - const depPkgPath = getDepPkgPath(packageName, cwd); - const depPkg = require(depPkgPath); - return depPkg.version; + return getVersionFromNodeModules(packageName, cwd); } // For normal dependencies, read from node_modules - const depPkgPath = getDepPkgPath(packageName, cwd); - const depPkg = require(depPkgPath); - return depPkg.version; + return getVersionFromNodeModules(packageName, cwd); } catch (error) { console.error(`Error getting version for ${packageName}:`, error); return null; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
packages/devkit/package.json(2 hunks)packages/devkit/src/builder/build/utils/getDepsConfig.ts(2 hunks)packages/devkit/src/builder/buildable-packages/plugin-package.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/devkit/src/builder/buildable-packages/plugin-package.ts (4)
packages/devkit/src/builder/build/index.ts (1)
PkgLog(6-6)packages/devkit/src/builder/build/utils/buildPluginUtils.ts (2)
getSourcePackages(68-70)getExcludePackages(78-81)packages/core/src/plugin-manager/constants.ts (1)
pluginPrefix(4-4)packages/devkit/src/builder/build/utils/getDepsConfig.ts (1)
getDepVersion(117-177)
packages/devkit/src/builder/build/utils/getDepsConfig.ts (1)
packages/devkit/src/builder/build/constant.ts (1)
ROOT_PATH(33-33)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: tests
- GitHub Check: test-coverage
🔇 Additional comments (6)
packages/devkit/src/builder/build/utils/getDepsConfig.ts (3)
73-87: LGTM!The workspace package resolution logic correctly uses
findWorkspacePackagesto locate packages by name, with appropriate error handling returning null on failure.
92-109: LGTM!The catalog version resolution correctly reads from
pnpm-workspace.yamland extracts the version from the catalog configuration. The protocol prefix handling is appropriate.
4-8: The review comment is based on an outdated security concern and does not apply to the version being used.The codebase uses js-yaml v4.1.0, which is covered by the v4.0.0+ changes where
load()is safe by default. In js-yaml v4.0.0+, unsafe JS tags were moved out and theload()function is safe by default, so there is no security issue with usingload()in this context. The warning in the original review applies only to js-yaml v3.x and earlier versions.Likely an incorrect or invalid review comment.
packages/devkit/src/builder/buildable-packages/plugin-package.ts (3)
27-27: LGTM!The import correctly includes
getDepVersionfor the new workspace-aware dependency resolution.
147-169: LGTM! Excellent async conversion with improved error handling.The conversion to async is well-implemented:
- For-of loop is more appropriate than reduce for async operations
- Null-checking ensures only valid versions are recorded
- Error messages now include the specific package name for easier debugging
397-397: LGTM!The call site correctly awaits the now-async
writeExternalPackageVersionfunction.
| "execa": "^8.0.1", | ||
| "fast-glob": "^3.3.3", | ||
| "fs-extra": "^11.3.2", | ||
| "js-yaml": "^4.1.0", |
There was a problem hiding this comment.
🧩 Analysis chain
Verify js-yaml versions for latest security patches.
The addition of js-yaml and @types/js-yaml to support YAML parsing (as mentioned in the PR objectives) looks correct. However, since the knowledge cutoff predates the current date by several months, please verify that these specific versions remain secure and up-to-date.
Also applies to: 70-70
🌐 Web query:
What is the latest version of js-yaml and are there any security advisories for js-yaml version 4.1.0?
💡 Result:
Latest published version: 4.1.1 (as of Nov 14, 2025). [1]
Security advisory: yes — CVE-2025-64718 (prototype pollution via merge/<<) affects js-yaml 4.1.0 and earlier; the issue is patched in 4.1.1. Upgrade to 4.1.1. [2][3]
Update js-yaml to 4.1.1 to address CVE-2025-64718 security vulnerability.
The package.json currently specifies "js-yaml": "^4.1.0" on line 47, but CVE-2025-64718 (prototype pollution via merge/<<) affects js-yaml 4.1.0 and earlier; the issue is patched in 4.1.1. Update the version constraint to "^4.1.1" to exclude the vulnerable version from the permitted range.
🤖 Prompt for AI Agents
In packages/devkit/package.json around line 47 the dependency "js-yaml" is
pinned to "^4.1.0", which allows the vulnerable 4.1.0 release; update the
version constraint to "^4.1.1" so the vulnerable version is excluded and the
patched 4.1.1 is used, then run install and lockfile update (npm/yarn/pnpm) to
persist the change.
Summary by CodeRabbit
New Features
Chores