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

CLI: Fix add command for non monorepo deps #25791

Merged
merged 4 commits into from
Jan 31, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yannbf This function was indeed just used by the add command. I looked it up wrongly previously.

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) {
valentinpalkovic marked this conversation as resolved.
Show resolved Hide resolved
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);
Loading