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

Performance optimizations on separate manager preview split, improved cold start, restart & rebuild #4834

Merged
merged 29 commits into from
Nov 30, 2018
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
83a1800
CHANGE webpack configs, dev & static build for performance
ndelangen Nov 19, 2018
a78b909
FIX build-static
ndelangen Nov 26, 2018
03abc7e
Merge branch 'next' into separate-manager-preview-p5
ndelangen Nov 26, 2018
cc4fb66
FIX snapshots
ndelangen Nov 26, 2018
d06fe56
CHANGE output of dll to lib/core so it's easier to reference
ndelangen Nov 26, 2018
9b66549
FIX linting
ndelangen Nov 26, 2018
05418b2
cache DLL
Hypnosphi Nov 26, 2018
b198e08
CLEANUP
ndelangen Nov 26, 2018
ce9a748
FIX location of dll
ndelangen Nov 27, 2018
0064293
Merge branch 'next' into separate-manager-preview-p5
ndelangen Nov 27, 2018
4924083
FIX dll cache on teamcity (maybe)
ndelangen Nov 27, 2018
3502bef
CHANGE the dll express route to dev-server
ndelangen Nov 28, 2018
c467818
CLEANUP
ndelangen Nov 28, 2018
e7c24ba
MOVE licence notices into their own file && OPTIMISE the dll by speci…
ndelangen Nov 28, 2018
eaad2e7
Merge branch 'next' into separate-manager-preview-p5
ndelangen Nov 29, 2018
cf8e979
CHANGE to use terser over uglify
ndelangen Nov 29, 2018
7096cf3
RENAME config to webpackDllsConfig for clarity and easier search-ability
ndelangen Nov 29, 2018
b088ed0
RENAME script to createDlls for clarity & search-ability
ndelangen Nov 29, 2018
6efe2a5
Update lib/core/src/server/manager/manager-webpack.config.js
igor-dv Nov 29, 2018
b35aafb
Merge branch 'separate-manager-preview-p5' of github.com:storybooks/s…
ndelangen Nov 29, 2018
7b8c7ae
REFACTOR to use fs-extra because I hear people like async/await && RE…
ndelangen Nov 29, 2018
26e9357
RENAME script to create DLLs && REMOVE unneeded babel loader in DLL c…
ndelangen Nov 29, 2018
4141f48
REMOVE @ndelangen/html-webpack-harddisk-plugin
ndelangen Nov 29, 2018
998dab0
REFACTOR build-static to use fs-extra as well && CLEANUP
ndelangen Nov 29, 2018
8231388
ADD watch mode for static build back
ndelangen Nov 29, 2018
80e4eeb
FIX bug in checks for errors/warnings
ndelangen Nov 30, 2018
d781dc2
FIX dll copy command
ndelangen Nov 30, 2018
0b22923
FIX bootstrap script after rename to createDlls
ndelangen Nov 30, 2018
67d2629
FIX after rename of iframe to preview
ndelangen Nov 30, 2018
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 7 additions & 2 deletions .babelrc.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
module.exports = {
presets: ['@babel/preset-env', '@babel/preset-react', '@babel/preset-flow'],
presets: [
['@babel/preset-env', { shippedProposals: true }],
'@babel/preset-react',
'@babel/preset-flow',
],
plugins: [
'babel-plugin-emotion',
'babel-plugin-macros',
'@babel/plugin-proposal-class-properties',
'@babel/plugin-proposal-object-rest-spread',
'@babel/plugin-proposal-export-default-from',
[
'@babel/plugin-transform-runtime',
Expand All @@ -20,7 +25,7 @@ module.exports = {
overrides: [
{
test: './examples/vue-kitchen-sink',
presets: ['@babel/preset-env', 'babel-preset-vue'],
presets: [['@babel/preset-env', { shippedProposals: true }], 'babel-preset-vue'],
},
{
test: [
Expand Down
1 change: 1 addition & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
dist
lib/**/dll
build
coverage
node_modules
Expand Down
6 changes: 6 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,12 @@ module.exports = {
],
},
overrides: [
{
files: ['**/__tests__/**', '**/*.test.js/**', '**/*.spec.js/**'],
rules: {
'import/no-extraneous-dependencies': ignore,
},
},
{
files: ['**/react-native*/**', '**/REACT_NATIVE*/**', '**/crna*/**'],
rules: {
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,4 @@ integration/__image_snapshots__/__diff_output__
.jest-test-results.json
/examples/cra-kitchen-sink/src/__image_snapshots__/__diff_output__/
lib/*.jar
lib/**/dll
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ object OpenSourceProjects_Storybook_Bootstrap : BuildType({
addons/storyshots/*/dist/** => dist.zip/addons/storyshots
app/*/dist/** => dist.zip/app
lib/*/dist/** => dist.zip/lib
lib/core/dll/** => dist.zip/lib/core/dll
""".trimIndent()

vcs {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ object OpenSourceProjects_Storybook_Examples : BuildType({
name = "Examples"

artifactRules = """
${StorybookApp.values().map { it.artifactPath }.joinToString("\n")}
examples/official-storybook/storybook-static => official.zip
examples/official-storybook/image-snapshots/__image_snapshots__ => image-snapshots
""".trimIndent()
${StorybookApp.values().map { it.artifactPath }.joinToString("\n")}
examples/official-storybook/storybook-static => official.zip
examples/official-storybook/image-snapshots/__image_snapshots__ => image-snapshots
""".trimIndent()

vcs {
root(OpenSourceProjects_Storybook.vcsRoots.OpenSourceProjects_Storybook_HttpsGithubComStorybooksStorybookRefsHeadsMaster)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,9 @@ enum class StorybookApp(val appName: String, val exampleDir: String, val merged:
}

artifacts {
artifactRules = "dist.zip!**"
artifactRules = """
dist.zip!**
""".trimIndent()
}
}
}
Expand Down
1 change: 1 addition & 0 deletions addons/backgrounds/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
"@emotion/styled": "^0.10.6",
"@storybook/addons": "4.1.0-alpha.8",
"@storybook/core-events": "4.1.0-alpha.8",
"eventemitter3": "^3.1.0",
"global": "^4.3.2",
"prop-types": "^15.6.2",
"util-deprecate": "^1.0.2"
Expand Down
2 changes: 1 addition & 1 deletion addons/backgrounds/src/__tests__/BackgroundPanel.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react';
import { shallow, mount } from 'enzyme';
import EventEmitter from 'events';
import EventEmitter from 'eventemitter3';

import BackgroundPanel from '../BackgroundPanel';
import Events from '../constants';
Expand Down
4 changes: 2 additions & 2 deletions addons/options/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ To install type definitions: `npm install -D @types/storybook__addon-options`

Make sure you also have the type definitions installed for the following libs:

- node
- react
- node
- react

You can install them using `npm install -D @types/node @types/react`, assuming you are using Typescript >2.0.
7 changes: 4 additions & 3 deletions lib/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@
"storybook"
],
"homepage": "https://github.com/storybooks/storybook/tree/master/lib/core",
"publishConfig": {
"access": "public"
},
"bugs": {
"url": "https://github.com/storybooks/storybook/issues"
},
Expand Down Expand Up @@ -53,6 +50,7 @@
"detect-port": "^1.2.3",
"dotenv-webpack": "^1.5.7",
"ejs": "^2.6.1",
"eventemitter3": "^3.1.0",
"express": "^4.16.3",
"file-loader": "^2.0.0",
"file-system-cache": "^1.0.5",
Expand Down Expand Up @@ -94,5 +92,8 @@
"babel-loader": "^7.0.0 || ^8.0.0",
"react": ">=16.3.0",
"react-dom": ">=16.3.0"
},
"publishConfig": {
"access": "public"
}
}
2 changes: 1 addition & 1 deletion lib/core/src/client/preview/story_store.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* eslint no-underscore-dangle: 0 */
import { EventEmitter } from 'events';
import EventEmitter from 'eventemitter3';
import Events from '@storybook/core-events';

let count = 0;
Expand Down
16 changes: 15 additions & 1 deletion lib/core/src/server/build-dev.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ import { getDevCli } from './cli';

const defaultFavIcon = require.resolve('./public/favicon.ico');

const cacheDir = findCacheDir({ name: 'storybook' });
const cache = Cache({
basePath: findCacheDir({ name: 'storybook' }),
basePath: cacheDir,
ns: 'storybook', // Optional. A grouping namespace for items.
});

Expand Down Expand Up @@ -52,6 +53,7 @@ function getServer(app, options) {

function applyStatic(app, options) {
const { staticDir } = options;

let hasCustomFavicon = false;

if (staticDir) {
Expand Down Expand Up @@ -224,6 +226,18 @@ export async function buildDevStandalone(options) {
);

if (options.smokeTest) {
fs.writeFileSync(
ndelangen marked this conversation as resolved.
Show resolved Hide resolved
path.join(cacheDir, 'preview-stats.json'),
JSON.stringify(iframeStats.toJson(), null, 2),
'utf8'
);
fs.writeFileSync(
path.join(cacheDir, 'manager-stats.json'),
JSON.stringify(managerStats.toJson(), null, 2),
'utf8'
);
logger.info(`stats written to => ${chalk.cyan(path.join(cacheDir, '[name].json'))}`);

process.exit(iframeStats.toJson().warnings.length ? 1 : 0);
process.exit(managerStats.toJson().warnings.length ? 1 : 0);
} else if (!options.ci) {
Expand Down
170 changes: 99 additions & 71 deletions lib/core/src/server/build-static.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,54 +11,22 @@ import loadManagerConfig from './manager/manager-config';

const defaultFavIcon = require.resolve('./public/favicon.ico');

export async function buildStaticStandalone(options) {
const { staticDir, watch, configDir, packageJson } = options;
export function buildStaticStandalone(options) {
const { staticDir, configDir, packageJson } = options;

const configType = 'PRODUCTION';
const outputDir = path.join(process.cwd(), options.outputDir);

const dllPath = path.join(__dirname, '../../dll/storybook_ui_dll.js');
igor-dv marked this conversation as resolved.
Show resolved Hide resolved

// create output directory if not exists
shelljs.mkdir('-p', outputDir);
// clear the static dir
shelljs.rm('-rf', path.join(outputDir, 'static'));
shelljs.cp(defaultFavIcon, outputDir);

logger.info('building manager..');
const managerStartTime = process.hrtime();

const managerConfig = await loadManagerConfig({
configType,
outputDir,
configDir,
corePresets: [require.resolve('./manager/manager-preset.js')],
});

await new Promise((res, rej) => {
webpack(managerConfig).run((err, stats) => {
const managerTotalTime = process.hrtime(managerStartTime);
logger.trace({ message: 'manager built', time: managerTotalTime });

if (stats.hasErrors()) {
rej(stats);
} else {
res(stats);
}
});
});
shelljs.mkdir('-p', path.join(outputDir, 'sb_dll'));

// Build the webpack configuration using the `baseConfig`
// custom `.babelrc` file and `webpack.config.js` files
// NOTE changes to env should be done before calling `getBaseConfig`
const config = await loadConfig({
configType,
outputDir,
packageJson,
corePresets: [require.resolve('./preview/preview-preset.js')],
overridePresets: [require.resolve('./preview/custom-webpack-preset.js')],
...options,
});
logger.info('clean outputDir..');
shelljs.rm('-rf', path.join(outputDir, 'static'));

// config.output.path = path.resolve(outputDir);
shelljs.cp(defaultFavIcon, outputDir);

// copy all static files
if (staticDir) {
Expand All @@ -72,42 +40,102 @@ export async function buildStaticStandalone(options) {
});
}

// compile all resources with webpack and write them to the disk.
return new Promise((resolve, reject) => {
const previewStartTime = process.hrtime();

const webpackCb = (err, stats) => {
if (err || stats.hasErrors()) {
logger.error('Failed to build the storybook');
// eslint-disable-next-line no-unused-expressions
err && logger.error(err.message);
// eslint-disable-next-line no-unused-expressions
stats && stats.hasErrors() && stats.toJson().errors.forEach(e => logger.error(e));
process.exitCode = 1;
return reject(err);
logger.info('copy dll..');
shelljs.cp(dllPath, path.join(outputDir, 'sb_dll'));

let startTime;

return new Promise(res => {
ndelangen marked this conversation as resolved.
Show resolved Hide resolved
logger.info('building manager..');
startTime = process.hrtime();

res();
})
.then(() =>
loadManagerConfig({
configType,
outputDir,
configDir,
corePresets: [require.resolve('./manager/manager-preset.js')],
})
)
.then(
managerConfig =>
new Promise((resolve, reject) => {
webpack(managerConfig).run((err, stats) => {
if (err || !stats || stats.hasErrors()) {
logger.error('Failed to build the manager');
// eslint-disable-next-line no-unused-expressions
err && logger.error(err.message);
// eslint-disable-next-line no-unused-expressions
stats && stats.hasErrors() && stats.toJson().errors.forEach(e => logger.error(e));
process.exitCode = 1;

reject(stats);
ndelangen marked this conversation as resolved.
Show resolved Hide resolved
return;
}

resolve(stats);
});
})
)
.then(stats => {
logger.trace({ message: 'manager built', time: process.hrtime(startTime) });
stats.toJson().warnings.forEach(e => logger.warn(e));
})
.then(() => {
startTime = process.hrtime();
logger.info('building preview..');

return loadConfig({
configType,
outputDir,
packageJson,
corePresets: [require.resolve('./preview/preview-preset.js')],
overridePresets: [require.resolve('./preview/custom-webpack-preset.js')],
...options,
});
})
.then(
config =>
new Promise((resolve, reject) => {
webpack(config).run((err, stats) => {
ndelangen marked this conversation as resolved.
Show resolved Hide resolved
if (err || !stats || stats.hasErrors()) {
logger.error('Failed to build the preview');
// eslint-disable-next-line no-unused-expressions
err && logger.error(err.message);
// eslint-disable-next-line no-unused-expressions
stats && stats.hasErrors() && stats.toJson().errors.forEach(e => logger.error(e));
process.exitCode = 1;
reject(err);
ndelangen marked this conversation as resolved.
Show resolved Hide resolved
return;
}

resolve(stats);
});
})
)
.then(stats => {
logger.trace({ message: 'preview built', time: process.hrtime(startTime) });
stats.toJson().warnings.forEach(e => logger.warn(e));
logger.info(`output directory: ${outputDir}`);
})
.catch(p => {
if (p && p.toJson) {
const stats = p.toJson();

stats.errors.forEach(e => logger.error(e));
stats.warnings.forEach(e => logger.error(e));
} else {
logger.error(p);
}

const previewTotalTime = process.hrtime(previewStartTime);
logger.trace({ message: 'preview built', time: previewTotalTime });

return resolve(stats);
};

logger.info('building preview..');
const compiler = webpack(config);

if (watch) {
compiler.watch({}, webpackCb);
} else {
compiler.run(webpackCb);
}
});
});
}

export async function buildStatic({ packageJson, ...loadOptions }) {
export function buildStatic({ packageJson, ...loadOptions }) {
Copy link
Contributor

Choose a reason for hiding this comment

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

since this function returns a promise I think it should be async still

Copy link
Member Author

Choose a reason for hiding this comment

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

async should only be used in combination with await right?

Copy link
Contributor

Choose a reason for hiding this comment

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

The typescript compiler is becoming part of my brain! https://javascript.info/async-await The async keyword on a function just means that the function will always return a promise. It doesn't have to be used in combination with await at all.

Probably an unneeded change, but it does communicate easily that this function returns a promise

const cliOptions = getProdCli(packageJson);

await buildStaticStandalone({
return buildStaticStandalone({
...cliOptions,
...loadOptions,
packageJson,
Expand Down