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 sb init to use renderer assets instead of frameworks #19091

Merged
merged 1 commit into from
Sep 2, 2022
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
2 changes: 1 addition & 1 deletion code/lib/cli/.babelrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
]
],
"ignore": [
"./src/frameworks",
"./src/rendererAssets",
"./src/generators/**/template",
"./src/generators/**/template-csf",
"./src/generators/**/template-csf-ts",
Expand Down
2 changes: 1 addition & 1 deletion code/lib/cli/src/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ module.exports = {
},
},
{
files: 'frameworks/**/*',
files: 'rendererAssets/**/*',
env: {
browser: true,
},
Expand Down
3 changes: 1 addition & 2 deletions code/lib/cli/src/automigrate/fixes/eslint-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,8 @@ export const eslintPlugin: Fix<EslintPluginRunOptions> = {
return null;
}

const config = getStorybookInfo(packageJson);
const { mainConfig } = getStorybookInfo(packageJson);

const { mainConfig } = config;
if (!mainConfig) {
logger.warn('Unable to find storybook main.js config, skipping');
return null;
Expand Down
2 changes: 2 additions & 0 deletions code/lib/cli/src/automigrate/fixes/mainjsFramework.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ export const mainjsFramework: Fix<MainjsFrameworkRunOptions> = {

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

// FIXME: use renderer in SB7?
const { mainConfig, framework, version: storybookVersion } = getStorybookInfo(packageJson);

if (!mainConfig) {
Expand Down
4 changes: 2 additions & 2 deletions code/lib/cli/src/automigrate/fixes/new-frameworks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@ export const newFrameworks: Fix<NewFrameworkRunOptions> = {
const packageJson = packageManager.retrievePackageJson();
const allDeps = { ...packageJson.dependencies, ...packageJson.devDependencies };

const config = getStorybookInfo(packageJson);
const { mainConfig, version: storybookVersion, framework } = config;
// FIXME: update to use renderer instead of framework
const { mainConfig, version: storybookVersion, framework } = getStorybookInfo(packageJson);
if (!mainConfig) {
logger.warn('Unable to find storybook main.js config, skipping');
return null;
Expand Down
1 change: 1 addition & 0 deletions code/lib/cli/src/generators/baseGenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ export async function baseGenerator(

const packages = [
'storybook',
`@storybook/${renderer}`,
...frameworkPackages,
...addonPackages,
...extraPackages,
Expand Down
20 changes: 10 additions & 10 deletions code/lib/cli/src/helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,18 +67,18 @@ describe('Helpers', () => {
`(
`should copy $expected when folder $exists exists for language $language`,
async ({ language, exists, expected }) => {
const componentsDirectory = exists.map((folder: string) => `frameworks/react/${folder}`);
const expectedDirectory = `frameworks/react${expected}`;
const componentsDirectory = exists.map((folder: string) => `rendererAssets/react/${folder}`);
const expectedDirectory = `rendererAssets/react${expected}`;
(fse.pathExists as jest.Mock).mockImplementation((filePath) => {
return componentsDirectory.includes(filePath) || filePath === 'frameworks/react';
return componentsDirectory.includes(filePath) || filePath === 'rendererAssets/react';
});
await helpers.copyComponents('react', language);

const copySpy = jest.spyOn(fse, 'copy');
expect(copySpy).toHaveBeenNthCalledWith(1, expectedDirectory, './stories', expect.anything());
expect(copySpy).toHaveBeenNthCalledWith(
2,
'frameworks/common',
'rendererAssets/common',
'./stories',
expect.anything()
);
Expand All @@ -87,25 +87,25 @@ describe('Helpers', () => {

it(`should copy to src folder when exists`, async () => {
(fse.pathExists as jest.Mock).mockImplementation((filePath) => {
return filePath === 'frameworks/react' || filePath === './src';
return filePath === 'rendererAssets/react' || filePath === './src';
});
await helpers.copyComponents('react', SupportedLanguage.JAVASCRIPT);
expect(fse.copy).toHaveBeenCalledWith(expect.anything(), './src/stories', expect.anything());
});

it(`should copy to root folder when src doesn't exist`, async () => {
(fse.pathExists as jest.Mock).mockImplementation((filePath) => {
return filePath === 'frameworks/react';
return filePath === 'rendererAssets/react';
});
await helpers.copyComponents('react', SupportedLanguage.JAVASCRIPT);
expect(fse.copy).toHaveBeenCalledWith(expect.anything(), './stories', expect.anything());
});

it(`should throw an error for unsupported framework`, async () => {
const framework = 'unknown framework' as SupportedRenderers;
const expectedMessage = `Unsupported framework: ${framework}`;
it(`should throw an error for unsupported renderer`, async () => {
const renderer = 'unknown renderer' as SupportedRenderers;
const expectedMessage = `Unsupported renderer: ${renderer}`;
await expect(
helpers.copyComponents(framework, SupportedLanguage.JAVASCRIPT)
helpers.copyComponents(renderer, SupportedLanguage.JAVASCRIPT)
).rejects.toThrowError(expectedMessage);
});

Expand Down
32 changes: 13 additions & 19 deletions code/lib/cli/src/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,33 +181,27 @@ export function copyTemplate(templateRoot: string) {
fse.copySync(templateDir, '.', { overwrite: true });
}

export async function copyComponents(framework: SupportedRenderers, language: SupportedLanguage) {
export async function copyComponents(renderer: SupportedRenderers, language: SupportedLanguage) {
const languageFolderMapping: Record<SupportedLanguage, string> = {
javascript: 'js',
typescript: 'ts',
};
const componentsPath = async () => {
const baseDir = getBaseDir();
const frameworkPath = join(baseDir, 'frameworks', framework);
const languageSpecific = path.resolve(
__dirname,
`${frameworkPath}/${languageFolderMapping[language]}`
);
if (await fse.pathExists(languageSpecific)) {
return languageSpecific;
const assetsRoot = join(baseDir, 'rendererAssets');
const assetsRenderer = join(assetsRoot, renderer);
const assetsLanguage = join(assetsRenderer, languageFolderMapping[language]);
if (await fse.pathExists(assetsLanguage)) {
return assetsLanguage;
}
const jsFallback = path.resolve(
__dirname,
`${frameworkPath}/${languageFolderMapping.javascript}`
);
if (await fse.pathExists(jsFallback)) {
return jsFallback;
const assetsJS = join(assetsRenderer, languageFolderMapping.javascript);
if (await fse.pathExists(assetsJS)) {
return assetsJS;
}
const frameworkRootPath = path.resolve(__dirname, frameworkPath);
if (await fse.pathExists(frameworkRootPath)) {
return frameworkRootPath;
if (await fse.pathExists(assetsRenderer)) {
return assetsRenderer;
}
throw new Error(`Unsupported framework: ${framework}`);
throw new Error(`Unsupported renderer: ${renderer}`);
};

const targetPath = async () => {
Expand All @@ -219,7 +213,7 @@ export async function copyComponents(framework: SupportedRenderers, language: Su

const destinationPath = await targetPath();
await fse.copy(await componentsPath(), destinationPath, { overwrite: true });
await fse.copy(join(getBaseDir(), 'frameworks/common'), destinationPath, {
await fse.copy(join(getBaseDir(), 'rendererAssets/common'), destinationPath, {
overwrite: true,
});
}
Expand Down
27 changes: 19 additions & 8 deletions code/lib/core-common/src/utils/get-storybook-info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,21 @@ import { getStorybookConfiguration } from './get-storybook-configuration';
import { PackageJson } from '../types';

interface StorybookInfo {
framework: string;
version: string;
// FIXME: these are renderers for now,
// need to update with framework OR fix
// the calling code
framework: string;
frameworkPackage: string;
renderer: string;
rendererPackage: string;
configDir?: string;
mainConfig?: string;
previewConfig?: string;
managerConfig?: string;
}

const viewLayers: Record<string, string> = {
const rendererPackages: Record<string, string> = {
'@storybook/react': 'react',
'@storybook/vue': 'vue',
'@storybook/vue3': 'vue3',
Expand Down Expand Up @@ -42,11 +47,11 @@ const findDependency = (
Object.entries(peerDependencies || {}).find(predicate),
];

const getFrameworkInfo = (packageJson: PackageJson) => {
const getRendererInfo = (packageJson: PackageJson) => {
// Pull the viewlayer from dependencies in package.json
const [dep, devDep, peerDep] = findDependency(packageJson, ([key]) => viewLayers[key]);
const [dep, devDep, peerDep] = findDependency(packageJson, ([key]) => rendererPackages[key]);
const [pkg, version] = dep || devDep || peerDep || [];
const framework = pkg ? viewLayers[pkg] : undefined;
const renderer = pkg ? rendererPackages[pkg] : undefined;

if (dep && devDep && dep[0] === devDep[0]) {
logger.warn(
Expand All @@ -59,7 +64,13 @@ const getFrameworkInfo = (packageJson: PackageJson) => {
);
}

return { framework, version, frameworkPackage: pkg };
return {
version,
framework: renderer,
frameworkPackage: pkg,
renderer,
rendererPackage: pkg,
};
};

const validConfigExtensions = ['ts', 'js', 'tsx', 'jsx', 'mjs', 'cjs'];
Expand Down Expand Up @@ -89,11 +100,11 @@ const getConfigInfo = (packageJson: PackageJson) => {
};

export const getStorybookInfo = (packageJson: PackageJson) => {
const frameworkInfo = getFrameworkInfo(packageJson);
const rendererInfo = getRendererInfo(packageJson);
const configInfo = getConfigInfo(packageJson);

return {
...frameworkInfo,
...rendererInfo,
...configInfo,
} as StorybookInfo;
};
2 changes: 2 additions & 0 deletions code/lib/telemetry/src/storybook-metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,8 @@ export const computeStorybookMetadata = async ({

const hasStorybookEslint = !!allDependencies['eslint-plugin-storybook'];

// FIXME: resolve framework/renderer split in 7.0
// OR should be getting this from mainConfig instead?
const storybookInfo = getStorybookInfo(packageJson);

const storybookVersion =
Expand Down