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: Addon postinstall hooks #8700

Merged
merged 13 commits into from
Nov 14, 2019
1 change: 1 addition & 0 deletions addons/docs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"angular/**/*",
"common/**/*",
"html/**/*",
"postinstall/**/*",
"react/**/*",
"vue/**/*",
"web-components/**/*",
Expand Down
47 changes: 47 additions & 0 deletions addons/docs/postinstall/presets.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
const fs = require('fs');
const { addPreset, getFrameworks } = require('@storybook/addons');
ndelangen marked this conversation as resolved.
Show resolved Hide resolved
// eslint-disable-next-line import/no-extraneous-dependencies
const { logger } = require('@storybook/node-logger');

const SUPPORTED_FRAMEWORKS = ['angular', 'html', 'react', 'vue', 'web-components'];

export default function transformer(file, api) {
const packageJson = JSON.parse(fs.readFileSync('./package.json'));
const frameworks = getFrameworks(packageJson);

let err = null;
let framework = null;
let presetOptions = null;
if (frameworks.length !== 1) {
err = `${frameworks.length === 0 ? 'No' : 'Multiple'} frameworks found: ${frameworks}`;
} else {
// eslint-disable-next-line prefer-destructuring
framework = frameworks[0];
if (!SUPPORTED_FRAMEWORKS.includes(framework)) {
err = `Unsupported framework: '${framework}'`;
}
}

if (err) {
logger.error(`${err}, please configure '@storybook/addon-docs' manually.`);
return file.source;
}

const { dependencies, devDependencies } = packageJson;
if (
framework === 'react' &&
((dependencies && dependencies['react-scripts']) ||
(devDependencies && devDependencies['react-scripts']))
) {
presetOptions = {
configureJSX: true,
};
}

const j = api.jscodeshift;
const root = j(file.source);

addPreset(`@storybook/addon-docs/${framework}/preset`, presetOptions, { root, api });

return root.toSource();
}
4 changes: 3 additions & 1 deletion lib/addons/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@
"util-deprecate": "^1.0.2"
},
"devDependencies": {
"@types/util-deprecate": "^1.0.0"
"@hypnosphi/jscodeshift": "^0.6.4",
"@types/util-deprecate": "^1.0.0",
"jest-specific-snapshot": "^2.0.0"
},
"publishConfig": {
"access": "public"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = ['foo'];
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`add-preset-options transforms correctly using "basic.input.js" data 1`] = `
"module.exports = ['foo', {
name: \\"test\\",
options: {\\"a\\":[1,2,3],\\"b\\":{\\"foo\\":\\"bar\\"},\\"c\\":\\"baz\\"}
}];"
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`add-preset-options transforms correctly using "empty.input.js" data 1`] = `
"module.exports = [{
name: \\"test\\",
options: {\\"a\\":[1,2,3],\\"b\\":{\\"foo\\":\\"bar\\"},\\"c\\":\\"baz\\"}
}];"
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = ['foo'];
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`add-preset transforms correctly using "basic.input.js" data 1`] = `"module.exports = ['foo', \\"test\\"];"`;
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`add-preset transforms correctly using "empty.input.js" data 1`] = `"module.exports = [\\"test\\"];"`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { addPreset } from '../codemods';

export default function transformer(file, api) {
const j = api.jscodeshift;
const root = j(file.source);

const options = {
a: [1, 2, 3],
b: { foo: 'bar' },
c: 'baz',
};

addPreset('test', options, { root, api });

return root.toSource();
}
10 changes: 10 additions & 0 deletions lib/addons/src/postinstall/__testtransforms__/add-preset.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { addPreset } from '../codemods';

export default function transformer(file, api) {
const j = api.jscodeshift;
const root = j(file.source);

addPreset('test', null, { root, api });

return root.toSource();
}
32 changes: 32 additions & 0 deletions lib/addons/src/postinstall/codemods.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import path from 'path';
import fs from 'fs';
import 'jest-specific-snapshot';
// TODO move back to original 'jscodeshift' package as soon as https://github.com/facebook/jscodeshift/pull/297 is released
import { applyTransform } from '@hypnosphi/jscodeshift/dist/testUtils';

jest.mock('@storybook/node-logger');

const inputRegExp = /\.input\.js$/;

const fixturesDir = path.resolve(__dirname, './__testfixtures__');
fs.readdirSync(fixturesDir).forEach(transformName => {
const transformFixturesDir = path.join(fixturesDir, transformName);
// eslint-disable-next-line jest/valid-describe
describe(transformName, () =>
fs
.readdirSync(transformFixturesDir)
.filter(fileName => inputRegExp.test(fileName))
.forEach(fileName => {
const inputPath = path.join(transformFixturesDir, fileName);
it(`transforms correctly using "${fileName}" data`, () =>
expect(
applyTransform(
// eslint-disable-next-line global-require,import/no-dynamic-require
require(path.join(__dirname, '__testtransforms__', transformName)),
null,
{ path: inputPath, source: fs.readFileSync(inputPath, 'utf8') }
)
).toMatchSpecificSnapshot(inputPath.replace(inputRegExp, '.output.snapshot')));
})
);
});
50 changes: 50 additions & 0 deletions lib/addons/src/postinstall/codemods.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
interface PostinstallContext {
root: any;
api: any;
}

export function addPreset(preset: string, presetOptions: any, { api, root }: PostinstallContext) {
Copy link
Member

Choose a reason for hiding this comment

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

Why is this part of addons? It seems not meant to be part of either the manager or preview's code at all?

Copy link
Member Author

Choose a reason for hiding this comment

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

Didn't want add-ons to have to take that dependency. You'll notice that this uses dependency injection to avoid needing any extra deps

const j = api.jscodeshift;
const moduleExports: any[] = [];
root
.find(j.AssignmentExpression)
.filter(
(assignment: any) =>
assignment.node.left.type === 'MemberExpression' &&
assignment.node.left.object.name === 'module' &&
assignment.node.left.property.name === 'exports'
)
.forEach((exp: any) => moduleExports.push(exp));

let exportArray = null;
switch (moduleExports.length) {
case 0: {
exportArray = j.arrayExpression([]);
const exportStatement = j.assignmentStatement(
'=',
j.memberExpression(j.identifier('module'), j.identifier('exports')),
exportArray
);
root.get().node.program.body.push(exportStatement);
break;
}
case 1:
exportArray = moduleExports[0].node.right;
break;
default:
throw new Error('Multiple module export statements');
}

let presetConfig = j.literal(preset);
if (presetOptions) {
const optionsJson = `const x = ${JSON.stringify(presetOptions)}`;
const optionsRoot = j(optionsJson);
const optionsNode = optionsRoot.find(j.VariableDeclarator).get().node.init;

presetConfig = j.objectExpression([
j.property('init', j.identifier('name'), j.literal(preset)),
j.property('init', j.identifier('options'), optionsNode),
]);
}
exportArray.elements.push(presetConfig);
}
41 changes: 41 additions & 0 deletions lib/addons/src/postinstall/frameworks.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { getFrameworks } from './frameworks';

const REACT = {
'@storybook/react': '5.2.5',
};

const VUE = {
'@storybook/vue': '5.2.5',
};

const NONE = {
'@storybook/addons': '5.2.5',
lodash: '^4.17.15',
};

describe('getFrameworks', () => {
it('single framework', () => {
const frameworks = getFrameworks({
dependencies: NONE,
devDependencies: REACT,
});
expect(frameworks).toEqual(['react']);
});
it('multi-framework', () => {
const frameworks = getFrameworks({
dependencies: VUE,
devDependencies: REACT,
});
expect(frameworks.sort()).toEqual(['react', 'vue']);
});
it('no deps', () => {
const frameworks = getFrameworks({});
expect(frameworks).toEqual([]);
});
it('no framework', () => {
const frameworks = getFrameworks({
dependencies: NONE,
});
expect(frameworks).toEqual([]);
});
});
30 changes: 30 additions & 0 deletions lib/addons/src/postinstall/frameworks.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
type Deps = Record<string, string>;
interface PackageJson {
dependencies?: Deps;
devDependencies?: Deps;
}

const FRAMEWORKS = [
'angular',
'ember',
'html',
'marko',
'mithril',
'polymer',
'preact',
'rax',
'react',
'react-native',
'riot',
'svelte',
'vue',
'web-components',
];

export const getFrameworks = ({ dependencies, devDependencies }: PackageJson): string[] => {
const allDeps: Deps = {};
Object.assign(allDeps, dependencies || {});
Object.assign(allDeps, devDependencies || {});

return FRAMEWORKS.filter(f => !!allDeps[`@storybook/${f}`]);
};
2 changes: 2 additions & 0 deletions lib/addons/src/postinstall/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export * from './codemods';
export * from './frameworks';
2 changes: 2 additions & 0 deletions lib/addons/src/public_api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,6 @@ export * from './types';
export * from './storybook-channel-mock';
export * from './hooks';

export * from './postinstall';

export default addons;
1 change: 1 addition & 0 deletions lib/addons/src/typings.d.ts
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
declare module 'global';
declare module '@hypnosphi/jscodeshift/dist/testUtils';
1 change: 1 addition & 0 deletions lib/cli/bin/generate.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ if (process.argv[1].includes('getstorybook')) {
.command('add <addon>')
.description('Add an addon to your Storybook')
.option('-N --use-npm', 'Use NPM to build the Storybook server')
.option('-s --skip-postinstall', 'Skip package specific postinstall config modifications')
.action((addonName, options) => add(addonName, options));

program
Expand Down
44 changes: 31 additions & 13 deletions lib/cli/lib/add.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,18 +90,34 @@ export const addStorybookAddonToFile = (addonName, addonsFile, isOfficialAddon)
];
};

const registerAddon = (addonName, isOfficialAddon) => {
const registerDone = commandLog(`Registering the ${addonName} Storybook addon`);
const addonsFilePath = path.resolve('.storybook/addons.js');
const addonsFile = fs
.readFileSync(addonsFilePath)
.toString()
.split('\n');
fs.writeFileSync(
addonsFilePath,
addStorybookAddonToFile(addonName, addonsFile, isOfficialAddon).join('\n')
);
registerDone();
const postinstallAddon = async (addonName, isOfficialAddon) => {
let skipMsg = null;
if (!isOfficialAddon) {
skipMsg = 'unofficial addon';
} else {
const presetsCodemod = require.resolve(
`${getPackageName(addonName, isOfficialAddon)}/postinstall/presets.js`
Copy link
Member

Choose a reason for hiding this comment

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

Are you expecting other files other then preset.js here?
Why not just make it index.js?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah will add support for other files in future

);
if (fs.existsSync(presetsCodemod)) {
if (fs.existsSync('.storybook')) {
Copy link
Member

Choose a reason for hiding this comment

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

I guess there's no way to know if there's a non-standard configDir?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the only way is to ok infer it from package.json which is what chromatic does

commandLog(`Running postinstall script for ${addonName}`)();
const presetsFile = path.join('.storybook', 'presets.js');
if (!fs.existsSync(presetsFile)) {
fs.writeFileSync(presetsFile, '', 'utf8');
}
spawnSync('npx', ['jscodeshift', '-t', presetsCodemod, presetsFile], {
Copy link
Member

Choose a reason for hiding this comment

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

So the postinstall is actually a "post-add-modemod". It's limited to be exactly a codemod for presets.js only?

I THINK presets.js can be a .ts file too.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope AFAIK it can't

stdio: 'inherit',
});
} else {
skipMsg = 'no .storybook config';
}
} else {
skipMsg = `no codemod: ${presetsCodemod}`;
}
}
if (skipMsg) {
commandLog(`Skipping postinstall for ${addonName}, ${skipMsg}`)();
}
};

export default async function add(addonName, options) {
Expand All @@ -119,5 +135,7 @@ export default async function add(addonName, options) {
}
addonCheckDone();
installAddon(addonName, npmOptions, isOfficialAddon);
registerAddon(addonName, isOfficialAddon);
if (!options.skipPostinstall) {
await postinstallAddon(addonName, isOfficialAddon);
}
}