Repository: ember-cli/ember-cli
Pull request #8941 (2019-11-25 16:06:31)

feat(blueprint): resolve remote blueprints via package manager
Builds on and closes #8927.

This PR refactors the `install-blueprint` task to always use npm for installing remote blueprints. This means that users can now invoke `ember new -b` (and friends) with exactly the same package specifiers as they would pass to `npm` / `yarn`. Valid invocations include, but are not limited to:

- `ember new foo -b ember-cli/app-blueprint-test`
- `ember new foo -b ember-cli/app-blueprint-test#named-ref`
- `ember new foo -b gitlab:foo/bar`
- `ember new foo -b https://github.com/ember-cli/app-blueprint-test.git#named-ref`

/cc @rwjblue 

buschtoens: Hm, a lot of tests in ['unit/tasks/install-blueprint-test.js'](https://github.com/ember-cli/ember-cli/blob/master/tests/unit/tasks/install-blueprint-test.js) are now failing, of course. TBH I'm not really sure whether they actually add a lot of value, since most of the heavy lifting is done by npm and we have sufficient coverage by the acceptance test.
rwjblue: Ya, I'm game to remove the unit tests (since we are actually removing our custom functionality for this also).
buschtoens: Seems like there's a valid failure with Node v8. I'll look into it.
buschtoens: So turns out that `npm@6.4.1`, the version that ships with `node@8`, is buggy. 🙄 

My (apparently too) clever trick to install the package of unknown name into a predefined location does not reliably work with git dependencies:

```json
{
  "dependencies": {
    "ember-cli-blueprint-1574873174904": "git+https://github.com/ember-cli/app-blueprint-test.git"
  }
}
```

In the current `npm` version the above works as expected and installs the blueprint to:

```
node_modules/ember-cli-blueprint-1574873174904/
```

In `npm@6.4.1` only the `package-lock.json` is created and `node_modules` is missing.
_Interestingly_, when I change `npm install` to `npm install --no-package-lock`, the `node_modules` directory _is_ created. Unfortunately the package is not installed at the predefined location but at:

```
node_modules/ember-blueprint-demo/
```

Which is the name used in the repo's [`package.json`](https://github.com/ember-cli/app-blueprint-test/blob/master/package.json#L2).

---

One solution would be:

```sh
echo "{}" > package.json
npm install --no-package-lock --save https://github.com/ember-cli/app-blueprint-test.git
```

This would create a `package.json` with only a single dependency, that has the correct name:

```json
{
  "dependencies": {
    "ember-blueprint-demo": "git+https://github.com/ember-cli/app-blueprint-test.git"
  }
}
```

Patch:
diff --git a/lib/tasks/install-blueprint.js b/lib/tasks/install-blueprint.js
index b07a42792..2ad74fdbc 100644
--- a/lib/tasks/install-blueprint.js
+++ b/lib/tasks/install-blueprint.js
@@ -4,7 +4,6 @@ const fs = require('fs-extra');
 const Blueprint = require('../models/blueprint');
 const Task = require('../models/task');
 const RSVP = require('rsvp');
-const isGitRepo = require('is-git-url');
 const temp = require('temp');
 const path = require('path');
 const merge = require('ember-cli-lodash-subset').merge;
@@ -54,19 +53,11 @@ class InstallBlueprintTask extends Task {
     name = name || 'app';
     logger.info(`Resolving blueprint "${name}" ...`);
 
-    if (!isGitRepo(name)) {
-      return this._lookupBlueprint(name).catch(error => this._handleLookupFailure(name, error));
-    }
-
-    return this._createTempFolder().then(pathName =>
-      this._gitClone(name, pathName)
-        .then(() => this._npmInstall(pathName))
-        .then(() => this._loadBlueprintFromPath(pathName))
-    );
+    return this._lookupLocalBlueprint(name).catch(error => this._handleLocalLookupFailure(name, error));
   }
 
-  _lookupBlueprint(name) {
-    logger.info(`Looking up blueprint "${name}" ...`);
+  _lookupLocalBlueprint(name) {
+    logger.info(`Looking up blueprint "${name}" locally...`);
     return RSVP.resolve().then(() =>
       Blueprint.lookup(name, {
         paths: this.project.blueprintLookupPaths(),
@@ -74,56 +65,99 @@ class InstallBlueprintTask extends Task {
     );
   }
 
-  _handleLookupFailure(name, error) {
-    logger.info(`Blueprint lookup for "${name}" failed`);
+  _handleLocalLookupFailure(name, error) {
+    logger.info(`Local blueprint lookup for "${name}" failed`);
 
-    let parsed;
     try {
-      parsed = npa(name);
+      npa(name);
     } catch (err) {
-      logger.info(`"${name} is not a valid npm package name -> rethrowing original error`);
+      logger.info(`"${name} is not a valid npm package specifier -> rethrowing original error`);
       throw error;
     }
 
-    logger.info(`"${name} is a valid npm package name -> trying npm`);
-    return this._tryNpmBlueprint(parsed.name, parsed.fetchSpec);
+    logger.info(`"${name} is a valid npm package specifier -> trying npm`);
+    return this._tryRemoteBlueprint(name);
   }
 
-  _tryNpmBlueprint(nameWithoutVersion, version) {
-    return this._createTempFolder()
-      .then(pathName =>
-        this._npmInstallModule(nameWithoutVersion, version, pathName).catch(error =>
-          this._handleNpmInstallModuleError(error)
-        )
-      )
-      .then(modulePath => {
-        this._validateNpmModule(modulePath, nameWithoutVersion);
-        return this._loadBlueprintFromPath(modulePath);
-      });
+  async _tryRemoteBlueprint(name) {
+    let tmpDir = await this._createTempFolder();
+    await this._createEmptyPackageJSON(tmpDir, name);
+
+    try {
+      await this._npmInstall(tmpDir, name);
+    } catch (error) {
+      this._handleNpmInstallModuleError(error); // re-throws
+    }
+
+    let packageName = await this._readSingleDependencyFromPackageJSON(tmpDir);
+    let blueprintPath = path.resolve(tmpDir, 'node_modules', packageName);
+    this._validateNpmModule(blueprintPath, packageName);
+
+    return this._loadBlueprintFromPath(blueprintPath);
   }
 
   _createTempFolder() {
     return mkdirTemp('ember-cli');
   }
 
-  _gitClone(source, destination) {
-    logger.info(`Cloning from git (${source}) into "${destination}" ...`);
-    return execa('git', ['clone', source, destination]);
+  _resolvePackageJSON(directoryPath) {
+    return path.resolve(directoryPath, 'package.json');
   }
 
-  _npmInstall(cwd) {
-    logger.info(`Running "npm install" in "${cwd}" ...`);
+  async _createEmptyPackageJSON(tempProjectPath, blueprintName) {
+    return fs.writeFile(
+      this._resolvePackageJSON(tempProjectPath),
+      JSON.stringify({
+        name: 'ember-cli-blueprint-install-wrapper',
+        version: '0.0.0',
+        description: `This is a transient package used to fetch this blueprint: ${blueprintName}`,
+        license: 'UNLICENSED',
+        private: true,
+        dependencies: {},
+      })
+    );
+  }
 
-    this._copyNpmrc(cwd);
-    return execa('npm', ['install'], { cwd });
+  async _readSingleDependencyFromPackageJSON(tempProjectPath) {
+    let packageJSON = JSON.parse(await fs.readFile(this._resolvePackageJSON(tempProjectPath), 'utf8'));
+
+    if (!packageJSON.dependencies || typeof packageJSON.dependencies !== 'object') {
+      throw new SilentError(`The transient package.json at '${tempProjectPath}' is missing 'dependencies'.`);
+    }
+
+    let dependencyNames = Object.keys(packageJSON.dependencies);
+
+    if (dependencyNames.length !== 1) {
+      throw new SilentError(
+        `The transient package.json at '${tempProjectPath}' has more than one or no entries in 'dependencies'.`
+      );
+    }
+
+    return dependencyNames[0];
   }
 
-  _npmInstallModule(nameWithoutVersion, version, cwd) {
-    let module = `${nameWithoutVersion}@${version}`;
-    logger.info(`Running "npm install ${module}" in "${cwd}" ...`);
+  async _createPackageJSON(tempProjectPath, descriptor) {
+    let packageName = descriptor.name || `ember-cli-blueprint-${Date.now()}`;
+    let specifier = descriptor.saveSpec || descriptor.fetchSpec;
+
+    await fs.writeFile(
+      this._resolvePackageJSON(tempProjectPath),
+      JSON.stringify({
+        dependencies: {
+          [packageName]: specifier,
+        },
+      })
+    );
+
+    return packageName;
+  }
+
+  _npmInstall(cwd, name) {
+    logger.info(`Running "npm install" for "${name}" in "${cwd}" ...`);
 
     this._copyNpmrc(cwd);
-    return execa('npm', ['install', module], { cwd }).then(() => path.join(cwd, 'node_modules', nameWithoutVersion));
+
+    return execa('npm', ['install', '--no-package-lock', '--save', name], { cwd });
   }
 
   _handleNpmInstallModuleError(error) {
@@ -138,7 +172,7 @@ class InstallBlueprintTask extends Task {
 
   _validateNpmModule(modulePath, packageName) {
     logger.info(`Checking for "ember-blueprint" keyword in "${packageName}" module ...`);
-    let pkg = require(path.join(modulePath, 'package.json'));
+    let pkg = require(this._resolvePackageJSON(modulePath));
     if (!pkg || !pkg.keywords || pkg.keywords.indexOf('ember-blueprint') === -1) {
       throw new SilentError(`The package '${packageName}' is not a valid Ember CLI blueprint.`);
     }


Test patch:
diff --git a/tests/acceptance/new-test.js b/tests/acceptance/new-test.js
index 1f84254e0..055fe423c 100644
--- a/tests/acceptance/new-test.js
+++ b/tests/acceptance/new-test.js
@@ -234,6 +234,42 @@ describe('Acceptance: ember new', function() {
     })
   );
 
+  it(
+    'ember new with git blueprint and ref checks out the blueprint with the correct ref and uses it',
+    co.wrap(function*() {
+      this.timeout(20000); // relies on GH network stuff
+
+      yield ember([
+        'new',
+        'foo',
+        '--skip-npm',
+        '--skip-bower',
+        '--skip-git',
+        '--blueprint=https://github.com/ember-cli/app-blueprint-test.git#named-ref',
+      ]);
+
+      expect(file('.named-ref')).to.exist;
+    })
+  );
+
+  it(
+    'ember new with shorthand git blueprint and ref checks out the blueprint with the correct ref and uses it',
+    co.wrap(function*() {
+      this.timeout(20000); // relies on GH network stuff
+
+      yield ember([
+        'new',
+        'foo',
+        '--skip-npm',
+        '--skip-bower',
+        '--skip-git',
+        '--blueprint=ember-cli/app-blueprint-test#named-ref',
+      ]);
+
+      expect(file('.named-ref')).to.exist;
+    })
+  );
+
   it(
     'ember new passes blueprint options through to blueprint',
     co.wrap(function*() {
diff --git a/tests/unit/tasks/install-blueprint-test.js b/tests/unit/tasks/install-blueprint-test.js
deleted file mode 100644
index c9c7e854d..000000000
--- a/tests/unit/tasks/install-blueprint-test.js
+++ /dev/null
@@ -1,239 +0,0 @@
-'use strict';
-
-const path = require('path');
-const fs = require('fs-extra');
-const td = require('testdouble');
-const SilentError = require('silent-error');
-
-const expect = require('../../chai').expect;
-const mkTmpDirIn = require('../../../lib/utilities/mk-tmp-dir-in');
-const InstallBlueprintTask = require('../../../lib/tasks/install-blueprint');
-
-let root = path.join(__dirname, '../../..');
-let tmproot = path.join(root, 'tmp');
-
-describe('InstallBlueprintTask', function() {
-  let task;
-  beforeEach(function() {
-    task = new InstallBlueprintTask();
-  });
-
-  describe('_resolveBlueprint', function() {
-    beforeEach(function() {
-      task._lookupBlueprint = td.function();
-      task._tryNpmBlueprint = td.function();
-      task._createTempFolder = td.function();
-      task._gitClone = td.function();
-      task._npmInstall = td.function();
-      task._loadBlueprintFromPath = td.function();
-
-      task._tempPath = '/tmp/foobar';
-      td.when(task._createTempFolder()).thenResolve(task._tempPath);
-    });
-
-    it('resolves "foobar" by looking up the "foobar" blueprint locally', async function() {
-      let foobarBlueprint = { name: 'foobar blueprint' };
-      td.when(task._lookupBlueprint('foobar')).thenResolve(foobarBlueprint);
-
-      expect(await task._resolveBlueprint('foobar')).to.equal(foobarBlueprint);
-    });
-
-    it('rejects invalid npm package name "foo:bar"', async function() {
-      let error = new Error('foobar not found');
-      td.when(task._lookupBlueprint('foo:bar')).thenReject(error);
-
-      await expect(task._resolveBlueprint('foo:bar')).to.be.rejectedWith(error);
-    });
-
-    it('tries to resolve "foobar" as npm package as a fallback', async function() {
-      let error = new Error('foobar not found');
-      td.when(task._lookupBlueprint('foobar')).thenReject(error);
-
-      let foobarBlueprint = { name: 'foobar npm blueprint' };
-      td.when(task._tryNpmBlueprint('foobar', 'latest')).thenResolve(foobarBlueprint);
-
-      expect(await task._resolveBlueprint('foobar')).to.equal(foobarBlueprint);
-    });
-
-    it('tries to resolve "@foo/bar@1.2.3" as npm package with a scope and a version', async function() {
-      let error = new Error('@foo/bar@1.2.3 not found');
-      td.when(task._lookupBlueprint('@foo/bar@1.2.3')).thenReject(error);
-
-      let foobarBlueprint = { name: 'foobar npm blueprint' };
-      td.when(task._tryNpmBlueprint('@foo/bar', '1.2.3')).thenResolve(foobarBlueprint);
-
-      expect(await task._resolveBlueprint('@foo/bar@1.2.3')).to.equal(foobarBlueprint);
-    });
-
-    it('rejects if npm module resolution failed', async function() {
-      let error = new Error('foobar not found');
-      td.when(task._lookupBlueprint('foobar')).thenReject(error);
-
-      let npmError = new Error('npm failure');
-      td.when(task._tryNpmBlueprint('foobar', 'latest')).thenReject(npmError);
-
-      await expect(task._resolveBlueprint('foobar')).to.be.rejectedWith(npmError);
-    });
-
-    it(
-      'resolves "https://github.com/ember-cli/app-blueprint-test.git" blueprint by cloning, ' +
-        'installing dependencies and loading the blueprint',
-      async function() {
-        let url = 'https://github.com/ember-cli/app-blueprint-test.git';
-        let gitBlueprint = { name: 'git blueprint' };
-        td.when(task._gitClone(url, task._tempPath)).thenResolve();
-        td.when(task._npmInstall(task._tempPath)).thenResolve();
-        td.when(task._loadBlueprintFromPath(task._tempPath)).thenResolve(gitBlueprint);
-
-        expect(await task._resolveBlueprint(url)).to.equal(gitBlueprint);
-      }
-    );
-
-    it('rejects if temp folder creation fails', async function() {
-      let url = 'https://github.com/ember-cli/app-blueprint-test.git';
-      let error = new Error('temp folder creation failed');
-      td.when(task._createTempFolder()).thenReject(error);
-
-      await expect(task._resolveBlueprint(url)).to.be.rejectedWith(error);
-    });
-
-    it('rejects if "git clone" fails', function() {
-      let url = 'https://github.com/ember-cli/app-blueprint-test.git';
-      let error = new Error('git clone failed');
-      td.when(task._gitClone(url, task._tempPath)).thenReject(error);
-
-      return expect(task._resolveBlueprint(url)).to.be.rejectedWith(error);
-    });
-
-    it('rejects if "npm install" fails', async function() {
-      let url = 'https://github.com/ember-cli/app-blueprint-test.git';
-      let error = new Error('npm install failed');
-      td.when(task._gitClone(url, task._tempPath)).thenResolve();
-      td.when(task._npmInstall(task._tempPath)).thenReject(error);
-
-      await expect(task._resolveBlueprint(url)).to.be.rejectedWith(error);
-    });
-
-    it('rejects if loading the blueprint fails', async function() {
-      let url = 'https://github.com/ember-cli/app-blueprint-test.git';
-      let error = new Error('loading blueprint failed');
-      td.when(task._gitClone(url, task._tempPath)).thenResolve();
-      td.when(task._npmInstall(task._tempPath)).thenResolve();
-      td.when(task._loadBlueprintFromPath(task._tempPath)).thenReject(error);
-
-      await expect(task._resolveBlueprint(url)).to.be.rejectedWith(error);
-    });
-  });
-
-  describe('_tryNpmBlueprint', function() {
-    beforeEach(async function() {
-      task._createTempFolder = td.function();
-      task._npmInstallModule = td.function();
-      task._validateNpmModule = td.function();
-      task._loadBlueprintFromPath = td.function();
-
-      task._tempPath = '/tmp/foobar';
-      td.when(task._createTempFolder()).thenResolve(task._tempPath);
-
-      const tmpdir = await mkTmpDirIn(tmproot);
-
-      task.project = { root: tmpdir };
-      process.chdir(tmpdir);
-    });
-
-    afterEach(function() {
-      process.chdir(root);
-      return fs.remove(tmproot);
-    });
-
-    it('if .npmrc exists in the project root it copys it to tmp location', function() {
-      let dir = 'foo';
-
-      fs.writeFileSync(path.join(task.project.root, '.npmrc'), 'foo');
-
-      if (!fs.existsSync(dir)) {
-        fs.mkdirSync(dir);
-      }
-
-      task._copyNpmrc(dir);
-      expect(fs.existsSync(path.join(task.project.root, dir, '.npmrc'))).to.be.true;
-    });
-
-    it('if .npmrc does not exists in the project root it is not copied', function() {
-      let dir = 'foo';
-
-      if (!fs.existsSync(dir)) {
-        fs.mkdirSync(dir);
-      }
-
-      task._copyNpmrc(dir);
-      expect(fs.existsSync(path.join(task.project.root, dir, '.npmrc'))).to.be.false;
-    });
-
-    it('resolves with blueprint after successful "npm install"', async function() {
-      let modulePath = '/path/to/foobar';
-      td.when(task._npmInstallModule('foobar', 'latest', task._tempPath)).thenResolve(modulePath);
-
-      let foobarBlueprint = { name: 'foobar blueprint' };
-      td.when(task._loadBlueprintFromPath(modulePath)).thenResolve(foobarBlueprint);
-
-      expect(await task._tryNpmBlueprint('foobar', 'latest')).to.equal(foobarBlueprint);
-    });
-
-    it('resolves with blueprint after successful "npm install" with a scope and a version', async function() {
-      let modulePath = '/path/to/@foo/bar';
-      td.when(task._npmInstallModule('@foo/bar', '1.2.3', task._tempPath)).thenResolve(modulePath);
-
-      let foobarBlueprint = { name: '@foo/bar blueprint' };
-      td.when(task._loadBlueprintFromPath(modulePath)).thenResolve(foobarBlueprint);
-
-      expect(await task._tryNpmBlueprint('@foo/bar', '1.2.3')).to.equal(foobarBlueprint);
-    });
-
-    it('rejects with SilentError if npm module "foobar" could not be found', async function() {
-      let error = new Error();
-      error.stderr = `
-          npm ERR! 404 Registry returned 404 for GET on https://registry.npmjs.org/ember-cli-app-blueprint-tes
-          npm ERR! 404
-          npm ERR! 404  'foobar' is not in the npm registry.
-          npm ERR! 404 You should bug the author to publish it (or use the name yourself!)
-          npm ERR! 404
-          npm ERR! 404 Note that you can also install from a
-          npm ERR! 404 tarball, folder, http url, or git url.`;
-
-      td.when(task._npmInstallModule('foobar', 'latest', task._tempPath)).thenReject(error);
-
-      await expect(task._tryNpmBlueprint('foobar', 'latest')).to.be.rejectedWith(
-        SilentError,
-        `The package 'foobar' was not found in the npm registry.`
-      );
-    });
-
-    it('rejects if "npm install" fails', async function() {
-      let error = new Error('npm install failed');
-      td.when(task._npmInstallModule('foobar', 'latest', task._tempPath)).thenReject(error);
-
-      await expect(task._tryNpmBlueprint('foobar', 'latest')).to.be.rejectedWith(error);
-    });
-
-    it('rejects if npm module validation fails', async function() {
-      let modulePath = '/path/to/foobar';
-      td.when(task._npmInstallModule('foobar', 'latest', task._tempPath)).thenResolve(modulePath);
-
-      let error = new Error('module validation failed');
-      td.when(task._validateNpmModule(modulePath, 'foobar')).thenThrow(error);
-
-      await expect(task._tryNpmBlueprint('foobar', 'latest')).to.be.rejectedWith(error);
-    });
-
-    it('rejects if loading blueprint fails', async function() {
-      let modulePath = '/path/to/foobar';
-      td.when(task._npmInstallModule('foobar', 'latest', task._tempPath)).thenResolve(modulePath);
-
-      let error = new Error('loading blueprint failed');
-      td.when(task._loadBlueprintFromPath(modulePath)).thenReject(error);
-
-      await expect(task._tryNpmBlueprint('foobar', 'latest')).to.be.rejectedWith(error);
-    });
-  });
-});
diff --git a/tests/unit/utilities/git-repo-test.js b/tests/unit/utilities/git-repo-test.js
index 12df4c71d..1448162d7 100644
--- a/tests/unit/utilities/git-repo-test.js
+++ b/tests/unit/utilities/git-repo-test.js
@@ -5,9 +5,16 @@ const expect = require('chai').expect;
 
 describe('is-git-url', function() {
   it('recognizes git-style urls in various formats', function() {
+    // without ref
     expect(isGitRepo('https://github.com/trek/app-blueprint-test.git')).to.be.ok;
     expect(isGitRepo('git@github.com:trek/app-blueprint-test.git')).to.be.ok;
     expect(isGitRepo('git+ssh://user@server/project.git')).to.be.ok;
     expect(isGitRepo('git+https://user@server/project.git')).to.be.ok;
+
+    // with ref
+    expect(isGitRepo('https://github.com/trek/app-blueprint-test.git#named-ref')).to.be.ok;
+    expect(isGitRepo('git@github.com:trek/app-blueprint-test.git#named-ref')).to.be.ok;
+    expect(isGitRepo('git+ssh://user@server/project.git#named-ref')).to.be.ok;
+    expect(isGitRepo('git+https://user@server/project.git#named-ref')).to.be.ok;
   });
 });