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: Refactor package manager methods to be async #22401

Merged
merged 2 commits into from
May 8, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const mockRunScript = jest.fn();
jest.mock('@storybook/cli', () => ({
JsPackageManagerFactory: {
getPackageManager: () => ({
runPackageCommand: mockRunScript,
runPackageCommandSync: mockRunScript,
}),
},
}));
Expand Down
2 changes: 1 addition & 1 deletion code/frameworks/angular/src/builders/utils/run-compodoc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export const runCompodoc = (
const packageManager = JsPackageManagerFactory.getPackageManager();

try {
const stdout = packageManager.runPackageCommand(
const stdout = packageManager.runPackageCommandSync(
valentinpalkovic marked this conversation as resolved.
Show resolved Hide resolved
'compodoc',
finalCompodocArgs,
context.workspaceRoot
Expand Down
6 changes: 3 additions & 3 deletions code/lib/cli/src/add.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export async function add(
pkgMgr = 'npm';
}
const packageManager = JsPackageManagerFactory.getPackageManager({ force: pkgMgr });
const packageJson = packageManager.retrievePackageJson();
const packageJson = await packageManager.retrievePackageJson();
const [addonName, versionSpecifier] = getVersionSpecifier(addon);

const { mainConfig, version: storybookVersion } = getStorybookInfo(packageJson);
Expand All @@ -90,7 +90,7 @@ export async function add(
}
const main = await readConfig(mainConfig);
logger.log(`Verifying ${addonName}`);
const latestVersion = packageManager.latestVersion(addonName);
const latestVersion = await packageManager.latestVersion(addonName);
if (!latestVersion) {
logger.error(`Unknown addon ${addonName}`);
}
Expand All @@ -100,7 +100,7 @@ export async function add(
const version = versionSpecifier || (isStorybookAddon ? storybookVersion : latestVersion);
const addonWithVersion = `${addonName}@${version}`;
logger.log(`Installing ${addonWithVersion}`);
packageManager.addDependencies({ installAsDevDependencies: true }, [addonWithVersion]);
await packageManager.addDependencies({ installAsDevDependencies: true }, [addonWithVersion]);

// add to main.js
logger.log(`Adding '${addon}' to main.js addons field.`);
Expand Down
2 changes: 1 addition & 1 deletion code/lib/cli/src/automigrate/fixes/add-react.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { addReact } from './add-react';

const checkAddReact = async (packageJson: PackageJson) => {
const packageManager = {
retrievePackageJson: () => ({ dependencies: {}, devDependencies: {}, ...packageJson }),
retrievePackageJson: async () => ({ dependencies: {}, devDependencies: {}, ...packageJson }),
} as JsPackageManager;
return addReact.check({ packageManager });
};
Expand Down
7 changes: 5 additions & 2 deletions code/lib/cli/src/automigrate/fixes/add-react.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export const addReact: Fix<AddReactOptions> = {
id: 'addReact',

async check({ packageManager }) {
const packageJson = packageManager.retrievePackageJson();
const packageJson = await packageManager.retrievePackageJson();
const installedDependencies = new Set(
Object.keys({ ...packageJson.dependencies, ...packageJson.devDependencies })
);
Expand Down Expand Up @@ -63,7 +63,10 @@ export const addReact: Fix<AddReactOptions> = {

async run({ packageManager, result: { additionalDependencies }, dryRun }) {
if (!dryRun) {
packageManager.addDependencies({ installAsDevDependencies: true }, additionalDependencies);
await packageManager.addDependencies(
{ installAsDevDependencies: true },
additionalDependencies
);
}
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@ export const angularBuildersMultiproject: Fix<AngularBuildersMultiprojectRunOpti
id: 'angular-builders-multiproject',
promptOnly: true,

async check({ packageManager, configDir }) {
const packageJSON = packageManager.retrievePackageJson();
async check({ packageManager }) {
const packageJSON = await packageManager.retrievePackageJson();

// Skip in case of NX
if (isNxProject(packageJSON)) {
return null;
}
const allDependencies = packageManager.getAllDependencies();
const allDependencies = await packageManager.getAllDependencies();

const angularVersion = allDependencies['@angular/core'];
const angularCoerced = semver.coerce(angularVersion)?.version;
Expand Down
6 changes: 3 additions & 3 deletions code/lib/cli/src/automigrate/fixes/angular-builders.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ export const angularBuilders: Fix<AngularBuildersRunOptions> = {
id: 'angular-builders',

async check({ packageManager, configDir }) {
const packageJSON = packageManager.retrievePackageJson();
const packageJSON = await packageManager.retrievePackageJson();

// Skip in case of NX
if (isNxProject(packageJSON)) {
return null;
}
const allDependencies = packageManager.getAllDependencies();
const allDependencies = await packageManager.getAllDependencies();

const angularVersion = allDependencies['@angular/core'];
const angularCoerced = semver.coerce(angularVersion)?.version;
Expand Down Expand Up @@ -98,7 +98,7 @@ export const angularBuilders: Fix<AngularBuildersRunOptions> = {

angularJSON.write();

packageManager.addScripts({
await packageManager.addScripts({
storybook: `ng run ${angularProjectName}:storybook`,
'build-storybook': `ng run ${angularProjectName}:build-storybook`,
});
Expand Down
2 changes: 1 addition & 1 deletion code/lib/cli/src/automigrate/fixes/angular12.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export const angular12: Fix<Angular12RunOptions> = {
id: 'angular12',

async check({ packageManager, configDir }) {
const allDependencies = packageManager.getAllDependencies();
const allDependencies = await packageManager.getAllDependencies();
const angularVersion = allDependencies['@angular/core'];
const angularCoerced = semver.coerce(angularVersion)?.version;

Expand Down
6 changes: 3 additions & 3 deletions code/lib/cli/src/automigrate/fixes/builder-vite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export const builderVite: Fix<BuilderViteOptions> = {
id: 'builder-vite',

async check({ configDir, packageManager }) {
const packageJson = packageManager.retrievePackageJson();
const packageJson = await packageManager.retrievePackageJson();
const { mainConfig } = await getStorybookData({ configDir, packageManager });
const builder = mainConfig.core?.builder;
const builderName = typeof builder === 'string' ? builder : builder?.name;
Expand Down Expand Up @@ -64,12 +64,12 @@ export const builderVite: Fix<BuilderViteOptions> = {
if (!dryRun) {
delete dependencies['storybook-builder-vite'];
delete devDependencies['storybook-builder-vite'];
packageManager.writePackageJson(packageJson);
await packageManager.writePackageJson(packageJson);
}

logger.info(`✅ Adding '@storybook/builder-vite' as dev dependency`);
if (!dryRun) {
packageManager.addDependencies({ installAsDevDependencies: true }, [
await packageManager.addDependencies({ installAsDevDependencies: true }, [
'@storybook/builder-vite',
]);
}
Expand Down
2 changes: 1 addition & 1 deletion code/lib/cli/src/automigrate/fixes/cra5.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export const cra5: Fix<CRA5RunOptions> = {
id: 'cra5',

async check({ packageManager, configDir }) {
const allDependencies = packageManager.getAllDependencies();
const allDependencies = await packageManager.getAllDependencies();
const craVersion = allDependencies['react-scripts'];
const craCoerced = semver.coerce(craVersion)?.version;

Expand Down
7 changes: 4 additions & 3 deletions code/lib/cli/src/automigrate/fixes/eslint-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export const eslintPlugin: Fix<EslintPluginRunOptions> = {
id: 'eslintPlugin',

async check({ packageManager }) {
const allDependencies = packageManager.getAllDependencies();
const allDependencies = await packageManager.getAllDependencies();

const eslintPluginStorybook = allDependencies['eslint-plugin-storybook'];
const eslintDependency = allDependencies.eslint;
Expand Down Expand Up @@ -64,8 +64,9 @@ export const eslintPlugin: Fix<EslintPluginRunOptions> = {
const deps = [`eslint-plugin-storybook`];

logger.info(`✅ Adding dependencies: ${deps}`);
if (!dryRun)
packageManager.addDependencies({ installAsDevDependencies: true, skipInstall }, deps);
if (!dryRun) {
await packageManager.addDependencies({ installAsDevDependencies: true, skipInstall }, deps);
}

if (!dryRun && unsupportedExtension) {
logger.info(dedent`
Expand Down
6 changes: 4 additions & 2 deletions code/lib/cli/src/automigrate/fixes/mdx-gfm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,10 @@ export const mdxgfm: Fix<Options> = {

async run({ packageManager, dryRun, mainConfigPath, skipInstall }) {
if (!dryRun) {
const packageJson = packageManager.retrievePackageJson();
const versionToInstall = getStorybookVersionSpecifier(packageManager.retrievePackageJson());
const packageJson = await packageManager.retrievePackageJson();
const versionToInstall = getStorybookVersionSpecifier(
await packageManager.retrievePackageJson()
);
await packageManager.addDependencies(
{ installAsDevDependencies: true, skipInstall, packageJson },
[`@storybook/addon-mdx-gfm@${versionToInstall}`]
Expand Down
2 changes: 1 addition & 1 deletion code/lib/cli/src/automigrate/fixes/missing-babelrc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export const missingBabelRc: Fix<MissingBabelRcOptions> = {
id: 'missing-babelrc',

async check({ configDir, packageManager }) {
const packageJson = packageManager.retrievePackageJson();
const packageJson = await packageManager.retrievePackageJson();
const { mainConfig, storybookVersion } = await getStorybookData({ configDir, packageManager });

if (!semver.gte(storybookVersion, '7.0.0')) {
Expand Down
8 changes: 4 additions & 4 deletions code/lib/cli/src/automigrate/fixes/new-frameworks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export const newFrameworks: Fix<NewFrameworkRunOptions> = {
configDir: userDefinedConfigDir,
packageManager,
}) {
const packageJson = packageManager.retrievePackageJson();
const packageJson = await packageManager.retrievePackageJson();
const { storybookVersion, mainConfig, mainConfigPath, configDir } = await getStorybookData({
packageManager,
configDir: userDefinedConfigDir,
Expand Down Expand Up @@ -108,7 +108,7 @@ export const newFrameworks: Fix<NewFrameworkRunOptions> = {
return null;
}

const allDependencies = packageManager.getAllDependencies();
const allDependencies = await packageManager.getAllDependencies();

const builderInfo = await detectBuilderInfo({
mainConfig,
Expand Down Expand Up @@ -448,7 +448,7 @@ export const newFrameworks: Fix<NewFrameworkRunOptions> = {
if (dependenciesToRemove.length > 0) {
logger.info(`✅ Removing dependencies: ${dependenciesToRemove.join(', ')}`);
if (!dryRun) {
packageManager.removeDependencies(
await packageManager.removeDependencies(
{ skipInstall: skipInstall || dependenciesToAdd.length > 0, packageJson },
dependenciesToRemove
);
Expand All @@ -460,7 +460,7 @@ export const newFrameworks: Fix<NewFrameworkRunOptions> = {
if (!dryRun) {
const versionToInstall = getStorybookVersionSpecifier(packageJson);
const depsToAdd = dependenciesToAdd.map((dep) => `${dep}@${versionToInstall}`);
packageManager.addDependencies(
await packageManager.addDependencies(
{ installAsDevDependencies: true, skipInstall, packageJson },
depsToAdd
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const check = async ({ packageJson = {}, contents }: any) => {
});
}
const packageManager = {
retrievePackageJson: () => ({ dependencies: {}, devDependencies: {}, ...packageJson }),
retrievePackageJson: async () => ({ dependencies: {}, devDependencies: {}, ...packageJson }),
} as JsPackageManager;
return migration.check({ packageManager });
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export const removedGlobalClientAPIs: Fix<GlobalClientAPIOptions> = {
promptOnly: true,

async check({ packageManager, configDir }) {
const packageJson = packageManager.retrievePackageJson();
const packageJson = await packageManager.retrievePackageJson();

const { previewConfig } = getStorybookInfo(packageJson, configDir);

Expand Down
8 changes: 4 additions & 4 deletions code/lib/cli/src/automigrate/fixes/sb-binary.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ export const sbBinary: Fix<SbBinaryRunOptions> = {
id: 'storybook-binary',

async check({ packageManager, configDir }) {
const packageJson = packageManager.retrievePackageJson();
const allDependencies = packageManager.getAllDependencies();
const packageJson = await packageManager.retrievePackageJson();
const allDependencies = await packageManager.getAllDependencies();
const { storybookVersion } = await getStorybookData({ packageManager, configDir });

// Nx provides their own binary, so we don't need to do anything
Expand Down Expand Up @@ -82,7 +82,7 @@ export const sbBinary: Fix<SbBinaryRunOptions> = {
if (hasSbBinary) {
logger.info(`✅ Removing 'sb' dependency`);
if (!dryRun) {
packageManager.removeDependencies(
await packageManager.removeDependencies(
{ skipInstall: skipInstall || !hasStorybookBinary, packageJson },
['sb']
);
Expand All @@ -95,7 +95,7 @@ export const sbBinary: Fix<SbBinaryRunOptions> = {
logger.log();
if (!dryRun) {
const versionToInstall = getStorybookVersionSpecifier(packageJson);
packageManager.addDependencies(
await packageManager.addDependencies(
{ installAsDevDependencies: true, packageJson, skipInstall },
[`storybook@${versionToInstall}`]
);
Expand Down
4 changes: 2 additions & 2 deletions code/lib/cli/src/automigrate/fixes/sb-scripts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export const sbScripts: Fix<SbScriptsRunOptions> = {
id: 'sb-scripts',

async check({ packageManager, configDir }) {
const packageJson = packageManager.retrievePackageJson();
const packageJson = await packageManager.retrievePackageJson();
const { scripts = {} } = packageJson;
const { storybookVersion } = await getStorybookData({ packageManager, configDir });

Expand Down Expand Up @@ -133,7 +133,7 @@ export const sbScripts: Fix<SbScriptsRunOptions> = {

logger.log();

packageManager.addScripts(newScripts);
await packageManager.addScripts(newScripts);
}
},
};
2 changes: 1 addition & 1 deletion code/lib/cli/src/automigrate/fixes/vue3.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export const vue3: Fix<Vue3RunOptions> = {
id: 'vue3',

async check({ configDir, packageManager }) {
const allDependencies = packageManager.getAllDependencies();
const allDependencies = await packageManager.getAllDependencies();
const vueVersion = allDependencies.vue;
const vueCoerced = semver.coerce(vueVersion)?.version;

Expand Down
6 changes: 4 additions & 2 deletions code/lib/cli/src/automigrate/fixes/webpack5.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export const webpack5: Fix<Webpack5RunOptions> = {
id: 'webpack5',

async check({ configDir, packageManager }) {
const allDependencies = packageManager.retrievePackageJson().dependencies;
const allDependencies = (await packageManager.retrievePackageJson()).dependencies;

const webpackVersion = allDependencies.webpack;
const webpackCoerced = semver.coerce(webpackVersion)?.version;
Expand Down Expand Up @@ -72,7 +72,9 @@ export const webpack5: Fix<Webpack5RunOptions> = {
deps.push('webpack@5');
}
logger.info(`✅ Adding dependencies: ${deps}`);
if (!dryRun) packageManager.addDependencies({ installAsDevDependencies: true }, deps);
if (!dryRun) {
await packageManager.addDependencies({ installAsDevDependencies: true }, deps);
}

logger.info('✅ Setting `core.builder` to `@storybook/builder-webpack5` in main.js');
if (!dryRun) {
Expand Down
2 changes: 1 addition & 1 deletion code/lib/cli/src/automigrate/helpers/mainConfigFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export const getStorybookData = async ({
packageManager: JsPackageManager;
configDir: string;
}) => {
const packageJson = packageManager.retrievePackageJson();
const packageJson = await packageManager.retrievePackageJson();
const {
mainConfig: mainConfigPath,
version: storybookVersionSpecifier,
Expand Down
4 changes: 2 additions & 2 deletions code/lib/cli/src/automigrate/helpers/testing-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ jest.mock('@storybook/core-common', () => ({
export const makePackageManager = (packageJson: PackageJson) => {
const { dependencies = {}, devDependencies = {}, peerDependencies = {} } = packageJson;
return {
retrievePackageJson: () => ({ dependencies: {}, devDependencies: {}, ...packageJson }),
getAllDependencies: () => ({
retrievePackageJson: async () => ({ dependencies: {}, devDependencies: {}, ...packageJson }),
getAllDependencies: async () => ({
...dependencies,
...devDependencies,
...peerDependencies,
Expand Down
2 changes: 1 addition & 1 deletion code/lib/cli/src/automigrate/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ export async function runFixes({
configDir: inferredConfigDir,
mainConfig: mainConfigPath,
version: storybookVersion,
} = getStorybookInfo(packageManager.retrievePackageJson(), userSpecifiedConfigDir);
} = getStorybookInfo(await packageManager.retrievePackageJson(), userSpecifiedConfigDir);

const sbVersionCoerced = storybookVersion && semver.coerce(storybookVersion)?.version;
if (!sbVersionCoerced) {
Expand Down
2 changes: 1 addition & 1 deletion code/lib/cli/src/babel-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ export const generateStorybookBabelConfig = async ({ target }: { target: string

const packageManager = JsPackageManagerFactory.getPackageManager();

packageManager.addDependencies({ installAsDevDependencies: true }, added);
await packageManager.addDependencies({ installAsDevDependencies: true }, added);
} else {
logger.info(
`⚠️ Please remember to install the required dependencies yourself: (${added.join(', ')})`
Expand Down
8 changes: 4 additions & 4 deletions code/lib/cli/src/detect-webpack.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
import type { JsPackageManager } from './js-package-manager';

export const detectWebpack = (packageManager: JsPackageManager): number | false => {
export const detectWebpack = async (packageManager: JsPackageManager): Promise<number | false> => {
try {
let out = '';
if (packageManager.type === 'npm') {
try {
// npm <= v7
out = packageManager.executeCommand('npm', ['ls', 'webpack']);
out = await packageManager.executeCommand({ command: 'npm', args: ['ls', 'webpack'] });
} catch (e2) {
// npm >= v8
out = packageManager.executeCommand('npm', ['why', 'webpack']);
out = await packageManager.executeCommand({ command: 'npm', args: ['why', 'webpack'] });
}
} else {
out = packageManager.executeCommand('yarn', ['why', 'webpack']);
out = await packageManager.executeCommand({ command: 'yarn', args: ['why', 'webpack'] });
}

// if the user has BOTH webpack 4 and 5 installed already, we'll pick the safest options (4)
Expand Down