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 pnp support & add auto-detection #21046

Merged
merged 6 commits into from
Feb 15, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ jobs:
type: integer
default: 2
executor:
class: medium+
class: large
name: sb_node_16_browsers
parallelism: << parameters.parallelism >>
steps:
Expand Down
2 changes: 0 additions & 2 deletions .yarnrc.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
installStatePath: ./.yarn/root-install-state.gz

nodeLinker: node-modules

yarnPath: .yarn/releases/yarn-3.4.1.cjs
2 changes: 0 additions & 2 deletions code/.yarnrc.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ logFilters:
level: discard
- code: YN0076
level: discard
- level: discard
pattern: '@storybook/root@workspace:.'
- level: discard
pattern: '@workspace:addons/storyshots-*/'

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 @@ -69,11 +69,13 @@
"commander": "^6.2.1",
"cross-spawn": "^7.0.3",
"detect-indent": "^6.1.0",
"download-tarball": "^2.0.0",
"envinfo": "^7.7.3",
"execa": "^5.0.0",
"express": "^4.17.3",
"find-up": "^5.0.0",
"fs-extra": "^11.1.0",
"get-npm-tarball-url": "^2.0.3",
"get-port": "^5.1.1",
"giget": "^1.0.0",
"globby": "^11.0.2",
Expand Down
4 changes: 4 additions & 0 deletions code/lib/cli/src/detect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ jest.mock('fs', () => ({
access: jest.fn(),
}));

jest.mock('fs-extra', () => ({
pathExistsSync: jest.fn(() => true),
}));

jest.mock('path', () => ({
// make it return just the second path, for easier testing
join: jest.fn((_, p) => p),
Expand Down
6 changes: 6 additions & 0 deletions code/lib/cli/src/detect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import findUp from 'find-up';
import semver from 'semver';
import { logger } from '@storybook/node-logger';

import { pathExistsSync } from 'fs-extra';
import { join } from 'path';
import type { TemplateConfiguration, TemplateMatcher } from './project_types';
import {
ProjectType,
Expand Down Expand Up @@ -148,6 +150,10 @@ export function isStorybookInstalled(
return false;
}

export function detectPnp() {
return pathExistsSync(join(process.cwd(), '.pnp.cjs'));
}

export function detectLanguage(packageJson?: PackageJson) {
let language = SupportedLanguage.JAVASCRIPT;

Expand Down
61 changes: 53 additions & 8 deletions code/lib/cli/src/dirs.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,63 @@
import { dirname } from 'path';
import type { SupportedFrameworks, SupportedRenderers } from './project_types';
import { dirname, join } from 'path';

// @ts-expect-error (has no typings)
import downloadTarball from 'download-tarball';
import getNpmTarballUrl from 'get-npm-tarball-url';
import * as tempy from 'tempy';

import { externalFrameworks } from './project_types';
import type { SupportedFrameworks, SupportedRenderers } from './project_types';
import type { JsPackageManager } from './js-package-manager';
import versions from './versions';

export function getCliDir() {
return dirname(require.resolve('@storybook/cli/package.json'));
}

export function getRendererDir(renderer: SupportedFrameworks | SupportedRenderers) {
const resolveUsingBranchInstall = async (packageManager: JsPackageManager, request: string) => {
const tempDirectory = tempy.directory();
const name = request as keyof typeof versions;

// FIXME: this might not be the right version for community packages
const { dependencies, peerDependencies, devDependencies } = packageManager.readPackageJson();
const all = { ...dependencies, ...peerDependencies, ...devDependencies };
const version = versions[name] || (await packageManager.latestVersion(request));

const url = getNpmTarballUrl(request, version, { registryUrl: packageManager.getRegistryURL() });

// this unzips the tarball into the temp directory
await downloadTarball({ url, dir: tempDirectory });

return join(tempDirectory, 'package');
};

export async function getRendererDir(
packageManager: JsPackageManager,
renderer: SupportedFrameworks | SupportedRenderers
) {
const externalFramework = externalFrameworks.find((framework) => framework.name === renderer);
const frameworkPackageName =
externalFramework?.renderer || externalFramework?.packageName || `@storybook/${renderer}`;
return dirname(
require.resolve(`${frameworkPackageName}/package.json`, {
paths: [process.cwd()],
})
);

const packageJsonPath = `${frameworkPackageName}/package.json`;

const errors: Error[] = [];

try {
return dirname(
require.resolve(packageJsonPath, {
paths: [process.cwd()],
})
);
} catch (e) {
errors.push(e);
}

try {
return await resolveUsingBranchInstall(packageManager, frameworkPackageName);
} catch (e) {
errors.push(e);
}

throw new Error(`Cannot find ${packageJsonPath}, ${errors.map((e) => e.stack).join('\n\n')}`);
}
1 change: 1 addition & 0 deletions code/lib/cli/src/generators/REACT_NATIVE/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ const generator = async (
const storybookConfigFolder = '.storybook';

await copyTemplateFiles({
packageManager,
renderer: 'react-native',
language: SupportedLanguage.JAVASCRIPT,
destination: storybookConfigFolder,
Expand Down
5 changes: 3 additions & 2 deletions code/lib/cli/src/generators/configure.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ describe('configureMain', () => {
expect(mainConfigPath).toEqual('./.storybook/main.ts');
expect(mainConfigContent).toMatchInlineSnapshot(`
"import type { StorybookConfig } from '@storybook/react-vite';

const config: StorybookConfig = {
\\"stories\\": [
\\"../stories/**/*.mdx\\",
Expand Down Expand Up @@ -91,7 +90,9 @@ describe('configureMain', () => {

expect(mainConfigPath).toEqual('./.storybook/main.js');
expect(mainConfigContent).toMatchInlineSnapshot(`
"/** @type { import('@storybook/react-webpack5').StorybookConfig } */
"import path from 'path';

/** @type { import('@storybook/react-webpack5').StorybookConfig } */
const config = {
\\"stories\\": [
\\"../stories/**/*.mdx\\",
Expand Down
26 changes: 17 additions & 9 deletions code/lib/cli/src/generators/configure.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,18 +74,26 @@ export async function configureMain({
logger.warn('Could not find framework package name');
}

const mainJsContents = mainConfigTemplate
.replace(
'<<import>>',
isTypescript
? `import type { StorybookConfig } from '${frameworkPackage}';\n\n`
: `/** @type { import('${frameworkPackage}').StorybookConfig } */\n`
)
.replace('<<type>>', isTypescript ? ': StorybookConfig' : '')
.replace('<<mainContents>>', JSON.stringify(config, null, 2))
const mainContents = JSON.stringify(config, null, 2)
.replace(/['"]%%/g, '')
.replace(/%%['"]/g, '');

const imports = [];

if (custom.framework?.name.includes('path.dirname(')) {
imports.push(`import path from 'path';`);
}

if (isTypescript) {
imports.push(`import type { StorybookConfig } from '${frameworkPackage}';`);
} else {
imports.push(`/** @type { import('${frameworkPackage}').StorybookConfig } */`);
}

const mainJsContents = mainConfigTemplate
.replace('<<import>>', `${imports.join('\n\n')}\n`)
.replace('<<type>>', isTypescript ? ': StorybookConfig' : '')
.replace('<<mainContents>>', mainContents);
await fse.writeFile(
`./${storybookConfigFolder}/main.${isTypescript ? 'ts' : 'js'}`,
dedent(mainJsContents),
Expand Down
3 changes: 2 additions & 1 deletion code/lib/cli/src/helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@ import fs from 'fs';
import fse from 'fs-extra';

import * as helpers from './helpers';
import type { JsPackageManager } from './js-package-manager';
import type { SupportedRenderers } from './project_types';
import { SupportedLanguage } from './project_types';

jest.mock('fs', () => ({
existsSync: jest.fn(),
}));
jest.mock('./dirs', () => ({
getRendererDir: (renderer: string) => `@storybook/${renderer}`,
getRendererDir: (_: JsPackageManager, renderer: string) => `@storybook/${renderer}`,
getCliDir: () => '@storybook/cli',
}));

Expand Down
6 changes: 4 additions & 2 deletions code/lib/cli/src/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,13 +187,15 @@ export function copyTemplate(templateRoot: string, destination = '.') {
}

type CopyTemplateFilesOptions = {
packageManager?: JsPackageManager;
renderer: SupportedFrameworks | SupportedRenderers;
language: SupportedLanguage;
includeCommonAssets?: boolean;
destination?: string;
};

export async function copyTemplateFiles({
packageManager,
renderer,
language,
destination,
Expand All @@ -205,7 +207,7 @@ export async function copyTemplateFiles({
[SupportedLanguage.TYPESCRIPT_4_9]: 'ts-4-9',
};
const templatePath = async () => {
const baseDir = getRendererDir(renderer);
const baseDir = await getRendererDir(packageManager, renderer);
const assetsDir = join(baseDir, 'template/cli');

const assetsLanguage = join(assetsDir, languageFolderMapping[language]);
Expand All @@ -228,7 +230,7 @@ export async function copyTemplateFiles({
if (await fse.pathExists(assetsDir)) {
return assetsDir;
}
throw new Error(`Unsupported renderer: ${renderer}`);
throw new Error(`Unsupported renderer: ${renderer} (${baseDir})`);
};

const targetPath = async () => {
Expand Down
5 changes: 3 additions & 2 deletions code/lib/cli/src/initiate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { telemetry } from '@storybook/telemetry';
import { withTelemetry } from '@storybook/core-server';

import { installableProjectTypes, ProjectType } from './project_types';
import { detect, isStorybookInstalled, detectLanguage, detectBuilder } from './detect';
import { detect, isStorybookInstalled, detectLanguage, detectBuilder, detectPnp } from './detect';
import { commandLog, codeLog, paddedLog } from './helpers';
import angularGenerator from './generators/ANGULAR';
import aureliaGenerator from './generators/AURELIA';
Expand Down Expand Up @@ -57,12 +57,13 @@ const installStorybook = <Project extends ProjectType>(
}

const language = detectLanguage(packageJson);
const pnp = detectPnp();

const generatorOptions = {
language,
builder: options.builder || detectBuilder(packageManager),
linkable: !!options.linkable,
pnp: options.usePnp,
pnp: pnp || options.usePnp,
};

const runGenerator: () => Promise<any> = async () => {
Expand Down
3 changes: 1 addition & 2 deletions code/lib/cli/src/repro-generators/scripts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,7 @@ const addLocalPackageResolutions = async ({ cwd }: Options) => {
const workspaceDir = path.join(__dirname, '..', '..', '..', '..', '..');
const { stdout } = await command('yarn workspaces list --json', { cwd: workspaceDir });

console.log({ stdout, workspaceDir });
const workspaces = JSON.parse(`[${stdout.split('\n').join(',')}]`);
console.log({ workspaces });

packageJson.resolutions = Object.keys(storybookVersions).reduce((acc, key) => {
return {
Expand All @@ -126,6 +124,7 @@ const installYarn2 = async ({ cwd, pnp, name }: Options) => {
const command = [
`yarn set version berry`,
`yarn config set enableGlobalCache true`,
`yarn config set checksumBehavior ignore`,
`yarn config set nodeLinker ${pnp ? 'pnp' : 'node-modules'}`,
];

Expand Down
7 changes: 7 additions & 0 deletions code/lib/cli/src/sandbox-templates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,13 @@ const internalTemplates = {
},
},
},
'internal/pnp': {
...baseTemplates['react-webpack/18-ts'],
name: 'PNP (react-webpack/18-ts)',
script: 'yarn create react-app . --use-pnp',
isInternal: true,
inDevelopment: true,
},
} satisfies Record<`internal/${string}`, Template & { isInternal: true }>;

export const allTemplates: Record<TemplateKey, Template> = {
Expand Down
5 changes: 3 additions & 2 deletions code/lib/csf-tools/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,11 @@
".": {
"node": "./dist/index.js",
"require": "./dist/index.js",
"import": "./dist/index.mjs",
"types": "./dist/index.d.ts"
},
"./package.json": "./package.json"
},
"main": "dist/index.js",
"module": "dist/index.mjs",
"types": "dist/index.d.ts",
"files": [
"dist/**/*",
Expand Down Expand Up @@ -65,6 +63,9 @@
"bundler": {
"entries": [
"./src/index.ts"
],
"formats": [
"cjs"
]
},
"gitHead": "a591d8eb579e68b26c277ab8ebdcafc2611530a5"
Expand Down
9 changes: 9 additions & 0 deletions code/lib/csf-tools/src/ConfigFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,15 @@ export class ConfigFile {
return undefined;
}

getSafeFieldValue(path: string[]) {
try {
return this.getFieldValue(path);
} catch (e) {
//
}
return undefined;
}

setFieldNode(path: string[], expr: t.Expression) {
const [first, ...rest] = path;
const exportNode = this._exports[first];
Expand Down
Loading