Support for cnpm and use electron as devDependence instead of electro… #183
Conversation
|
pnpm is using link too https://github.com/pnpm/pnpm |
|
@zcbenz we find the root cause finally. |
src/rebuild.ts
Outdated
| @@ -42,6 +44,8 @@ class Rebuilder { | |||
| this.ABI = nodeAbi.getAbi(electronVersion, 'electron'); | |||
| this.prodDeps = extraModules.reduce((acc, x) => acc.add(x), new Set()); | |||
| this.rebuilds = []; | |||
| this.realModulePaths = []; | |||
| this.realNodeModulesPaths = []; | |||
There was a problem hiding this comment.
use Map instead Array will be more faster.
There was a problem hiding this comment.
See my comment below about using sets, all we need is a concept of "has been used before", we don't need to store a value with the path so maps will be less efficient than using a raw set
src/rebuild.ts
Outdated
| @@ -42,6 +44,8 @@ class Rebuilder { | |||
| this.ABI = nodeAbi.getAbi(electronVersion, 'electron'); | |||
| this.prodDeps = extraModules.reduce((acc, x) => acc.add(x), new Set()); | |||
| this.rebuilds = []; | |||
| this.realModulePaths = []; | |||
There was a problem hiding this comment.
For performance reasons these two path arrays should be sets
src/rebuild.ts
Outdated
| const realNodeModulesPath = fs.realpathSync(nodeModulesPath); | ||
| if (this.realNodeModulesPaths.indexOf(realNodeModulesPath) > -1) { | ||
| return; | ||
| } else { |
There was a problem hiding this comment.
No need for this else statement as their is a return in the main if condition
src/rebuild.ts
Outdated
|
|
||
| if (this.realModulePaths.indexOf(realPath) > -1) { | ||
| continue; | ||
| } else { |
There was a problem hiding this comment.
Again no need for the else block as their is a continue statement in the main if block
|
Also if you pull in latest master the travis builds should go green 👍 |
I am using
cnpminstead ofnpmoryarnto install my node modules, and I find out that electron-rebuild does not support forcnpm.cnpmuse link to handle the dependencies, as I can see electron-rebuild may make circles while scanning the dependencies tree and will rebuild a module twice and this will make node-gyp throw errors.