feat: The web-ext build and web-ext run commands can handle exten…

…sions without explicit IDs

.eslintrc

@@ -61,7 +61,7 @@
     "no-redeclare": 0,
     "no-script-url": 2,
     "no-sequences": 2,
-    "no-trailing-spaces": [2, {"skipBlankLines": true}],
+    "no-trailing-spaces": [2, {"skipBlankLines": false}],
     "no-undef": 2,
     "no-underscore-dangle": 0,
     "no-unused-vars": 2,

src/cmd/build.js

@@ -6,7 +6,7 @@ import streamToPromise from 'stream-to-promise';
 
 import defaultSourceWatcher from '../watcher';
 import {zipDir} from '../util/zip-dir';
-import getValidatedManifest from '../util/manifest';
+import getValidatedManifest, {getManifestId} from '../util/manifest';
 import {prepareArtifactsDir} from '../util/artifacts';
 import {createLogger} from '../util/logger';
 
@@ -19,7 +19,8 @@ function defaultPackageCreator(
   return new Promise(
     (resolve) => {
       if (manifestData) {
-        log.debug(`Using manifest id=${manifestData.applications.gecko.id}`);
+        const id = getManifestId(manifestData);
+        log.debug(`Using manifest id=${id || '[not specified]'}`);
         resolve(manifestData);
       } else {
         resolve(getValidatedManifest(sourceDir));

src/cmd/run.js

@@ -6,24 +6,19 @@ import {
   WebExtError,
 } from '../errors';
 import {createLogger} from '../util/logger';
-import getValidatedManifest from '../util/manifest';
+import getValidatedManifest, {getManifestId} from '../util/manifest';
 import defaultSourceWatcher from '../watcher';
 
 const log = createLogger(__filename);
 
 
 export function defaultWatcherCreator(
-    {client, sourceDir, artifactsDir, createRunner,
+    {addonId, client, sourceDir, artifactsDir,
      onSourceChange=defaultSourceWatcher}: Object): Object {
   return onSourceChange({
     sourceDir, artifactsDir, onChange: () => {
-      return createRunner()
-        .then((runner) => {
-          log.debug('Attempting to reload extension');
-          const addonId = runner.manifestData.applications.gecko.id;
-          log.debug(`Reloading add-on ID ${addonId}`);
-          return client.reloadAddon(addonId);
-        })
+      log.debug(`Reloading add-on ID ${addonId}`);
+      return client.reloadAddon(addonId)
         .catch((error) => {
           log.error(error.stack);
           throw error;
@@ -34,7 +29,7 @@ export function defaultWatcherCreator(
 
 
 export function defaultReloadStrategy(
-    {firefox, client, profile, sourceDir, artifactsDir, createRunner}: Object,
+    {addonId, firefox, client, profile, sourceDir, artifactsDir}: Object,
     {createWatcher=defaultWatcherCreator}: Object = {}) {
   let watcher;
 
@@ -43,9 +38,7 @@ export function defaultReloadStrategy(
     watcher.close();
   });
 
-  watcher = createWatcher({
-    client, sourceDir, artifactsDir, createRunner,
-  });
+  watcher = createWatcher({addonId, client, sourceDir, artifactsDir});
 }
 
 
@@ -103,22 +96,18 @@ export default function run(
   // When not pre-installing the extension, we require a remote
   // connection to Firefox.
   const requiresRemote = !preInstall;
+  let installed = false;
 
-  function createRunner() {
-    return getValidatedManifest(sourceDir)
-      .then((manifestData) => {
-        return new ExtensionRunner({
-          sourceDir,
-          firefox,
-          firefoxBinary,
-          manifestData,
-          firefoxProfile,
-        });
+  return getValidatedManifest(sourceDir)
+    .then((manifestData) => {
+      return new ExtensionRunner({
+        sourceDir,
+        firefox,
+        firefoxBinary,
+        manifestData,
+        firefoxProfile,
       });
-  }
-
-  let installed = false;
-  return createRunner()
+    })
     .then((runner) => {
       return runner.getProfile().then((profile) => {
         return {runner, profile};
@@ -131,19 +120,20 @@ export default function run(
           if (!preInstall) {
             log.debug('Deferring extension installation until after ' +
                       'connecting to the remote debugger');
-            resolve();
+            resolve(config);
           } else {
             log.debug('Pre-installing extension as a proxy file');
-            resolve(runner.installAsProxy(profile).then(() => {
+            resolve(runner.installAsProxy(profile).then((addonId) => {
               installed = true;
+              return {addonId, ...config};
             }));
           }
-        })
-        .then(() => config);
+        });
     })
-    .then(({runner, profile}) => {
+    .then((config) => {
+      const {runner, profile} = config;
       return runner.run(profile).then((firefox) => {
-        return {runner, profile, firefox};
+        return {firefox, ...config};
       });
     })
     .then((config) => {
@@ -156,19 +146,17 @@ export default function run(
       }
     })
     .then((config) => {
-      return new Promise(
-        (resolve) => {
-          const {runner} = config;
-          if (installed) {
-            log.debug('Not installing as temporary add-on because the ' +
-                      'add-on was already installed');
-            resolve();
-          } else {
-            const {client} = config;
-            resolve(runner.installAsTemporaryAddon(client));
-          }
-        })
-        .then(() => config);
+      if (installed) {
+        log.debug('Not installing as temporary add-on because the ' +
+                  'add-on was already installed');
+        return config;
+      } else {
+        const {runner, client} = config;
+        return runner.installAsTemporaryAddon(client)
+          .then((installResult) => {
+            return {addonId: installResult.addon.id, ...config};
+          });
+      }
     })
     .catch(onlyInstancesOf(RemoteTempInstallNotSupported, (error) => {
       log.debug(`Caught: ${error}`);
@@ -177,13 +165,15 @@ export default function run(
         'of Firefox (you need Firefox 49 or higher). For older Firefox ' +
         'versions, use --pre-install');
     }))
-    .then(({firefox, profile, client}) => {
+    .then(({firefox, profile, client, addonId}) => {
       if (noReload) {
         log.debug('Extension auto-reloading has been disabled');
       } else {
-        log.debug('Reloading extension when the source changes');
-        reloadStrategy({firefox, profile, client, sourceDir,
-                        artifactsDir, createRunner});
+        log.debug(
+          `Reloading extension when the source changes; id=${addonId}`);
+        reloadStrategy({
+          firefox, profile, client, sourceDir, artifactsDir, addonId,
+        });
       }
       return firefox;
     });
@@ -225,12 +215,14 @@ export class ExtensionRunner {
 
   installAsProxy(profile: Object): Promise {
     const {firefox, sourceDir, manifestData} = this;
-    return firefox.installExtension({
-      manifestData,
-      asProxy: true,
-      extensionPath: sourceDir,
-      profile,
-    });
+    return firefox.installExtension(
+      {
+        manifestData,
+        asProxy: true,
+        extensionPath: sourceDir,
+        profile,
+      })
+      .then(() => getManifestId(manifestData));
   }
 
   run(profile: Object): Promise {

src/cmd/sign.js

@@ -2,6 +2,7 @@
 import {signAddon as defaultAddonSigner} from '../util/es6-modules';
 
 import defaultBuilder from './build';
+import {InvalidManifest} from '../errors';
 import {withTempDir} from '../util/temp-dir';
 import getValidatedManifest from '../util/manifest';
 import {prepareArtifactsDir} from '../util/artifacts';
@@ -26,6 +27,15 @@ export default function sign(
             return getValidatedManifest(sourceDir);
           }
         })
+        .then((manifestData) => {
+          if (!manifestData.applications) {
+            // TODO: remove this when signing supports manifests
+            // without IDs: https://github.com/mozilla/web-ext/issues/178
+            throw new InvalidManifest(
+              'applications.gecko.id in manifest.json is required for signing');
+          }
+          return manifestData;
+        })
         .then((manifestData) => {
           return build(
             {sourceDir, artifactsDir: tmpDir.path()},

src/firefox/index.js

@@ -11,6 +11,7 @@ import isDirectory from '../util/is-directory';
 import {promisify} from '../util/es6-modules';
 import {onlyErrorsWithCode, WebExtError} from '../errors';
 import {getPrefs as defaultPrefGetter} from './preferences';
+import {getManifestId} from '../util/manifest';
 import {createLogger} from '../util/logger';
 import {default as defaultFirefoxConnector, REMOTE_PORT} from './remote';
 
@@ -219,8 +220,15 @@ export function installExtension(
       return fs.mkdir(profile.extensionsDir);
     }))
     .then(() => {
-      const id = manifestData.applications.gecko.id;
-
+      const id = getManifestId(manifestData);
+      if (!id) {
+        throw new WebExtError(
+          'An explicit extension ID is required when installing to ' +
+          'a profile (applications.gecko.id not found in manifest.json)');
+      }
+      return id;
+    })
+    .then((id) => {
       if (asProxy) {
         log.debug(`Installing as an extension proxy; source: ${extensionPath}`);
         return isDirectory(extensionPath)
@@ -242,6 +250,7 @@ export function installExtension(
             return streamToPromise(writeStream);
           });
       } else {
+        // Write the XPI file to the profile.
         const readStream = nodeFs.createReadStream(extensionPath);
         const destPath = path.join(profile.extensionsDir, `${id}.xpi`);
         const writeStream = nodeFs.createWriteStream(destPath);

src/firefox/remote.js

@@ -63,7 +63,7 @@ export class RemoteFirefox {
     });
   }
 
-  installTemporaryAddon(addonPath: string) {
+  installTemporaryAddon(addonPath: string): Promise {
     return new Promise((resolve, reject) => {
       this.client.request('listTabs', (error, response) => {
         if (error) {
@@ -88,7 +88,7 @@ export class RemoteFirefox {
             }
             log.debug(`installTemporaryAddon: ${JSON.stringify(response)}`);
             log.info(`Installed ${addonPath} as a temporary add-on`);
-            resolve();
+            resolve(response);
           });
       });
     });

src/util/manifest.js

@@ -33,18 +33,11 @@ export default function getValidatedManifest(sourceDir: string): Promise {
         errors.push('missing "version" property');
       }
 
-      // Make sure the manifest defines a gecko id.
-      let idProps = ['applications', 'gecko', 'id'];
-      var propPath = '';
-      var objectToCheck = manifestData;
-
-      for (let nextProp of idProps) {
-        propPath = propPath ? `${propPath}.${nextProp}` : nextProp;
-        objectToCheck = objectToCheck[nextProp];
-        if (!objectToCheck) {
-          errors.push(`missing "${propPath}" property`);
-          break;
-        }
+      if (manifestData.applications && !manifestData.applications.gecko) {
+        // Since the applications property only applies to gecko, make
+        // sure 'gecko' exists when 'applications' is defined. This should
+        // make introspection of gecko properties easier.
+        errors.push('missing "applications.gecko" property');
       }
 
       if (errors.length) {
@@ -54,3 +47,9 @@ export default function getValidatedManifest(sourceDir: string): Promise {
       return manifestData;
     });
 }
+
+
+export function getManifestId(manifestData: Object): string|void {
+  return manifestData.applications ?
+    manifestData.applications.gecko.id : undefined;
+}

tests/test-cmd/test.build.js

@@ -8,7 +8,7 @@ import sinon from 'sinon';
 import build, {safeFileName, FileFilter} from '../../src/cmd/build';
 import {withTempDir} from '../../src/util/temp-dir';
 import {fixturePath, makeSureItFails, ZipFile} from '../helpers';
-import {basicManifest} from '../test-util/test.manifest';
+import {basicManifest, manifestWithoutApps} from '../test-util/test.manifest';
 
 
 describe('build', () => {
@@ -37,6 +37,19 @@ describe('build', () => {
     );
   });
 
+  it('can build an extension without an ID', () => {
+    return withTempDir(
+      (tmpDir) => {
+        // Make sure a manifest without an ID doesn't throw an error.
+        return build({
+          sourceDir: fixturePath('minimal-web-ext'),
+          manifestData: manifestWithoutApps,
+          artifactsDir: tmpDir.path(),
+        });
+      }
+    );
+  });
+
   it('prepares the artifacts dir', () => withTempDir(
     (tmpDir) => {
       const artifactsDir = path.join(tmpDir.path(), 'artifacts');
@@ -209,7 +222,7 @@ describe('build', () => {
       assert.equal(filter.wantFile('some.xpi'), true);
       assert.equal(filter.wantFile('manifest.json'), false);
     });
-    
+
     it('ignores node_modules by default', () => {
       assert.equal(defaultFilter.wantFile('path/to/node_modules'), false);
     });