Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix determining viewLayer when using transitive dependency #344

Merged
merged 4 commits into from May 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
57 changes: 36 additions & 21 deletions 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 = {
Expand All @@ -23,7 +20,6 @@ const viewLayers = {
'@storybook/svelte': 'svelte',
'@storybook/preact': 'preact',
'@storybook/rax': 'rax',
'@web/dev-server-storybook': 'web-components',
};

const supportedAddons = {
Expand Down Expand Up @@ -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]));
ghengeveld marked this conversation as resolved.
Show resolved Hide resolved

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) => [
Expand Down Expand Up @@ -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.`
Expand All @@ -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),
]);
};
Expand Down
16 changes: 11 additions & 5 deletions 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' };
Expand All @@ -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);
Expand All @@ -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' } };
Expand Down
13 changes: 0 additions & 13 deletions bin/ui/messages/errors/noViewLayerDependency.js

This file was deleted.

7 changes: 0 additions & 7 deletions bin/ui/messages/errors/noViewLayerDependency.stories.js

This file was deleted.