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 "Invalid version null" issues by improved version detection #22642

Merged
merged 30 commits into from
Jun 15, 2023
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
4154066
Add support for "@yarnpkg/fslib" and "@yarnpkg/libzip", and implement…
valentinpalkovic May 16, 2023
9c57628
Remove Angular 12 automigration
valentinpalkovic May 16, 2023
ed7813d
Read proper installed package version
valentinpalkovic May 19, 2023
6c06430
Fix lint
valentinpalkovic May 19, 2023
f3bc515
Remove unused import
valentinpalkovic May 19, 2023
ff9caf8
Take backslash paths into account
valentinpalkovic May 22, 2023
4fc7e9d
Await project type detection
valentinpalkovic May 22, 2023
32cce05
Await language detection
valentinpalkovic May 23, 2023
79c9724
Pass packageManager instead of packageJSON to detectLanguage
valentinpalkovic May 23, 2023
82d4e7a
Fix pnp/workspace related issues and read exact installed versions fr…
valentinpalkovic May 31, 2023
c226dcd
Get proper installed Storybook version
valentinpalkovic May 31, 2023
82f819e
Fix eslint plugin setup
valentinpalkovic May 31, 2023
5e2a357
Fix isNxProject function by returning boolean instead of number
valentinpalkovic May 31, 2023
f07c4c0
Fix error log
valentinpalkovic Jun 1, 2023
6aca79c
Fix test
valentinpalkovic Jun 1, 2023
a67383a
Fix return value of detectPnp
valentinpalkovic Jun 1, 2023
ec27b9b
Fix types
valentinpalkovic Jun 2, 2023
cf68d65
Remove comment
valentinpalkovic Jun 2, 2023
8f557f7
Add todo comment
valentinpalkovic Jun 2, 2023
e18f0dd
Merge branch 'next' into valentin/invalid-version-null
valentinpalkovic Jun 6, 2023
3789a2e
Merge branch 'next' into valentin/invalid-version-null
yannbf Jun 9, 2023
e724228
Merge branch 'next' into valentin/invalid-version-null
yannbf Jun 15, 2023
0a88f95
await removeDependencies
yannbf Jun 15, 2023
f786b0f
fix async logic in removeDependencies
yannbf Jun 15, 2023
bd00ccd
Merge branch 'next' into valentin/invalid-version-null
yannbf Jun 15, 2023
a884dcd
account for correct cwd in package proxies
yannbf Jun 15, 2023
33f8cc2
pass sandbox directory when adding stories
yannbf Jun 15, 2023
7d91a00
Merge branch 'next' into valentin/invalid-version-null
yannbf Jun 15, 2023
2c3be33
prebundle boxen
yannbf Jun 15, 2023
f00b9e0
Merge branch 'next' into valentin/invalid-version-null
yannbf Jun 15, 2023
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
10 changes: 9 additions & 1 deletion code/frameworks/nextjs/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,15 @@ export const addScopedAlias = (baseConfig: WebpackConfig, name: string, alias?:
* scopedResolve('styled-jsx') === '/some/path/node_modules/styled-jsx'
*/
export const scopedResolve = (id: string): string => {
const scopedModulePath = require.resolve(id, { paths: [path.resolve()] });
let scopedModulePath;

try {
// TODO: Remove in next major release (SB 8.0) and use the statement in the catch block per default instead
scopedModulePath = require.resolve(id, { paths: [path.resolve()] });
} catch (e) {
scopedModulePath = require.resolve(id);
}

const moduleFolderStrPosition = scopedModulePath.lastIndexOf(
id.replace(/\//g /* all '/' occurances */, path.sep)
);
Expand Down
2 changes: 2 additions & 0 deletions code/lib/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@
"@storybook/telemetry": "7.1.0-alpha.34",
"@storybook/types": "7.1.0-alpha.34",
"@types/semver": "^7.3.4",
"@yarnpkg/fslib": "^2.10.3",
"@yarnpkg/libzip": "^2.3.0",
"boxen": "^5.1.2",
"chalk": "^4.1.0",
"commander": "^6.2.1",
Expand Down
6 changes: 4 additions & 2 deletions code/lib/cli/src/add.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
useNpmWarning,
type PackageManagerName,
} from './js-package-manager';
import { getStorybookVersion } from './utils';

const logger = console;

Expand Down Expand Up @@ -83,7 +84,7 @@ export async function add(
const packageJson = await packageManager.retrievePackageJson();
const [addonName, versionSpecifier] = getVersionSpecifier(addon);

const { mainConfig, version: storybookVersion } = getStorybookInfo(packageJson);
const { mainConfig } = getStorybookInfo(packageJson);
if (!mainConfig) {
logger.error('Unable to find storybook main.js config');
return;
Expand All @@ -97,8 +98,9 @@ export async function add(

// add to package.json
const isStorybookAddon = addonName.startsWith('@storybook/');
const storybookVersion = await getStorybookVersion(packageManager);
const version = versionSpecifier || (isStorybookAddon ? storybookVersion : latestVersion);
const addonWithVersion = `${addonName}@${version}`;
const addonWithVersion = `${addonName}@^${version}`;
logger.log(`Installing ${addonWithVersion}`);
await packageManager.addDependencies({ installAsDevDependencies: true }, [addonWithVersion]);

Expand Down
8 changes: 7 additions & 1 deletion code/lib/cli/src/automigrate/fixes/add-react.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
import type { StorybookConfig } from '@storybook/types';
import type { JsPackageManager, PackageJson } from '../../js-package-manager';
import { addReact } from './add-react';

const checkAddReact = async (packageJson: PackageJson) => {
const packageManager = {
retrievePackageJson: async () => ({ dependencies: {}, devDependencies: {}, ...packageJson }),
} as JsPackageManager;
return addReact.check({ packageManager });

return addReact.check({
packageManager,
mainConfig: {} as StorybookConfig,
storybookVersion: '7.0.0',
});
};

describe('addReact fix', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,25 +1,20 @@
import type { StorybookConfig } from '@storybook/types';
import type { PackageJson } from '../../js-package-manager';
import { makePackageManager, mockStorybookData } from '../helpers/testing-helpers';
import type { JsPackageManager } from '../../js-package-manager';
import { angularBuildersMultiproject } from './angular-builders-multiproject';
import * as helpers from '../../helpers';
import * as angularHelpers from '../../generators/ANGULAR/helpers';

const checkAngularBuilders = async ({
packageJson,
main: mainConfig = {},
storybookVersion = '7.0.0',
packageManager,
mainConfig = {},
}: {
packageJson: PackageJson;
main?: Partial<StorybookConfig> & Record<string, unknown>;
storybookVersion?: string;
packageManager: Partial<JsPackageManager>;
mainConfig?: Partial<StorybookConfig>;
}) => {
mockStorybookData({ mainConfig, storybookVersion });

// mock file system (look at eslint plugin test)

return angularBuildersMultiproject.check({
packageManager: makePackageManager(packageJson),
packageManager: packageManager as any,
mainConfig: mainConfig as any,
storybookVersion: '7.0.0',
});
};

Expand All @@ -34,51 +29,69 @@ jest.mock('../../generators/ANGULAR/helpers', () => ({
}));

describe('is Nx project', () => {
const packageManager = {
getPackageVersion: () => {
return null;
},
} as Partial<JsPackageManager>;

beforeEach(() => {
(helpers.isNxProject as any as jest.SpyInstance).mockReturnValue(true);
(helpers.isNxProject as any as jest.SpyInstance).mockResolvedValue(true);
});

it('should return null', async () => {
const packageJson = {
dependencies: { '@storybook/angular': '^7.0.0-alpha.0' },
};

await expect(checkAngularBuilders({ packageJson })).resolves.toBeNull();
await expect(checkAngularBuilders({ packageManager })).resolves.toBeNull();
});
});

describe('is not Nx project', () => {
beforeEach(() => {
(helpers.isNxProject as any as jest.SpyInstance).mockReturnValue(false);
(helpers.isNxProject as any as jest.SpyInstance).mockResolvedValue(false);
});

describe('angular builders', () => {
afterEach(jest.restoreAllMocks);

describe('Angular not found', () => {
const packageJson = {
dependencies: { '@storybook/angular': '^7.0.0-alpha.0' },
};
const packageManager = {
getPackageVersion: jest.fn().mockResolvedValue(null),
} as Partial<JsPackageManager>;

it('should return null', async () => {
await expect(checkAngularBuilders({ packageJson })).resolves.toBeNull();
await expect(
checkAngularBuilders({ packageManager, mainConfig: { framework: '@storybook/angular' } })
).resolves.toBeNull();
});
});

describe('Angular < 14.0.0', () => {
const packageJson = {
dependencies: { '@storybook/angular': '^7.0.0-alpha.0', '@angular/core': '^12.0.0' },
};
const packageManager = {
getPackageVersion: (packageName) => {
if (packageName === '@angular/core') {
return Promise.resolve('12.0.0');
}

return null;
},
} as Partial<JsPackageManager>;

it('should return null', async () => {
await expect(checkAngularBuilders({ packageJson })).resolves.toBeNull();
await expect(
checkAngularBuilders({ packageManager, mainConfig: { framework: '@storybook/angular' } })
).resolves.toBeNull();
});
});

describe('Angular >= 14.0.0', () => {
const packageJson = {
dependencies: { '@storybook/angular': '^7.0.0-alpha.0', '@angular/core': '^15.0.0' },
};
const packageManager = {
getPackageVersion: (packageName) => {
if (packageName === '@angular/core') {
return Promise.resolve('15.0.0');
}

return null;
},
} as Partial<JsPackageManager>;

describe('has one Storybook builder defined', () => {
beforeEach(() => {
Expand All @@ -89,7 +102,12 @@ describe('is not Nx project', () => {
});

it('should return null', async () => {
await expect(checkAngularBuilders({ packageJson })).resolves.toBeNull();
await expect(
checkAngularBuilders({
packageManager,
mainConfig: { framework: '@storybook/angular' },
})
).resolves.toBeNull();
});
});

Expand All @@ -106,7 +124,12 @@ describe('is not Nx project', () => {
});

it('should return null', async () => {
await expect(checkAngularBuilders({ packageJson })).resolves.toBeNull();
await expect(
checkAngularBuilders({
packageManager,
mainConfig: { framework: '@storybook/angular' },
})
).resolves.toBeNull();
});
});

Expand All @@ -124,7 +147,12 @@ describe('is not Nx project', () => {
});

it('should return an empty object', async () => {
await expect(checkAngularBuilders({ packageJson })).resolves.toMatchObject({});
await expect(
checkAngularBuilders({
packageManager,
mainConfig: { framework: '@storybook/angular' },
})
).resolves.toMatchObject({});
});
});
});
Expand Down
29 changes: 11 additions & 18 deletions code/lib/cli/src/automigrate/fixes/angular-builders-multiproject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import chalk from 'chalk';
import type { Fix } from '../types';
import { isNxProject } from '../../helpers';
import { AngularJSON } from '../../generators/ANGULAR/helpers';
import { getFrameworkPackageName } from '../helpers/mainConfigFile';

// eslint-disable-next-line @typescript-eslint/no-empty-interface
interface AngularBuildersMultiprojectRunOptions {}
Expand All @@ -12,25 +13,17 @@ export const angularBuildersMultiproject: Fix<AngularBuildersMultiprojectRunOpti
id: 'angular-builders-multiproject',
promptOnly: true,

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

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

const angularVersion = allDependencies['@angular/core'];
const angularCoerced = semver.coerce(angularVersion)?.version;

// skip non-angular projects
if (!angularCoerced) {
return null;
}

// Is Angular version lower than 14? -> throw an error (only supports ng 14)
if (semver.lt(angularCoerced, '14.0.0')) {
const angularVersion = await packageManager.getPackageVersion('@angular/core');
const frameworkPackageName = getFrameworkPackageName(mainConfig);

if (
(await isNxProject(packageManager)) ||
frameworkPackageName !== '@storybook/angular' ||
!angularVersion ||
semver.lt(angularVersion, '14.0.0')
) {
return null;
}

Expand Down
Loading