[Package] Exclude unused dependencies when building package.json files#3232
[Package] Exclude unused dependencies when building package.json files#3232
package.json files#3232Conversation
|
@adamziel "dependencies": {
"express": "4.22.0",
"ini": "4.1.2",
"wasm-feature-detect": "1.8.0",
"ws": "8.18.3",
"yargs": "17.7.2",
"@php-wasm/universal": "3.0.46",
"@php-wasm/util": "3.0.46",
"@php-wasm/logger": "3.0.46"
},I can reliably say That is expected since I only indicate which dependency should be ignored. Two solutions : - "excludedDependencies": ["@php-wasm/node"]
+ "excludedDependencies": ["@php-wasm/node", "express", "ini", "wasm-feature-detect", "ws", "yargs"]Or we should find another way to ignore these subdependencies. Maybe this :
- if (!packageJson.dependencies) {
packageJson.dependencies = {};
- }
for (const dep of monorepoDependencies) {
packageJson.dependencies[dep.name] = dep.version;
}I expect having to clean each |
|
I may have a better suggestion here. Instead of For example "dependencies": {
- "express": "4.22.0",
- "ini": "4.1.2",
- "wasm-feature-detect": "1.8.0",
- "ws": "8.18.3",
- "yargs": "17.7.2",
"@php-wasm/universal": "3.0.46",
"@php-wasm/util": "3.0.46",
"@php-wasm/logger": "3.0.46"
- "@php-wasm/node": "3.0.46"
},So "filteredDependencies": ["@php-wasm/universal", "@php-wasm/util", "@php-wasm/logger"]Same with "dependencies": {
- "ini": "4.1.2",
"wasm-feature-detect": "1.8.0",
- "ws": "8.18.3",
"@php-wasm/universal": "3.0.46"
},So : "filteredDependencies": ["@php-wasm/universal", "wasm-feature-detect"]
|
6a75247 to
3b16f72
Compare
3b16f72 to
55d3597
Compare
55d3597 to
e629587
Compare
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR updates the Nx package-json executor to prevent test-only and transitive dependencies from leaking into published package.json files, aligning declared dependencies with actual source imports.
Changes:
- Filter Nx file map to exclude test directories and use it to drive
createPackageJsondependency detection. - Remove non-source (transitive / test-derived) dependencies from the generated
package.jsonoutput. - Rewire many tests to import from
../lib/...and add dependency-verification tests for built npm packages.
Reviewed changes
Copilot reviewed 61 out of 78 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/playground/wordpress-builds/src/test/get-wordpress-module-details.spec.ts | Updates test import path to match new source vs test folder layout. |
| packages/playground/test-built-npm-packages/es-modules-and-vitest/tests/deps.spec.ts | Adds an automated check that built packages only declare deps used by source imports. |
| packages/playground/test-built-npm-packages/es-modules-and-vitest/run-tests.ts | Runs both dependency checks and existing tests per PHP version. |
| packages/playground/test-built-npm-packages/commonjs-and-jest/tests/deps.spec.ts | Adds the same dependency declaration check for the CJS/Jest build pipeline. |
| packages/playground/storage/src/test/paths.spec.ts | Updates test imports to reference ../lib modules. |
| packages/playground/storage/src/test/git-sparse-checkout.spec.ts | Updates test imports to reference ../lib modules. |
| packages/playground/storage/src/test/filesystems.spec.ts | Updates imports/types and fixture path to align with new test folder structure. |
| packages/playground/storage/src/test/changeset.spec.ts | Updates test import to reference ../lib. |
| packages/playground/storage/package.json | Removes dependency artifacts so the package only declares needed runtime deps. |
| packages/playground/blueprints/src/tests/v1/resources.spec.ts | Updates test import paths to reference lib output. |
| packages/playground/blueprints/src/tests/v1/compile.spec.ts | Updates imports to lib and adjusts fixture path resolution. |
| packages/playground/blueprints/src/tests/steps/write-files.spec.ts | Updates test import path to reference lib step implementation. |
| packages/playground/blueprints/src/tests/steps/write-file.spec.ts | Updates test import path to reference lib step implementation. |
| packages/playground/blueprints/src/tests/steps/wp-cli.spec.ts | Updates imports to lib and adjusts fixture path resolution. |
| packages/playground/blueprints/src/tests/steps/site-data.spec.ts | Updates step import path to reference lib. |
| packages/playground/blueprints/src/tests/steps/set-site-language.spec.ts | Updates step import path to reference lib. |
| packages/playground/blueprints/src/tests/steps/run-sql.spec.ts | Updates step import path to reference lib. |
| packages/playground/blueprints/src/tests/steps/run-php.spec.ts | Updates step import path to reference lib. |
| packages/playground/blueprints/src/tests/steps/rmdir.spec.ts | Updates step import path to reference lib. |
| packages/playground/blueprints/src/tests/steps/rm.spec.ts | Updates step import path to reference lib. |
| packages/playground/blueprints/src/tests/steps/reset-data.spec.ts | Updates step import path to reference lib. |
| packages/playground/blueprints/src/tests/steps/mv.spec.ts | Updates step import path to reference lib. |
| packages/playground/blueprints/src/tests/steps/mkdir.spec.ts | Updates step import path to reference lib. |
| packages/playground/blueprints/src/tests/steps/login.spec.ts | Updates step imports to reference lib. |
| packages/playground/blueprints/src/tests/steps/install-theme.spec.ts | Updates step import path to reference lib. |
| packages/playground/blueprints/src/tests/steps/install-plugin.spec.ts | Updates step import path to reference lib. |
| packages/playground/blueprints/src/tests/steps/import-wxr.spec.ts | Updates imports to lib and adjusts fixture paths. |
| packages/playground/blueprints/src/tests/steps/import-wordpress-files.spec.ts | Updates step imports to reference lib. |
| packages/playground/blueprints/src/tests/steps/import-theme-starter-content.spec.ts | Updates step import path to reference lib. |
| packages/playground/blueprints/src/tests/steps/enable-multisite.spec.ts | Updates step imports and adjusts fixture paths. |
| packages/playground/blueprints/src/tests/steps/define-wp-config-consts.spec.ts | Updates step import path to reference lib. |
| packages/playground/blueprints/src/tests/steps/cp.spec.ts | Updates step import path to reference lib. |
| packages/playground/blueprints/src/tests/steps/activate-theme.spec.ts | Updates step import path to reference lib. |
| packages/playground/blueprints/src/tests/steps/activate-plugin.spec.ts | Updates step import path to reference lib. |
| packages/php-wasm/web/src/test/tls/prf.spec.ts | Updates test import path to reference lib TLS implementation. |
| packages/php-wasm/web/src/test/tls/certificates.spec.ts | Updates TLS test imports to reference lib. |
| packages/php-wasm/web/src/test/tcp-over-fetch-websocket.spec.ts | Updates imports to lib and adjusts test util import path. |
| packages/php-wasm/web/playwright.config.ts | Restricts Playwright matching to a specific spec file. |
| packages/php-wasm/web-service-worker/src/test/utils.spec.ts | Updates test imports to lib and adds explicit Vitest imports. |
| packages/php-wasm/web-service-worker/src/test/fetch-with-cors-proxy.spec.ts | Updates test imports to lib. |
| packages/php-wasm/web-service-worker/src/index.ts | Changes package exports to re-export from ./lib/*. |
| packages/php-wasm/util/src/test/split-shell-command.spec.ts | Updates test import path to reference ../lib. |
| packages/php-wasm/util/src/test/semaphore.spec.ts | Updates test import path to reference ../lib. |
| packages/php-wasm/util/src/test/paths.spec.ts | Updates test import path to reference ../lib. |
| packages/php-wasm/util/src/test/create-spawn-handler.spec.ts | Updates test imports to reference ../lib. |
| packages/php-wasm/universal/src/test/process-id-allocator.spec.ts | Updates test import path to reference ../lib. |
| packages/php-wasm/universal/src/test/php-worker.spec.ts | Updates test imports to reference ../lib. |
| packages/php-wasm/universal/src/test/php-response.spec.ts | Updates test import path to reference ../lib. |
| packages/php-wasm/universal/src/test/php-request-handler.spec.ts | Updates test imports to reference ../lib. |
| packages/php-wasm/universal/src/test/object-pool-proxy.spec.ts | Updates test import path to reference ../lib. |
| packages/php-wasm/universal/src/test/http-cookie-store.spec.ts | Updates test import path to reference ../lib. |
| packages/php-wasm/universal/src/test/file-lock-manager-in-memory.spec.ts | Updates test import path to reference ../lib. |
| packages/php-wasm/universal/src/test/file-lock-interval-tree.spec.ts | Updates test imports to reference ../lib. |
| packages/php-wasm/universal/src/test/comlink-sync.spec.ts | Updates test import path and adjusts referenced file path to ../lib. |
| packages/php-wasm/scopes/src/test/scope.test.ts | Updates tests to import from a new ../lib/scope module. |
| packages/php-wasm/scopes/src/lib/scope.ts | Extracts scope URL helpers into src/lib for cleaner source/test separation. |
| packages/php-wasm/scopes/src/index.ts | Re-exports from ./lib/scope instead of defining logic inline. |
| packages/php-wasm/progress/src/test/progress-tracker.spec.ts | Updates test imports to reference ../lib. |
| packages/php-wasm/node-polyfills/src/test/custom-event.spec.ts | Updates test import to reference ../lib. |
| packages/php-wasm/node-polyfills/src/test/blob.spec.ts | Updates test import to reference ../lib. |
| packages/nx-extensions/src/executors/package-json/executor.ts | Implements source-only dependency detection and filters generated dependency lists accordingly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function getSourceOnlyFileMap(): ProjectFileMap { | ||
| const cache = readFileMapCache(); | ||
| const fullFileMap = cache?.fileMap?.projectFileMap || {}; | ||
| const filtered: ProjectFileMap = {}; | ||
| for (const [project, files] of Object.entries(fullFileMap)) { | ||
| filtered[project] = (files as FileData[]).filter((f) => | ||
| isSourceFile(f.file) | ||
| ); | ||
| } | ||
| return filtered; | ||
| } |
There was a problem hiding this comment.
When the Nx file map cache is missing/unavailable, fullFileMap becomes {}, and the executor still passes the resulting empty map into createPackageJson and uses it to compute sourceDeps. This can cause all dependencies (including legitimate runtime deps) to be dropped from the generated package.json. Consider returning undefined/null when the cache is unavailable and skipping both createPackageJson(..., sourceFileMap) and dependency filtering in that case (or falling back to the unfiltered createPackageJson behavior).
| import { readFileMapCache } from 'nx/src/project-graph/nx-deps-cache'; | ||
| import { fileDataDepTarget } from 'nx/src/config/project-graph'; |
There was a problem hiding this comment.
These imports reach into Nx internal source paths (nx/src/...), which are not part of the public API surface and can break on Nx upgrades or even patch releases. If possible, prefer a public @nx/devkit API (or encapsulate this behind a small compatibility layer with a well-defined fallback) so the executor remains stable across Nx versions.
| if (entry.isDirectory()) { | ||
| if (entry.name !== 'test' && entry.name !== 'node_modules') { | ||
| walk(full); | ||
| } | ||
| } else if ( |
There was a problem hiding this comment.
This dependency check intends to ignore test code, but it only skips a directory named test. The repo also contains tests under src/tests (plural), so those files will still be scanned and their imports will be treated as “source imports”, weakening the check and potentially masking dependency leaks. Update the walker to exclude both test and tests directories (and keep the logic aligned with the executor’s test-file filtering).
Motivation for the change, related issues
Based on this issue
While working on #3093, I found out
@php-wasm/nodewas installed alongside@php-wasm/web. While looking deeper into it, I found in thepackage-lock.jsonfile thatfs-journalhad@php-wasm/nodeas an unused dependency, since it was only used infs-journaltest files.The root cause : NX's
createPackageJsonscans all project files, including test files, to resolve dependencies, and the customgetMonorepoDependenciesfunction includes all static dependency edges from the project graph regardless of whether they come from source or test files. This caused test-only dependencies to leak into publishedpackage.jsonfiles.Additionally,
createPackageJsonflattens transitive dependencies, which meant packages like@php-wasm/webended up declaring dozens of indirect npm dependencies likeoctokitorasync-lockthat belong to their transitive dependencies, not to them directly.Implementation details
Instead of maintaining a manual
filteredDependencieswhitelist per package, the package-json executor now automatically excludes test-only dependencies by:test/directories.createPackageJsonso it only resolves dependencies from source files.createPackageJsonoutput.This also required moving test files into dedicated
src/test/directories across published packages, so the directory-based filter works consistently. There are still directories that doesn't have atestdirectory :I also removed 10 unused dependency artifacts [ probably related to former isomorphic-git ] from
@wp-playground/storage's package.json.Testing Instructions (or ideally a Blueprint)
CI
🧪
test-built-npm-packages/commonjs-and-jest/tests/deps.spec.ts🧪
test-built-npm-packages/es-modules-and-vitest/tests/deps.spec.tsor manually run :
In
trunk:node_modules/.bin/nx reset && node_modules/.bin/nx run php-wasm-web:buildand compare the
dist/packages/php-wasm/web/package.jsonwith the one built in this branch.