Merge pull request #344 from chromaui/fix-determine-viewlayer

Fix determining viewLayer when using transitive dependency

bin/lib/getStorybookInfo.js

@@ -1,11 +1,8 @@
-// Figure out the Storybook version and view layer
-
 import fs from 'fs-extra';
 import meow from 'meow';
 import argv from 'string-argv';
 import semver from 'semver';
 
-import noViewLayerDependency from '../ui/messages/errors/noViewLayerDependency';
 import noViewLayerPackage from '../ui/messages/errors/noViewLayerPackage';
 
 const viewLayers = {
@@ -23,7 +20,6 @@ const viewLayers = {
   '@storybook/svelte': 'svelte',
   '@storybook/preact': 'preact',
   '@storybook/rax': 'rax',
-  '@web/dev-server-storybook': 'web-components',
 };
 
 const supportedAddons = {
@@ -55,18 +51,24 @@ const supportedAddons = {
   '@storybook/addon-viewport': 'viewport',
 };
 
-const resolve = (pkg) => {
+const resolvePackageJson = (pkg) => {
   try {
     const path = require.resolve(`${pkg}/package.json`, { paths: [process.cwd()] });
-    return Promise.resolve(path);
+    return fs.readJson(path);
   } catch (error) {
     return Promise.reject(error);
   }
 };
 
+// Double inversion on Promise.all means fulfilling with the first fulfilled promise, or rejecting
+// when _everything_ rejects. This is different from Promise.race, which immediately rejects on the
+// first rejection.
+const invert = (promise) => new Promise((resolve, reject) => promise.then(reject, resolve));
+const raceFulfilled = (promises) => invert(Promise.all(promises.map(invert)).then((arr) => arr[0]));
+
 const timeout = (count) =>
   new Promise((_, rej) => {
-    setTimeout(() => rej(new Error('The attempt to find the Storybook version timed out')), count);
+    setTimeout(() => rej(new Error('Timeout while resolving Storybook view layer package')), count);
   });
 
 const findDependency = ({ dependencies, devDependencies, peerDependencies }, predicate) => [
@@ -94,11 +96,9 @@ const findViewlayer = async ({ env, log, options, packageJson }) => {
 
   // Pull the viewlayer from dependencies in package.json
   const [dep, devDep, peerDep] = findDependency(packageJson, ([key]) => viewLayers[key]);
-  const dependency = dep || devDep || peerDep;
+  const [pkg, version] = dep || devDep || peerDep || [];
+  const viewLayer = viewLayers[pkg];
 
-  if (!dependency) {
-    throw new Error(noViewLayerDependency());
-  }
   if (dep && devDep && dep[0] === devDep[0]) {
     log.warn(
       `Found "${dep[0]}" in both "dependencies" and "devDependencies". This is probably a mistake.`
@@ -110,19 +110,34 @@ const findViewlayer = async ({ env, log, options, packageJson }) => {
     );
   }
 
-  const [pkg, version] = dependency;
-  const viewLayer = pkg.replace('@storybook/', '');
+  if (viewLayer) {
+    if (options.storybookBuildDir) {
+      // If we aren't going to invoke the Storybook CLI later, we can exit early.
+      // Note that `version` can be a semver range in this case.
+      return { viewLayer, version };
+    }
+
+    // Verify that the viewlayer package is actually present in node_modules.
+    return Promise.race([
+      resolvePackageJson(pkg)
+        .then((json) => ({ viewLayer, version: json.version }))
+        .catch(() => Promise.reject(new Error(noViewLayerPackage(pkg)))),
+      timeout(10000),
+    ]);
+  }
 
-  // If we won't need the Storybook CLI, we can exit early
-  // Note that `version` can be a semver range in this case.
-  if (options.storybookBuildDir) return { viewLayer, version };
+  log.debug(`No viewlayer package listed in dependencies. Checking transitive dependencies.`);
 
-  // Try to find the viewlayer package in node_modules so we know it's installed
+  // We might have a transitive dependency (e.g. through `@nuxtjs/storybook` which depends on
+  // `@storybook/vue`). In this case we look for any viewlayer package present in node_modules,
+  // and return the first one we find.
   return Promise.race([
-    resolve(pkg)
-      .then(fs.readJson)
-      .then((json) => ({ viewLayer, version: json.version }))
-      .catch(() => Promise.reject(new Error(noViewLayerPackage(pkg)))),
+    raceFulfilled(
+      Object.entries(viewLayers).map(async ([key, value]) => {
+        const json = await resolvePackageJson(key);
+        return { viewLayer: value, version: json.version };
+      })
+    ).catch(() => Promise.reject(new Error(noViewLayerPackage(pkg)))),
     timeout(10000),
   ]);
 };

bin/lib/getStorybookInfo.test.js

@@ -1,6 +1,6 @@
 import getStorybookInfo from './getStorybookInfo';
 
-const log = { warn: jest.fn() };
+const log = { warn: jest.fn(), debug: jest.fn() };
 const context = { env: {}, log, options: {}, packageJson: {} };
 
 const REACT = { '@storybook/react': '1.2.3' };
@@ -15,10 +15,6 @@ describe('getStorybookInfo', () => {
     );
   });
 
-  it('throws on missing dependency', async () => {
-    await expect(getStorybookInfo(context)).rejects.toThrow('Storybook dependency not found');
-  });
-
   it('warns on duplicate devDependency', async () => {
     const ctx = { ...context, packageJson: { dependencies: REACT, devDependencies: REACT } };
     await getStorybookInfo(ctx);
@@ -40,6 +36,16 @@ describe('getStorybookInfo', () => {
     await expect(getStorybookInfo(ctx)).rejects.toThrow('Storybook package not installed');
   });
 
+  it('looks up package in node_modules on missing dependency', async () => {
+    await expect(getStorybookInfo(context)).resolves.toEqual(
+      // We're getting the result of tracing chromatic-cli's node_modules here.
+      expect.objectContaining({ viewLayer: 'react', version: expect.any(String) })
+    );
+    expect(log.debug).toHaveBeenCalledWith(
+      expect.stringContaining('No viewlayer package listed in dependencies')
+    );
+  });
+
   describe('with CHROMATIC_STORYBOOK_VERSION', () => {
     it('returns viewLayer and version from env', async () => {
       const ctx = { ...context, env: { CHROMATIC_STORYBOOK_VERSION: '@storybook/react@3.2.1' } };