Fix packaging/making with npm/yarn

The code was running 'npm prune', but that doesn't do anything if no dependencies are already installed. What we really want is 'npm install --production' and I also added '--no-bin-links' to match the yarn behavior.

Additionally, we weren't using the same logic as ember to decide when to use yarn vs. npm. We would use yarn anytime it was present on the system, without checking to see if the project we are running in has a yarn.lock file. So, now we run the same logic to decide which to use, and set an environment variable to instruct electron-forge to match us.

Also, added end-to-end unit tests running with npm in addition to the ones running with yarn.

Fixes #308, #309, #312, #313

Author: Ben Demboski

.appveyor.yml

@@ -8,6 +8,7 @@ cache:
 
 install:
   - ps: Install-Product node $env:nodejs_version $env:platform
+  - npm install yarn -g
   - yarn
 
 test_script:

.travis.yml

@@ -21,7 +21,7 @@ cache:
   yarn: true
 
 before_install:
-  - curl -o- -L https://yarnpkg.com/install.sh | bash -s -- --version 0.27.5
+  - curl -o- -L https://yarnpkg.com/install.sh | bash -s -- --version 1.1.0
   - export PATH=$HOME/.yarn/bin:$PATH
   - yarn config set no-progress
 

blueprints/ember-electron/index.js

@@ -4,6 +4,7 @@ const { all, denodeify } = require('rsvp');
 
 const Blueprint = require('ember-cli/lib/models/blueprint');
 const efImport = require('electron-forge/dist/api/import').default;
+const { setupForgeEnv } = require('../../lib/utils/yarn-or-npm');
 
 const Logger = require('../../lib/utils/logger');
 
@@ -53,6 +54,7 @@ module.exports = class EmberElectronBlueprint extends Blueprint {
   _installElectronTooling(logger) {
     // n.b. addPackageToProject does not let us save prod deps, so we task
     let npmInstall = this.taskFor('npm-install');
+    setupForgeEnv(this.project.root);
 
     logger.startProgress('Installing electron build tools');
 

lib/tasks/assemble.js

@@ -2,10 +2,10 @@
 
 const chalk = require('chalk');
 const execa = require('execa');
-const { hasYarn } = require('yarn-or-npm');
 const Task = require('ember-cli/lib/models/task');
 const BuildTask = require('./build');
 const Assembler = require('../models/assembler');
+const { shouldUseYarn } = require('../utils/yarn-or-npm');
 
 //
 // A task for assembling an electron forge-compatible project by combining the
@@ -64,10 +64,10 @@ class AssembleTask extends Task {
   }
 
   pruneCommand() {
-    if (hasYarn()) {
+    if (shouldUseYarn(this.project.root)) {
       return 'yarn install --production --no-bin-links';
     } else {
-      return 'npm prune --production';
+      return 'npm install --production --no-bin-links';
     }
   }
 

lib/tasks/package.js

@@ -6,6 +6,7 @@ const { resolve } = require('rsvp');
 const quickTemp = require('quick-temp');
 const Task = require('ember-cli/lib/models/task');
 const AssembleTask = require('./assemble');
+const { setupForgeEnv } = require('../utils/yarn-or-npm');
 const forgePackage = require('electron-forge/dist/api/package').default;
 
 //
@@ -37,6 +38,7 @@ class PackageTask extends Task {
     }
 
     return assemblePromise.then(() => {
+      setupForgeEnv(this.project.root);
       ui.writeLine(chalk.green('Packaging Electron project.'));
 
       let options = {

lib/utils/yarn-or-npm.js

@@ -0,0 +1,23 @@
+const path = require('path');
+const { existsSync } = require('fs');
+const { hasYarn } = require('yarn-or-npm');
+
+//
+// This function mimics ember-cli's logic for deciding when to use yarn vs. npm
+//
+function shouldUseYarn(projectRoot) {
+  return existsSync(path.join(projectRoot, 'yarn.lock')) && hasYarn();
+}
+
+//
+// This function sets an environment variable that forces electron-forge to use
+// either yarn or npm, according to what shouldUseYarn() says.
+//
+function setupForgeEnv(projectRoot) {
+  process.env.NODE_INSTALLER = shouldUseYarn(projectRoot) ? 'yarn' : 'npm';
+}
+
+module.exports = {
+  shouldUseYarn,
+  setupForgeEnv,
+};

node-tests/acceptance/end-to-end-test.js

@@ -22,105 +22,143 @@ function run(cmd, args, opts = {}) {
 }
 
 describe('end-to-end', function() {
-  this.timeout(5 * 60 * 1000);
+  this.timeout(10 * 60 * 1000);
 
+  let oldEnv;
   let rootDir = process.cwd();
   let emberPath = path.join(rootDir, 'node_modules', '.bin', 'ember');
+  let { name: packageTmpDir } = tmp.dirSync();
 
   function ember(...args) {
     return listenForPrompts(run(emberPath, args, {
       stdio: ['pipe', 'pipe', process.stderr],
     }));
   }
 
-  before(function() {
-    this.timeout(10 * 60 * 1000);
-
-    let { name: tmpDir } = tmp.dirSync();
+  before(() => {
+    // If we're running via a yarn script like `yarn test`, then we'll have
+    // a whole bunch of npm_* environment variables set by yarn that can mess
+    // things up, so let's scrub them.
+    oldEnv = Object.assign({}, process.env);
+    Object.keys(process.env).forEach((key) => {
+      if (key.startsWith('npm_')) {
+        delete process.env[key];
+      }
+    });
 
-    return run('yarn', ['pack', '--filename', path.join(tmpDir, 'ember-electron.tgz')]).then(() => {
-      process.chdir(tmpDir);
+    //
+    // Pack up current ember-electron directory so it can be installed in new
+    // ember projects.
+    //
+    return run('yarn', ['pack', '--filename', path.join(packageTmpDir, 'ember-electron.tgz')]).then(() => {
+      process.chdir(packageTmpDir);
 
-      // yarn won't install from a gzipped tarball, and try as I might I can't
-      // get node-tar or tar.gz to untar the tarballs created by yarn or npm
       return run('tar', ['-xzf', 'ember-electron.tgz']);
     }).then(() => {
       // Prevent yarn caching from screwing us
       let packageJson = readJsonSync(path.join('package', 'package.json'));
-      packageJson.version = `packageJson.version-${new Date().getTime()}`;
+      packageJson.version = `${packageJson.version}-${new Date().getTime()}`;
       writeJsonSync(path.join('package', 'package.json'), packageJson);
-
-      return ember('new', 'ee-test-app', '--yarn');
-    }).then(() => {
-      process.chdir('ee-test-app');
-
-      return ember('install', `ember-electron@file:${tmpDir}/package`);
-    }).then(() => {
-      // yarn has some kind of bug that causes it to hoist the wrong version of
-      // npmlog during one of the operations performed by the ember-electron
-      // blueprint, which causes things to explode. Running yarn upgrade
-      // corrects the situation.
-      return run('yarn', ['upgrade']);
     });
   });
 
   after(() => {
-    process.chdir(rootDir);
+    process.env = oldEnv;
   });
 
   afterEach(() => {
     removeSync('electron-out');
   });
 
-  it('tests', () => {
-    return expect(ember('electron:test')).to.eventually.be.fulfilled;
-  });
+  describe('with yarn', function() {
+    before(function() {
+      let { name: tmpDir } = tmp.dirSync();
+      process.chdir(tmpDir);
+
+      return ember('new', 'ee-test-app', '--yarn').then(() => {
+        process.chdir('ee-test-app');
 
-  it('builds', () => {
-    return ember('electron:build').then(() => {
-      expect(existsSync(path.join('electron-out', 'ember'))).to.be.ok;
+        return ember('install', `ember-electron@${path.join(packageTmpDir, 'package')}`);
+      });
     });
-  });
 
-  it('assembles', () => {
-    return ember('electron:assemble').then(() => {
-      expect(existsSync(path.join('electron-out', 'project'))).to.be.ok;
+    after(() => {
+      process.chdir(rootDir);
     });
+
+    runTests();
   });
 
-  it('packages', () => {
-    return ember('electron:package').then(() => {
-      expect(existsSync(path.join('electron-out', `ee-test-app-${process.platform}-${process.arch}`))).to.be.ok;
+  describe('with npm', function() {
+    before(function() {
+      let { name: tmpDir } = tmp.dirSync();
+      process.chdir(tmpDir);
+
+      return ember('new', 'ee-test-app').then(() => {
+        process.chdir('ee-test-app');
+
+        return ember('install', `ember-electron@${path.join(packageTmpDir, 'package')}`);
+      });
     });
-  });
 
-  it('makes', () => {
-    // Only build zip target so we don't fail from missing platform dependencies
-    // (e.g. rpmbuild)
-    return ember('electron:make', '--targets', 'zip').then(() => {
-      expect(existsSync(path.join('electron-out', 'make'))).to.be.ok;
+    after(() => {
+      process.chdir(rootDir);
     });
+
+    runTests();
   });
 
-  it('extra checks pass', () => {
-    let fixturePath = path.resolve(__dirname, '..', 'fixtures', 'ember-test');
-
-    // Append our extra test content to the end of test-main.js
-    let testMainPath = path.join('ember-electron', 'test-main.js');
-    let extraContentPath = path.join(fixturePath, 'test-main-extra.js');
-    let content = [
-      readFileSync(testMainPath),
-      readFileSync(extraContentPath),
-    ].join('\n');
-    writeFileSync(path.join('ember-electron', 'test-main.js'), content);
-
-    // Copy the lib and resources directories over
-    ['lib', 'resources'].forEach((dir) => {
-      copySync(path.join(fixturePath, dir), path.join('ember-electron', dir));
+  function runTests() {
+    it('tests', () => {
+      return expect(ember('electron:test')).to.eventually.be.fulfilled;
     });
 
-    return expect(ember('electron:test')).to.eventually.be.fulfilled;
-  });
+    it('builds', () => {
+      return ember('electron:build').then(() => {
+        expect(existsSync(path.join('electron-out', 'ember'))).to.be.ok;
+      });
+    });
+
+    it('assembles', () => {
+      return ember('electron:assemble').then(() => {
+        expect(existsSync(path.join('electron-out', 'project'))).to.be.ok;
+      });
+    });
+
+    it('packages', () => {
+      return ember('electron:package').then(() => {
+        expect(existsSync(path.join('electron-out', `ee-test-app-${process.platform}-${process.arch}`))).to.be.ok;
+      });
+    });
+
+    it('makes', () => {
+      // Only build zip target so we don't fail from missing platform dependencies
+      // (e.g. rpmbuild)
+      return ember('electron:make', '--targets', 'zip').then(() => {
+        expect(existsSync(path.join('electron-out', 'make'))).to.be.ok;
+      });
+    });
+
+    it('extra checks pass', () => {
+      let fixturePath = path.resolve(__dirname, '..', 'fixtures', 'ember-test');
+
+      // Append our extra test content to the end of test-main.js
+      let testMainPath = path.join('ember-electron', 'test-main.js');
+      let extraContentPath = path.join(fixturePath, 'test-main-extra.js');
+      let content = [
+        readFileSync(testMainPath),
+        readFileSync(extraContentPath),
+      ].join('\n');
+      writeFileSync(path.join('ember-electron', 'test-main.js'), content);
+
+      // Copy the lib and resources directories over
+      ['lib', 'resources'].forEach((dir) => {
+        copySync(path.join(fixturePath, dir), path.join('ember-electron', dir));
+      });
+
+      return expect(ember('electron:test')).to.eventually.be.fulfilled;
+    });
+  }
 });
 
 function listenForPrompts(child) {

node-tests/unit/tasks/assemble-test.js

@@ -19,6 +19,9 @@ describe('AssembleTask', () => {
   let assemblerOptions;
   let assemblerFail;
 
+  let useYarn;
+
+  let installCommand;
   let installOptions;
 
   let task;
@@ -56,6 +59,7 @@ describe('AssembleTask', () => {
 
   function mockInstall(command, args, opts) {
     operations.push('install-dependencies');
+    installCommand = command;
     installOptions = opts;
 
     return resolve();
@@ -83,6 +87,7 @@ describe('AssembleTask', () => {
 
     mockery.registerMock('./build', MockBuildTask);
     mockery.registerMock('../models/assembler', MockAssembler);
+    mockery.registerMock('../utils/yarn-or-npm', { shouldUseYarn: () => useYarn });
     mockery.registerMock('execa', mockInstall);
 
     const AssembleTask = require('../../../lib/tasks/assemble');
@@ -182,6 +187,28 @@ describe('AssembleTask', () => {
     });
   });
 
+  it('should use yarn when appropriate', () => {
+    let options = {
+      outputPath: 'output',
+    };
+    useYarn = true;
+
+    expect(task.run(options)).to.be.rejected.then(() => {
+      expect(installCommand).to.equal('yarn');
+    });
+  });
+
+  it('should use npm when appropriate', () => {
+    let options = {
+      outputPath: 'output',
+    };
+    useYarn = false;
+
+    expect(task.run(options)).to.be.rejected.then(() => {
+      expect(installCommand).to.equal('npm');
+    });
+  });
+
   it('should fail on assemble failures', () => {
     let options = {
       buildPath: 'ember-build',