Skip to content

Commit

Permalink
Merge pull request #25791 from storybookjs/valentin/fix-add-command-f…
Browse files Browse the repository at this point in the history
…or-non-monorepo-deps

CLI: Fix add command for non monorepo deps
  • Loading branch information
valentinpalkovic authored Jan 31, 2024
2 parents de6d3f5 + 5c368f2 commit b10ece6
Show file tree
Hide file tree
Showing 10 changed files with 22 additions and 67 deletions.
5 changes: 3 additions & 2 deletions code/lib/cli/src/add.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {
getStorybookInfo,
serverRequire,
getStorybookVersion,
getCoercedStorybookVersion,
isCorePackage,
JsPackageManagerFactory,
type PackageManagerName,
Expand Down Expand Up @@ -107,8 +107,9 @@ export async function add(
// add to package.json
const isStorybookAddon = addonName.startsWith('@storybook/');
const isAddonFromCore = isCorePackage(addonName);
const storybookVersion = await getStorybookVersion(packageManager);
const storybookVersion = await getCoercedStorybookVersion(packageManager);
const version = versionSpecifier || (isAddonFromCore ? storybookVersion : latestVersion);

const addonWithVersion = SemVer.valid(version)
? `${addonName}@^${version}`
: `${addonName}@${version}`;
Expand Down
4 changes: 2 additions & 2 deletions code/lib/cli/src/automigrate/helpers/mainConfigFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import chalk from 'chalk';
import dedent from 'ts-dedent';
import path from 'path';
import type { JsPackageManager } from '@storybook/core-common';
import { getStorybookVersion } from '@storybook/core-common';
import { getCoercedStorybookVersion } from '@storybook/core-common';

const logger = console;

Expand Down Expand Up @@ -93,7 +93,7 @@ export const getStorybookData = async ({
configDir: configDirFromScript,
previewConfig: previewConfigPath,
} = getStorybookInfo(packageJson, userDefinedConfigDir);
const storybookVersion = await getStorybookVersion(packageManager);
const storybookVersion = await getCoercedStorybookVersion(packageManager);

const configDir = userDefinedConfigDir || configDirFromScript || '.storybook';

Expand Down
4 changes: 2 additions & 2 deletions code/lib/cli/src/automigrate/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import invariant from 'tiny-invariant';
import {
getStorybookInfo,
loadMainConfig,
getStorybookVersion,
getCoercedStorybookVersion,
JsPackageManagerFactory,
} from '@storybook/core-common';
import type { PackageManagerName } from '@storybook/core-common';
Expand Down Expand Up @@ -156,7 +156,7 @@ export async function runFixes({
userSpecifiedConfigDir
);

const storybookVersion = await getStorybookVersion(packageManager);
const storybookVersion = await getCoercedStorybookVersion(packageManager);

if (!storybookVersion) {
logger.info(dedent`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,10 @@ export abstract class JsPackageManager {
basePath?: string
): Promise<PackageJson | null>;

public abstract getPackageVersion(packageName: string, basePath?: string): Promise<string | null>;
async getPackageVersion(packageName: string, basePath = this.cwd): Promise<string | null> {
const packageJSON = await this.getPackageJSON(packageName, basePath);
return packageJSON ? packageJSON.version ?? null : null;
}

// NOTE: for some reason yarn prefers the npm registry in
// local development, so always use npm
Expand Down
6 changes: 0 additions & 6 deletions code/lib/core-common/src/js-package-manager/NPMProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import dedent from 'ts-dedent';
import { sync as findUpSync } from 'find-up';
import { existsSync, readFileSync } from 'fs';
import path from 'path';
import semver from 'semver';
import { logger } from '@storybook/node-logger';
import { JsPackageManager } from './JsPackageManager';
import type { PackageJson } from './PackageJson';
Expand Down Expand Up @@ -102,11 +101,6 @@ export class NPMProxy extends JsPackageManager {
return packageJson;
}

public async getPackageVersion(packageName: string, basePath = this.cwd): Promise<string | null> {
const packageJson = await this.getPackageJSON(packageName, basePath);
return packageJson ? semver.coerce(packageJson.version)?.version ?? null : null;
}

getInstallArgs(): string[] {
if (!this.installArgs) {
this.installArgs = [];
Expand Down
7 changes: 0 additions & 7 deletions code/lib/core-common/src/js-package-manager/PNPMProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import dedent from 'ts-dedent';
import { sync as findUpSync } from 'find-up';
import path from 'path';
import fs from 'fs';
import semver from 'semver';
import { JsPackageManager } from './JsPackageManager';
import type { PackageJson } from './PackageJson';
import type { InstallationMetadata, PackageMetadata } from './types';
Expand Down Expand Up @@ -159,12 +158,6 @@ export class PNPMProxy extends JsPackageManager {
return JSON.parse(fs.readFileSync(packageJsonPath, 'utf-8'));
}

async getPackageVersion(packageName: string, basePath = this.cwd): Promise<string | null> {
const packageJSON = await this.getPackageJSON(packageName, basePath);

return packageJSON ? semver.coerce(packageJSON.version)?.version ?? null : null;
}

protected getResolutions(packageJson: PackageJson, versions: Record<string, string>) {
return {
overrides: {
Expand Down
6 changes: 0 additions & 6 deletions code/lib/core-common/src/js-package-manager/Yarn1Proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import dedent from 'ts-dedent';
import { sync as findUpSync } from 'find-up';
import { existsSync, readFileSync } from 'fs';
import path from 'path';
import semver from 'semver';
import { createLogStream } from '../utils/cli';
import { JsPackageManager } from './JsPackageManager';
import type { PackageJson } from './PackageJson';
Expand Down Expand Up @@ -82,11 +81,6 @@ export class Yarn1Proxy extends JsPackageManager {
return JSON.parse(readFileSync(packageJsonPath, 'utf-8')) as Record<string, any>;
}

public async getPackageVersion(packageName: string, basePath = this.cwd): Promise<string | null> {
const packageJson = await this.getPackageJSON(packageName, basePath);
return packageJson ? semver.coerce(packageJson.version)?.version ?? null : null;
}

public async findInstallations(pattern: string[]) {
const commandResult = await this.executeCommand({
command: 'yarn',
Expand Down
6 changes: 0 additions & 6 deletions code/lib/core-common/src/js-package-manager/Yarn2Proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { existsSync, readFileSync } from 'fs';
import path from 'path';
import { PosixFS, VirtualFS, ZipOpenFS } from '@yarnpkg/fslib';
import { getLibzipSync } from '@yarnpkg/libzip';
import semver from 'semver';
import { createLogStream } from '../utils/cli';
import { JsPackageManager } from './JsPackageManager';
import type { PackageJson } from './PackageJson';
Expand Down Expand Up @@ -174,11 +173,6 @@ export class Yarn2Proxy extends JsPackageManager {
return packageJson;
}

async getPackageVersion(packageName: string, basePath = this.cwd): Promise<string | null> {
const packageJSON = await this.getPackageJSON(packageName, basePath);
return packageJSON ? semver.coerce(packageJSON.version)?.version ?? null : null;
}

protected getResolutions(packageJson: PackageJson, versions: Record<string, string>) {
return {
resolutions: {
Expand Down
5 changes: 2 additions & 3 deletions code/lib/core-common/src/utils/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,12 @@ describe('UTILS', () => {
describe.each([
['@storybook/react', true],
['@storybook/node-logger', true],
['@storybook/addon-info', true],
['@storybook/something-random', true],
['@storybook/preset-create-react-app', false],
['@storybook/linter-config', false],
['@storybook/design-system', false],
['@storybook/addon-styling', false],
['@storybook/addon-styling-webpack', false],
['@storybook/addon-webpack5-compiler-swc', false],
['@storybook/addon-webpack5-compiler-babel', false],
['@nx/storybook', false],
['@nrwl/storybook', false],
])('isCorePackage', (input, output) => {
Expand Down
41 changes: 9 additions & 32 deletions code/lib/core-common/src/utils/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { join } from 'path';
import tempy from 'tempy';
import { rendererPackages } from './get-storybook-info';
import type { JsPackageManager } from '../js-package-manager';
import versions from '../versions';

export function parseList(str: string): string[] {
return str
Expand All @@ -12,7 +13,13 @@ export function parseList(str: string): string[] {
.filter((item) => item.length > 0);
}

export async function getStorybookVersion(packageManager: JsPackageManager) {
/**
* Given a package manager, returns the coerced version of Storybook.
* It tries to find renderer packages in the project and returns the coerced version of the first one found.
* Example:
* If @storybook/react version 8.0.0-alpha.14 is installed, it returns the coerced version 8.0.0
*/
export async function getCoercedStorybookVersion(packageManager: JsPackageManager) {
const packages = (
await Promise.all(
Object.keys(rendererPackages).map(async (pkg) => ({
Expand Down Expand Up @@ -97,34 +104,4 @@ export const createLogStream = async (
});
};

const PACKAGES_EXCLUDED_FROM_CORE = [
'@storybook/addon-bench',
'@storybook/addon-console',
'@storybook/addon-onboarding',
'@storybook/addon-postcss',
'@storybook/addon-designs',
'@storybook/addon-styling',
'@storybook/addon-styling-webpack',
'@storybook/bench',
'@storybook/builder-vite',
'@storybook/csf',
'@storybook/design-system',
'@storybook/ember-cli-storybook',
'@storybook/eslint-config-storybook',
'@storybook/expect',
'@storybook/jest',
'@storybook/linter-config',
'@storybook/mdx1-csf',
'@storybook/mdx2-csf',
'@storybook/react-docgen-typescript-plugin',
'@storybook/storybook-deployer',
'@storybook/test-runner',
'@storybook/testing-library',
'@storybook/testing-react',
'@nrwl/storybook',
'@nx/storybook',
];
export const isCorePackage = (pkg: string) =>
pkg.startsWith('@storybook/') &&
!pkg.startsWith('@storybook/preset-') &&
!PACKAGES_EXCLUDED_FROM_CORE.includes(pkg);
export const isCorePackage = (pkg: string) => Object.keys(versions).includes(pkg);

0 comments on commit b10ece6

Please sign in to comment.