Skip to content

Commit

Permalink
Merge pull request #22401 from storybookjs/feat/turn-package-manager-…
Browse files Browse the repository at this point in the history
…to-async

CLI: Refactor package manager methods to be async
  • Loading branch information
yannbf authored and shilman committed May 9, 2023
1 parent 103b4e3 commit b0ba133
Show file tree
Hide file tree
Showing 46 changed files with 752 additions and 511 deletions.
10 changes: 10 additions & 0 deletions code/frameworks/angular/src/builders/utils/run-compodoc.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,16 @@ jest.doMock('child_process', () => cpSpawnMock);

const { runCompodoc } = require('./run-compodoc');

const mockRunScript = jest.fn();

jest.mock('@storybook/cli', () => ({
JsPackageManagerFactory: {
getPackageManager: () => ({
runPackageCommandSync: mockRunScript,
}),
},
}));

const builderContextLoggerMock: LoggerApi = {
createChild: jest.fn(),
log: jest.fn(),
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(
'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
Loading

0 comments on commit b0ba133

Please sign in to comment.