Skip to content
Merged
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
28 changes: 19 additions & 9 deletions lib/cli/src/js-package-manager/JsPackageManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,18 @@ import storybookPackagesVersions from '../versions.json';

const logger = console;

function getPackageDetails(pkg: string): [string, string?] {
const idx = pkg.lastIndexOf('@');
// If the only `@` is the first character, it is a scoped package
// If it isn't in the string, it will be -1
if (idx <= 0) {
return [pkg, undefined];
}
const packageName = pkg.slice(0, idx);
const packageVersion = pkg.slice(idx + 1);
return [packageName, packageVersion];
}

export abstract class JsPackageManager {
public abstract readonly type: 'npm' | 'yarn1' | 'yarn2';

Expand Down Expand Up @@ -81,10 +93,7 @@ export abstract class JsPackageManager {
const { packageJson } = options;

const dependenciesMap = dependencies.reduce((acc, dep) => {
const idx = dep.lastIndexOf('@');
const packageName = dep.slice(0, idx);
const packageVersion = dep.slice(idx + 1);

const [packageName, packageVersion] = getPackageDetails(dep);
return { ...acc, [packageName]: packageVersion };
}, {});

Expand Down Expand Up @@ -115,13 +124,14 @@ export abstract class JsPackageManager {
/**
* Return an array of strings matching following format: `<package_name>@<package_latest_version>`
*
* @param packageNames
* @param packages
*/
public getVersionedPackages(...packageNames: string[]): Promise<string[]> {
public getVersionedPackages(...packages: string[]): Promise<string[]> {
return Promise.all(
packageNames.map(
async (packageName) => `${packageName}@${await this.getVersion(packageName)}`
)
packages.map(async (pkg) => {
const [packageName, packageVersion] = getPackageDetails(pkg);
return `${packageName}@${await this.getVersion(packageName, packageVersion)}`;
Copy link
Member

Choose a reason for hiding this comment

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

AFAICT this will fail when packageVersion is a literal version. getVersion calls satisfies and if it fails it uses the latest version. Consider the case where latest is 1.3.5 and we need version 1.2.0.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this only fails if the version doesn't exist. I tested semver.satisfies('1.0.0', '1.0.0') and it returns true so as long as the literal version exists inside versions.reverse(), it should find one.

That being said, the latestVersion method could probably be hardened against a constraint not being satisfied by all versions on npm. Do you want me to do that as part of this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(current is only ever set for @storybook packages, so the versionToUse ternary only ever gets applied to those)

Copy link
Member

Choose a reason for hiding this comment

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

Aha my bad

})
);
}

Expand Down