Skip to content

Adds Danger and a rule showing build size differences #11865

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

Merged
merged 7 commits into from
Jan 17, 2018
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
151 changes: 151 additions & 0 deletions dangerfile.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
/**
* Copyright (c) 2013-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

'use strict';

const {markdown, danger} = require('danger');
const fetch = require('node-fetch');

const {generateResultsArray} = require('./scripts/rollup/stats');
const {readFileSync} = require('fs');

const currentBuildResults = JSON.parse(
readFileSync('./scripts/rollup/results.json')
);

/**
* Generates a Markdown table
* @param {string[]} headers
* @param {string[][]} body
*/
function generateMDTable(headers, body) {
const tableHeaders = [
headers.join(' | '),
headers.map(() => ' --- ').join(' | '),
];

const tablebody = body.map(r => r.join(' | '));
return tableHeaders.join('\n') + '\n' + tablebody.join('\n');
}

/**
* Generates a user-readable string from a percentage change
* @param {string[]} headers
*/
function emojiPercent(change) {
if (change > 0) {
return `:small_red_triangle:+${change}%`;
} else if (change <= 0) {
return `${change}%`;
}
}

// Grab the results.json before we ran CI via the GH API
// const baseMerge = danger.github.pr.base.sha
const parentOfOldestCommit = danger.git.commits[0].parents[0];
const commitURL = sha =>
`http://react.zpao.com/builds/master/_commits/${sha}/results.json`;

fetch(commitURL(parentOfOldestCommit)).then(async response => {
const previousBuildResults = await response.json();
const results = generateResultsArray(
currentBuildResults,
previousBuildResults
);

const percentToWarrentShowing = 1;
const packagesToShow = results
.filter(
r =>
Math.abs(r.prevFileSizeChange) > percentToWarrentShowing ||
Math.abs(r.prevGzipSizeChange) > percentToWarrentShowing
)
.map(r => r.packageName);

if (packagesToShow.length) {
let allTables = [];

// Highlight React and React DOM changes inline
// e.g. react: `react.production.min.js`: -3%, `react.development.js`: +4%

if (packagesToShow.includes('react')) {
const reactProd = results.find(
r => r.bundleType === 'UMD_PROD' && r.packageName === 'react'
);
if (
reactProd.prevFileSizeChange !== 0 ||
reactProd.prevGzipSizeChange !== 0
) {
const changeSize = emojiPercent(reactProd.prevFileSizeChange);
const changeGzip = emojiPercent(reactProd.prevGzipSizeChange);
markdown(`React: size: ${changeSize}, gzip: ${changeGzip}`);
}
}

if (packagesToShow.includes('react-dom')) {
const reactDOMProd = results.find(
r => r.bundleType === 'UMD_PROD' && r.packageName === 'react-dom'
);
if (
reactDOMProd.prevFileSizeChange !== 0 ||
reactDOMProd.prevGzipSizeChange !== 0
) {
const changeSize = emojiPercent(reactDOMProd.prevFileSizeChange);
const changeGzip = emojiPercent(reactDOMProd.prevGzipSizeChange);
markdown(`ReactDOM: size: ${changeSize}, gzip: ${changeGzip}`);
}
}

// Show a hidden summary table for all diffs

// eslint-disable-next-line no-var
for (var name of new Set(packagesToShow)) {
const thisBundleResults = results.filter(r => r.packageName === name);
const changedFiles = thisBundleResults.filter(
r => r.prevGzipSizeChange !== 0 || r.prevGzipSizeChange !== 0
);

const mdHeaders = [
'File',
'Filesize Diff',
'Gzip Diff',
'Prev Size',
'Current Size',
'Prev Gzip',
'Current Gzip',
'ENV',
];

const mdRows = changedFiles.map(r => [
r.filename,
emojiPercent(r.prevFileSizeChange),
emojiPercent(r.prevGzipSizeChange),
r.prevSize,
r.prevFileSize,
r.prevGzip,
r.prevGzipSize,
r.bundleType,
]);

allTables.push(`\n## ${name}`);
allTables.push(generateMDTable(mdHeaders, mdRows));
}

const summary = `
<details>
<summary>Details of bundled changes.</summary>

<p>Comparing: ${parentOfOldestCommit}...${danger.github.pr.head.sha}</p>


${allTables.join('\n')}

</details>
`;
markdown(summary);
}
});
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
"coveralls": "^2.11.6",
"create-react-class": "^15.6.2",
"cross-env": "^5.1.1",
"danger": "^3.0.4",
"del": "^2.0.2",
"derequire": "^2.0.3",
"escape-string-regexp": "^1.0.5",
Expand Down
1 change: 1 addition & 0 deletions scripts/circleci/test_entry_point.sh
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ if [ $((2 % CIRCLE_NODE_TOTAL)) -eq "$CIRCLE_NODE_INDEX" ]; then
COMMANDS_TO_RUN+=('./scripts/circleci/build.sh')
COMMANDS_TO_RUN+=('yarn test-build --maxWorkers=2')
COMMANDS_TO_RUN+=('yarn test-build-prod --maxWorkers=2')
COMMANDS_TO_RUN+=('node ./scripts/tasks/danger')
COMMANDS_TO_RUN+=('./scripts/circleci/upload_build.sh')
fi

Expand Down
2 changes: 1 addition & 1 deletion scripts/rollup/results.json
Original file line number Diff line number Diff line change
Expand Up @@ -400,4 +400,4 @@
"gzip": 525
}
]
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this file should not be checked in. It should be gitignored.

Locally, instead of comparing to it during the run, we should locate a special cache folder (like babel-loader uses for caching). Inside that folder, we should have {sha}.json for every build done locally (that hasn't been purged from the cache yet).

When we build with a clean git state, we can copy the JSON as {sha.json} into the cache. On every build (clean or not) we should also look up {merge-base-sha}.json in local cache. If it exists, that's what we compare against. Otherwise, we don't print the stats locally.

Would that make sense?

Copy link
Collaborator

Choose a reason for hiding this comment

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

(It's not a blocker though if we can get the remote results to be right at least)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, it should be gitignored 👍

I'm about to head into a meeting but I'll gonna take a look into why the results I'm seeing locally are different in general, maybe a node_modules introduced by Danger changes the results?

96 changes: 61 additions & 35 deletions scripts/rollup/stats.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,60 +21,86 @@ function saveResults() {
}

function percentChange(prev, current) {
const change = Math.floor((current - prev) / prev * 100);
return Math.floor((current - prev) / prev * 100);
}

function percentChangeString(change) {
if (change > 0) {
return chalk.red.bold(`+${change} %`);
} else if (change <= 0) {
return chalk.green.bold(change + ' %');
}
}

const resultsHeaders = [
'Bundle',
'Prev Size',
'Current Size',
'Diff',
'Prev Gzip',
'Current Gzip',
'Diff',
];

function generateResultsArray(current, prevResults) {
return current.bundleSizes
.map(result => {
const prev = prevResults.bundleSizes.filter(
res =>
res.filename === result.filename &&
res.bundleType === result.bundleType
)[0];
if (result === prev) {
// We didn't rebuild this bundle.
return;
}

const size = result.size;
const gzip = result.gzip;
let prevSize = prev ? prev.size : 0;
let prevGzip = prev ? prev.gzip : 0;

return {
filename: result.filename,
bundleType: result.bundleType,
packageName: result.packageName,
prevSize: filesize(prevSize),
prevFileSize: filesize(size),
prevFileSizeChange: percentChange(prevSize, size),
prevGzip: filesize(prevGzip),
prevGzipSize: filesize(gzip),
prevGzipSizeChange: percentChange(prevGzip, gzip),
};
// Strip any nulls
})
.filter(f => f);
}

function printResults() {
const table = new Table({
head: [
chalk.gray.yellow('Bundle'),
chalk.gray.yellow('Prev Size'),
chalk.gray.yellow('Current Size'),
chalk.gray.yellow('Diff'),
chalk.gray.yellow('Prev Gzip'),
chalk.gray.yellow('Current Gzip'),
chalk.gray.yellow('Diff'),
],
head: resultsHeaders.map(chalk.gray.yellow),
});
currentBuildResults.bundleSizes.forEach(result => {
const matches = prevBuildResults.bundleSizes.filter(
({filename, bundleType}) =>
filename === result.filename && bundleType === result.bundleType
);
if (matches.length > 1) {
throw new Error(`Ambiguous bundle size record for: ${result.filename}`);
}
const prev = matches[0];
if (result === prev) {
// We didn't rebuild this bundle.
return;
}

const size = result.size;
const gzip = result.gzip;
let prevSize = prev ? prev.size : 0;
let prevGzip = prev ? prev.gzip : 0;
const results = generateResultsArray(currentBuildResults, prevBuildResults);
results.forEach(result => {
table.push([
chalk.white.bold(`${result.filename} (${result.bundleType})`),
chalk.gray.bold(filesize(prevSize)),
chalk.white.bold(filesize(size)),
percentChange(prevSize, size),
chalk.gray.bold(filesize(prevGzip)),
chalk.white.bold(filesize(gzip)),
percentChange(prevGzip, gzip),
chalk.white.bold(`${result.filename} (${result.bundleType})`),
chalk.gray.bold(result.prevSize),
chalk.white.bold(result.prevFileSize),
percentChangeString(result.prevFileSizeChange),
chalk.gray.bold(result.prevGzip),
chalk.white.bold(result.prevGzipSize),
percentChangeString(result.prevGzipSizeChange),
]);
});

return table.toString();
}

module.exports = {
currentBuildResults,
generateResultsArray,
printResults,
saveResults,
currentBuildResults,
resultsHeaders,
};
32 changes: 32 additions & 0 deletions scripts/tasks/danger.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/**
* Copyright (c) 2013-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

'use strict';

const path = require('path');
const spawn = require('child_process').spawn;

const extension = process.platform === 'win32' ? '.cmd' : '';

// This came from React Native's circle.yml
const token = 'e622517d9f1136ea8900' + '07c6373666312cdfaa69';
spawn(path.join('node_modules', '.bin', 'danger-ci' + extension), [], {
// Allow colors to pass through
stdio: 'inherit',
env: {
...process.env,
DANGER_GITHUB_API_TOKEN: token,
},
}).on('close', function(code) {
if (code !== 0) {
console.error('Danger failed');
} else {
console.log('Danger passed');
}

process.exit(code);
});
Loading