From 1acdf85f79ab4e42daddd8c489da4d64ecbfdcb5 Mon Sep 17 00:00:00 2001
From: Paul Marechal <paul.marechal@ericsson.com>
Date: Fri, 8 Oct 2021 18:40:57 -0400
Subject: [PATCH] fix usage of this.prodDeps as a Set<string>

For some reason TypeScript is AOK with stuffing `Set` objects with
random keys. This is problematic as this is not how `Set` are supposed
to be used (like a plain JavaScript object map).

Fix the logic to respect the `-w/--which-module` CLI argument.
---
 src/module-walker.ts |  8 +++----
 src/rebuild.ts       |  2 +-
 test/rebuild.ts      | 53 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/src/module-walker.ts b/src/module-walker.ts
index d03485c8a..a8b6e123a 100644
--- a/src/module-walker.ts
+++ b/src/module-walker.ts
@@ -53,7 +53,7 @@ export class ModuleWalker {
     }
 
     for (const key of depKeys) {
-      this.prodDeps[key] = true;
+      this.prodDeps.add(key);
       const modulePaths: string[] = await searchForModule(
         this.buildPath,
         key,
@@ -97,11 +97,11 @@ export class ModuleWalker {
 
     const callback = this.markChildrenAsProdDeps.bind(this);
     for (const key of Object.keys(childPackageJson.dependencies || {}).concat(Object.keys(childPackageJson.optionalDependencies || {}))) {
-      if (this.prodDeps[key]) {
+      if (this.prodDeps.has(key)) {
         continue;
       }
 
-      this.prodDeps[key] = true;
+      this.prodDeps.add(key);
 
       moduleWait.push(this.findModule(key, modulePath, callback));
     }
@@ -133,7 +133,7 @@ export class ModuleWalker {
       }
       this.realModulePaths.add(realPath);
 
-      if (this.prodDeps[`${prefix}${modulePath}`] && (!this.onlyModules || this.onlyModules.includes(modulePath))) {
+      if (this.prodDeps.has(`${prefix}${modulePath}`) && (!this.onlyModules || this.onlyModules.includes(modulePath))) {
         this.modulesToRebuild.push(realPath);
       }
 
diff --git a/src/rebuild.ts b/src/rebuild.ts
index 58a54d145..f427556a0 100644
--- a/src/rebuild.ts
+++ b/src/rebuild.ts
@@ -95,7 +95,7 @@ export class Rebuilder implements IRebuilder {
 
     this.ABIVersion = options.forceABI?.toString();
     const onlyModules = options.onlyModules || null;
-    const extraModules = (options.extraModules || []).reduce((acc: Set<string>, x: string) => acc.add(x), new Set<string>());
+    const extraModules = new Set(options.extraModules);
     const types = options.types || defaultTypes;
     this.moduleWalker = new ModuleWalker(
       this.buildPath,
diff --git a/test/rebuild.ts b/test/rebuild.ts
index 92fef74fb..b7dfcd32a 100644
--- a/test/rebuild.ts
+++ b/test/rebuild.ts
@@ -194,4 +194,57 @@ describe('rebuilder', () => {
       await expectNativeModuleToBeRebuilt(testModulePath, 'ffi-napi');
     });
   });
+
+  describe('rebuilding from different package', function() {
+    this.timeout(MINUTES_IN_MILLISECONDS);
+
+    beforeEach(async () => {
+      await resetTestModule(testModulePath);
+      const packageJsonPath = path.join(testModulePath, 'package.json');
+      const packageJson = await fs.readJSON(packageJsonPath);
+      packageJson.dependencies = {};
+      packageJson.devDependencies = {};
+      packageJson.optionalDependencies = {};
+      packageJson.peerDependencies = {};
+      await fs.writeJSON(packageJsonPath, packageJson, { spaces: 2 });
+    });
+    afterEach(async() => await cleanupTestModule(testModulePath));
+
+    it('should not rebuild anything if package.json does not depend on anything', async () => {
+      const nativeModuleBinary = path.join(testModulePath, 'node_modules', 'native-hello-world', 'build', 'Release', 'hello_world.node');
+      expect(await fs.pathExists(nativeModuleBinary)).to.be.true;
+      await fs.remove(nativeModuleBinary);
+      expect(await fs.pathExists(nativeModuleBinary)).to.be.false;
+      const rebuilder = rebuild({
+        buildPath: testModulePath,
+        electronVersion: testElectronVersion,
+        arch: process.arch,
+        force: true
+      });
+      let built = 0;
+      rebuilder.lifecycle.on('module-done', () => built++);
+      await rebuilder;
+      expect(built).to.equal(0);
+      expect(await fs.pathExists(nativeModuleBinary)).to.be.false;
+    });
+
+    it('should rebuild extraModules even if package.json does not depend on it', async () => {
+      const nativeModuleBinary = path.join(testModulePath, 'node_modules', 'native-hello-world', 'build', 'Release', 'hello_world.node');
+      expect(await fs.pathExists(nativeModuleBinary)).to.be.true;
+      await fs.remove(nativeModuleBinary);
+      expect(await fs.pathExists(nativeModuleBinary)).to.be.false;
+      const rebuilder = rebuild({
+        buildPath: testModulePath,
+        electronVersion: testElectronVersion,
+        arch: process.arch,
+        extraModules: ['native-hello-world'],
+        force: true
+      });
+      let built = 0;
+      rebuilder.lifecycle.on('module-done', () => built++);
+      await rebuilder;
+      expect(built).to.equal(1);
+      expect(await fs.pathExists(nativeModuleBinary)).to.be.true;
+    });
+  });
 });