NPM lockfile versions do not match

[TomCaserta]

For context:

So I have a little different of a use case to the main 'firebase' one that others seem to have but it seems to mostly need to achieve the same things. In effect what I want to be able to do is take a package from my monorepo with all its dependencies (as in, able to be npm instaled later) and place its built artefacts into a docker image.

The issue I'm having is that the lockfile is re-generated from scratch in the isolated package and it pays no attention to the current lock file at the workspace root. This means if one of the dependencies are not pinned correctly it could cause a difference in execution between running any code inside my isolated package vs my monorepo itself.

Of course, ideally my package files would be well defined and any dependencies should be pinned or at least specify a proper version range, but this isn't always the case and I think the default behaviour of effectively ignoring the workspace lockfile could be dangerous to those not aware of this.

This is why I believe this may be a bug. I did some testing myself and managed to get it to work if I copied the workspace package-lock.json to the isolateDir in generate-npm-lockfile.ts and then await arborist.loadVirtual() before building the ideal tree. I'm not too well versed in using the arborist package (so that last step may be unnecessary) but this seemed to generate a lockfile as I expected with the same dependencies as were defined in the workspace itself.

Please let me know if you need any more information.

[0x80]

@TomCaserta Thanks for reporting. We might have messed this up along the way... I'm surprised that you are the first to report it. Maybe not a lot of people use NPM with this package.

Having the versions in the isolate output the same as the original lockfile is one of the fundamental things that you'd use this for. In that sense your use-case is no different from the others. If you deploy to Firebase you'd still want to make sure your deployed code is using the original lockfile versions.

Initially, I could only get NPM to work by moving the entire node_modules folder to isolate and then moving it back after isolation was done, which was or course both fragile and hacky.

Later someone reported that it also works fine without moving the folder, and maybe I didn't properly verify that, but then we removed that step...

I'm not familiar with Arborist either.

In your workaround, does the process still generate a pruned lockfile, or does the lockfile contain everything? Having locked versions is more important so if you want to submit a PR I would be willing to accept it.

I don't have much time to work on this but I'll try to verify this problem myself in the coming weeks.

[0x80]

@TomCaserta I can see the issue, but unfortunately I am not able to reproduce your solution.

I've published 1.20.0-2 under @next, so you could try that. It calls loadVirtual but I don't see it working. I tested it in mono-ts in the use-npm branch. The root lockfile has firebase-admin locked to 12.1, but the resulting lockfile still shows 12.7 for me.

Here are my changes so far #113

[TomCaserta]

Thanks for your response!

Looking at the code there it doesn't look different to the patch I applied to the built libraries dist (besides naming and the fact I wasn't using path join ofc):

--- a/node_modules/isolate-package/dist/isolate-bin.mjs
+++ b/node_modules/isolate-package/dist/isolate-bin.mjs
@@ -1,5 +1,3 @@
-#!/usr/bin/env node
-
 // src/isolate-bin.ts
 import console2 from "node:console";
 import sourceMaps from "source-map-support";
@@ -406,6 +404,8 @@ async function generateNpmLockfile({
       throw new Error(`Failed to find node_modules at ${nodeModulesPath}`);
     }
     const arborist = new Arborist({ path: isolateDir });
+    await fs9.copy(workspaceRootDir +'/package-lock.json', isolateDir + '/package-lock.json')
+    await arborist.loadVirtual();
     const { meta } = await arborist.buildIdealTree();
     meta?.commit();
     const lockfilePath = path7.join(isolateDir, "package-lock.json");

One thing I do notice is that I was patching version 1.19.0 so could be the other changes in 1.20.0+ that makes this not function (or perhaps it's only working in my repo due to some other configuration or pure coincidence).

In any case, I will give your fix a test in my repo and see if it has different behaviour to the patched version that has been working and debug further.

Edit: looking at each version with my monorepo:

1.19.0 -> Dependencies are as described in the original post (all dependencies have effectively changed)
1.20.0 -> Lock file is the same as above
1.20.0-2 -> Lock file is the same as the patched version I describe above and at least in the case of my monorepo its using the correct dependencies and its pruned to only the dependencies the isolated package uses

I checked out mono-ts use-npm branch and configured it to use the 1.20.0-2 release of isolate package and yep you're right it doesn't have the correct behaviour.

What I think may be going on here is something to do with how npm hoists packages when the versions match across all packages.

In my repository I am using the same version of a particular package across all of my packages so only one entry node_modules/external-dependency-xyz root entry is listed in the lockfile but in your mono-ts repository you have a root entry for node_modules/firebase-admin with version 11.11.1 and then your fns and api packages have their own entry for services/api/node_modules/firebase-admin which has the version you mentioned.

I think the act of copying it the root lock file works in my case because the path to the module is the same when isolating the package (ie. node_modules/dependency-name) and when in the workspace itself. I'm presuming that when isolating the module in the mono-ts scenario since the root entry is of a different major version (11 vs 12) its discarding and building the tree with the requested version and re-resolving what that is (ie. updates to 12.7).

To me this signifies that my 'solution' will not work in all cases and therefore is invalid.

Now this makes me wonder though, why doesn't arborist respect the workspace lockfile without copying it. Probably it could be to do with the isolated package not being considered as part of the workspace. I think realistically speaking I need to dive deeper into what npm is doing under the hood to fully understand where the issue lies, fairly certain its going to be a case of trying to wrangle npm to do something its just not supposed to do.

Just off the top of my head: A less than ideal solution (without fully understanding everything) though might be to pre-process the lockfile replacing <path-to-isolated-package>/node_modules/ with node_modules/ - obviously may have undesired side effects though.

[0x80]

Hi Tom, Thanks for diving deeper. That seems plausible.

To be honest I am not willing to spend a lot of time on this. This library already cost me way more time than I initially imagined. I'm happy that PNPM support is solid at least, because that covers my personal needs. I have too many other things on my plate.

I tried reading the Arborist code, but I find it hard to grasp, so I'm putting my hopes on other people like you to figure this out if you care enough about NPM.

In the meantime, I would recommend anyone to use PNPM, especially for monorepos. I think the transition was painless and I'm enjoying things like recursive interactive dependency upgrades and the Node version manager.

But with loadVirtual we are at least one step closer to the desired behavior. If it makes things work for only a part of the use-cases I still think it's worth merging.