nrwl / nx

Smart Monorepos · Fast CI
https://nx.dev
MIT License
23.77k stars 2.37k forks source link

GeneratePackageJson: package.json with overrides or patches generate incorrect lockfile #18402

Open alumni opened 1 year ago

alumni commented 1 year ago

Current Behavior

GeneratePackageJson will not copy and trim overrides / patchedDependencies to the generated lockfile (or package.json)

Expected Behavior

The expectation is that the generate package.json and pnpm-lock.yaml would be correct and pnpm install --frozen-lockfile does not error.

GitHub Repo

No response

Steps to Reproduce

  1. create e.g. NestJS app in monorepo with the following package.json:
{
  "dependencies": {
    "@nestjs/axios": "3.0.0"
  },
  "pnpm": {
    "overrides": {
      "axios": "0.27.2"
    }
  }
}
  1. Add an app with a build target that contains generatePackageJson: true.
  2. Build the app.
  3. Unexpected 1: pnpm overrides (and patches) are missing from the generated package.json and pnpm-lock.yaml.
  4. Unexpected 2: Running pnpm install --frozen-lockfile fails.

Nx Report

Node : 18.17.0
   OS   : win32 x64
   pnpm : 8.6.10

   nx                      : 15.9.4
   @nrwl/js                : 15.9.4
   @nrwl/jest              : 15.9.4
   @nrwl/linter            : 15.9.4
   @nrwl/workspace         : 15.9.4
   @nrwl/cli               : 15.9.4
   @nrwl/devkit            : 15.9.4
   @nrwl/eslint-plugin-nx  : 15.9.4
   @nrwl/nest              : 15.9.4
   @nrwl/node              : 15.9.4
   @nrwl/tao               : 15.9.4
   @nrwl/webpack           : 15.9.4
   typescript              : 5.1.6

Failure Logs

No response

Operating System

Additional Information

No response

craigneasbey commented 1 year ago

Thanks. This is the same for npm overrides too. We got vulnerabilities and since we did not control the CI/CD process, we had to copy over the whole package.json as a work around which doubled the image size.

jaysoo commented 1 year ago

@alumni Thanks for reporting. We'll investigate this.

alumni commented 1 year ago

pnpm supports 2 types of overrides: pnpm.overrides and resolutions (for yarn compatibility). We ended up using resolutions because Renovate doesn't handle overrides at the moment.

Current workaround is to patch nx to copy the needed keys from the root package.json:

patches/nx@15.9.4.patch:

diff --git a/src/plugins/js/package-json/create-package-json.js b/src/plugins/js/package-json/create-package-json.js
index 8b8c0224eb9c91dbf61f2d1ad73bca879b3c6d26..1d5a3dc2bbeac48b8eefc25fb147e2ee15ada725 100644
--- a/src/plugins/js/package-json/create-package-json.js
+++ b/src/plugins/js/package-json/create-package-json.js
@@ -100,6 +100,9 @@ function createPackageJson(projectName, graph, options = {}) {
     packageJson.dependencies && (packageJson.dependencies = (0, object_sort_1.sortObjectByKeys)(packageJson.dependencies));
     packageJson.peerDependencies && (packageJson.peerDependencies = (0, object_sort_1.sortObjectByKeys)(packageJson.peerDependencies));
     packageJson.peerDependenciesMeta && (packageJson.peerDependenciesMeta = (0, object_sort_1.sortObjectByKeys)(packageJson.peerDependenciesMeta));
+    rootPackageJson.packageManager && (packageJson.packageManager = rootPackageJson.packageManager);
+    rootPackageJson.pnpm && (packageJson.pnpm = rootPackageJson.pnpm);
+    rootPackageJson.resolutions && (packageJson.resolutions = rootPackageJson.resolutions);
     return packageJson;
 }
 exports.createPackageJson = createPackageJson;

Fix for patches in package.json:

{
    "pnpm": {
        "allowNonAppliedPatches": true,
        "patchedDependencies": {...}
    }
}

To fix lockfile pruning and the creation of node_modules, we use the root lockfile and run pnpm prune:

COPY patches patches
COPY ["dist/apps/<app>/package.json", "pnpm-lock.yaml", "./"]
RUN pnpm prune --prod \
    && rm -rf patches

I know nx 15.9 has limited pnpm lockfile support, but we also cherry picked some fixes from 16.x (IIRC the entire lockfile parser), it seems it still doesn't handle patches/resolutions.

riceboyler commented 1 year ago

I don't know that it will get merged, but I created a PR to hopefully just copy the pnpm and packageManager sections with every newly generated package.json file.

meeroslav commented 1 year ago

@riceboyler, that would unfortunately not work. We need to copy over the patched files as well. And those need to be pruned.

stephkoltun commented 7 months ago

is there any resolution on this? I desperately need to use overrides

tafaust commented 4 months ago

I'd need a solution for this as well. I can prepare a PR if someone can provide some guidance / assistance. Thanks!

dboune commented 1 month ago

I've just upgraded from NX 17 to 19. In 17, the generated lock file did not include patchedDependencies. In 19, patchedDependencies is copied as-is (no pruning) into the lock file but not into package.json, which breaks installs.

I used the following patch to remove patchedDependencies from the lockfile. This works because the only dependencies I'm patching are build tools that my packages don't directly depend on.

patches/nx@19.7.2.patch

diff --git a/src/plugins/js/lock-file/utils/pnpm-normalizer.js b/src/plugins/js/lock-file/utils/pnpm-normalizer.js
index 4e2d861ca8d409caec972a7f7b114590d7ced4fc..be972ca610b0fbbf80f9ccafcf1f553c08a32a1e 100644
--- a/src/plugins/js/lock-file/utils/pnpm-normalizer.js
+++ b/src/plugins/js/lock-file/utils/pnpm-normalizer.js
@@ -454,8 +454,7 @@ function normalizeLockfileV6(lockfile, isLockfileV6) {
     if (lockfileToSave.overrides != null && isEmpty(lockfileToSave.overrides)) {
         delete lockfileToSave.overrides;
     }
-    if (lockfileToSave.patchedDependencies != null &&
-        isEmpty(lockfileToSave.patchedDependencies)) {
+    if (lockfileToSave.patchedDependencies != null) {
         delete lockfileToSave.patchedDependencies;
     }
     if (lockfileToSave['neverBuiltDependencies'] != null) {
@@ -635,8 +634,7 @@ function normalizeLockfile(lockfile, opts) {
     if (lockfileToSave.overrides != null && isEmpty(lockfileToSave.overrides)) {
         delete lockfileToSave.overrides;
     }
-    if (lockfileToSave.patchedDependencies != null &&
-        isEmpty(lockfileToSave.patchedDependencies)) {
+    if (lockfileToSave.patchedDependencies != null) {
         delete lockfileToSave.patchedDependencies;
     }
     if (!lockfileToSave.packageExtensionsChecksum) {
flo-sch commented 1 month ago

Facing this using nx@20.0.1 here, building a node app with esbuild, with a production dependency that needs patching.

I added patches/* to the assets option of the esbuild executor to include patches in the dist folder:

{
  "name": "<my-app>",
  "targets": {
    "build": {
      "executor": "@nx/esbuild:esbuild",
      "options": {
        "assets": ["patches/*", "apps/<my-app>/src/assets"],
      }
    }
  }
}

Then patched NX with this to ensure patches are present in the generated package.json file, matching the lock file structure:

diff --git a/src/plugins/js/package-json/create-package-json.js b/src/plugins/js/package-json/create-package-json.js
index 63cd927085d0ea9dc54d6d263742be5453707ef2..2c95fa5a7c130325eb59e954289e41467d336c38 100644
--- a/src/plugins/js/package-json/create-package-json.js
+++ b/src/plugins/js/package-json/create-package-json.js
@@ -137,6 +137,13 @@ function createPackageJson(projectName, graph, options = {}, fileMap = null) {
             ...packageJson.pnpm.overrides,
         };
     }
+    if (rootPackageJson.pnpm?.patchedDependencies) {
+        packageJson.pnpm ??= {};
+        packageJson.pnpm.patchedDependencies = {
+          ...rootPackageJson.pnpm.patchedDependencies,
+          ...packageJson.pnpm.patchedDependencies,
+        };
+    }
     // yarn
     if (rootPackageJson.resolutions && !options.skipOverrides) {
         packageJson.resolutions = {
seyfert commented 3 weeks ago

After upgrading to nx@19 and pnpm@9, we hit this same issue when generatePackageJson was set to true.

In our case the fix was to exclude packages from patchedDependencies in the generated pnpm-lock.yaml if the package wasn't included in the generated package.json.

diff --git a/src/plugins/js/lock-file/utils/pnpm-normalizer.js b/src/plugins/js/lock-file/utils/pnpm-normalizer.js
index 4e2d861ca8d409caec972a7f7b114590d7ced4fc..1b8b0188c94112a100f64642e39c430b836fbd02 100644
--- a/src/plugins/js/lock-file/utils/pnpm-normalizer.js
+++ b/src/plugins/js/lock-file/utils/pnpm-normalizer.js
@@ -542,6 +542,7 @@ function pruneTime(time, importers) {
 function convertToLockfileFile(lockfile, opts) {
     const packages = {};
     const snapshots = {};
+    const patchedDependencies = {};
     for (const [depPath, pkg] of Object.entries(lockfile.packages ?? {})) {
         snapshots[depPath] = pick([
             'dependencies',
@@ -566,10 +567,15 @@ function convertToLockfileFile(lockfile, opts) {
                 'peerDependenciesMeta',
                 'version',
             ], pkg);
+
+            if (lockfile.patchedDependencies && lockfile.patchedDependencies[pkgId]) {
+                patchedDependencies[pkgId] = lockfile.patchedDependencies[pkgId];
+            }
         }
     }
     const newLockfile = {
         ...lockfile,
+        patchedDependencies,
         snapshots,
         packages,
         lockfileVersion: lockfile.lockfileVersion.toString(),