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.
electron · Oct 8, 2021 · 1acdf85 · 1acdf85
1 parent f32f0b3
commit 1acdf85
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 5 deletions.
diff --git 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
@@ -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
@@ -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;
+    });
+  });
 });