Skip to content

Commit 72b4a92

Browse files
committed
Migrate sizebot to combined workflow
Replaces the two separate sizebot jobs (one for each channel, stable and experimental) with a single combined job that outputs size information for all bundles in a single GitHub comment. I didn't attempt to simplify the output at all, but we should. I think what I would do is remove our custom Rollup sizes plugin, and read the sizes from the filesystem instead. We would lose some information about the build configuration used to generate each artifact, but that can be inferred from the filepath. For example, the filepath `fb-www/ReactDOM-dev.classic.js` already tells us everything we need to know about the artifact. Leaving this for a follow up.
1 parent b8b4f22 commit 72b4a92

File tree

2 files changed

+72
-178
lines changed

2 files changed

+72
-178
lines changed

.circleci/config.yml

Lines changed: 26 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -170,17 +170,9 @@ jobs:
170170
- *restore_node_modules
171171
- run: yarn build-combined
172172
- persist_to_workspace:
173-
root: build2
173+
root: .
174174
paths:
175-
- facebook-www
176-
- facebook-react-native
177-
- facebook-relay
178-
- oss-stable
179-
- oss-experimental
180-
- react-native
181-
- dist
182-
- sizes-stable
183-
- sizes-experimental
175+
- build2
184176

185177
get_base_build:
186178
docker: *docker
@@ -198,33 +190,41 @@ jobs:
198190
scripts/release/download-experimental-build.js --commit=16f3572
199191
mv ./build2 ./base-build
200192
- persist_to_workspace:
201-
root: base-build
193+
root: .
202194
paths:
203-
- facebook-www
204-
- facebook-react-native
205-
- facebook-relay
206-
- oss-stable
207-
- oss-experimental
208-
- react-native
209-
- dist
210-
- sizes-stable
211-
- sizes-experimental
195+
- base-build
212196

213197
process_artifacts_combined:
214198
docker: *docker
215199
environment: *environment
216200
steps:
217201
- checkout
218202
- attach_workspace:
219-
at: build2
203+
at: .
220204
- run: yarn workspaces info | head -n -1 > workspace_info.txt
221205
- *restore_node_modules
222206
- run: echo "<< pipeline.git.revision >>" >> build2/COMMIT_SHA
207+
- persist_to_workspace:
208+
root: .
209+
paths:
210+
- build2/COMMIT_SHA
223211
# Compress build directory into a single tarball for easy download
224212
- run: tar -zcvf ./build2.tgz ./build2
225213
- store_artifacts:
226214
path: ./build2.tgz
227215

216+
sizebot:
217+
docker: *docker
218+
environment: *environment
219+
steps:
220+
- checkout
221+
- attach_workspace:
222+
at: .
223+
- run: yarn workspaces info | head -n -1 > workspace_info.txt
224+
- *restore_node_modules
225+
- run:
226+
command: node ./scripts/tasks/danger
227+
228228
build_devtools_and_process_artifacts:
229229
docker: *docker
230230
environment: *environment
@@ -291,38 +291,6 @@ jobs:
291291
process_artifacts: *process_artifacts
292292
process_artifacts_experimental: *process_artifacts
293293

294-
sizebot_stable:
295-
docker: *docker
296-
environment: *environment
297-
steps:
298-
- checkout
299-
- attach_workspace: *attach_workspace
300-
- run: yarn workspaces info | head -n -1 > workspace_info.txt
301-
- *restore_node_modules
302-
# This runs in the process_artifacts job, too, but it's faster to run
303-
# this step in both jobs instead of running the jobs sequentially
304-
- run: node ./scripts/rollup/consolidateBundleSizes.js
305-
- run:
306-
environment:
307-
RELEASE_CHANNEL: stable
308-
command: node ./scripts/tasks/danger
309-
310-
sizebot_experimental:
311-
docker: *docker
312-
environment: *environment
313-
steps:
314-
- checkout
315-
- attach_workspace: *attach_workspace
316-
- run: yarn workspaces info | head -n -1 > workspace_info.txt
317-
- *restore_node_modules
318-
# This runs in the process_artifacts job, too, but it's faster to run
319-
# this step in both jobs instead of running the jobs sequentially
320-
- run: node ./scripts/rollup/consolidateBundleSizes.js
321-
- run:
322-
environment:
323-
RELEASE_CHANNEL: experimental
324-
command: node ./scripts/tasks/danger
325-
326294
yarn_lint_build:
327295
docker: *docker
328296
environment: *environment
@@ -371,7 +339,7 @@ jobs:
371339
steps:
372340
- checkout
373341
- attach_workspace:
374-
at: build2
342+
at: .
375343
- run: yarn workspaces info | head -n -1 > workspace_info.txt
376344
- *restore_node_modules
377345
- run: yarn test --build <<parameters.args>> --ci
@@ -424,9 +392,6 @@ workflows:
424392
- process_artifacts:
425393
requires:
426394
- RELEASE_CHANNEL_stable_yarn_build
427-
- sizebot_stable:
428-
requires:
429-
- RELEASE_CHANNEL_stable_yarn_build
430395
- RELEASE_CHANNEL_stable_yarn_lint_build:
431396
requires:
432397
- RELEASE_CHANNEL_stable_yarn_build
@@ -443,9 +408,6 @@ workflows:
443408
- process_artifacts_experimental:
444409
requires:
445410
- yarn_build
446-
- sizebot_experimental:
447-
requires:
448-
- yarn_build
449411
- yarn_lint_build:
450412
requires:
451413
- yarn_build
@@ -528,6 +490,10 @@ workflows:
528490
- get_base_build:
529491
requires:
530492
- setup
493+
- sizebot:
494+
requires:
495+
- get_base_build
496+
- process_artifacts_combined
531497
fuzz_tests:
532498
triggers:
533499
- schedule:

dangerfile.js

Lines changed: 46 additions & 118 deletions
Original file line numberDiff line numberDiff line change
@@ -25,35 +25,11 @@
2525
//
2626
// `DANGER_GITHUB_API_TOKEN=[ENV_ABOVE] yarn danger pr https://github.com/facebook/react/pull/11865
2727

28-
const {markdown, danger, warn} = require('danger');
29-
const fetch = require('node-fetch');
28+
const {markdown, danger} = require('danger');
3029

3130
const {generateResultsArray} = require('./scripts/rollup/stats');
32-
const {existsSync, readFileSync} = require('fs');
33-
const {exec} = require('child_process');
34-
35-
// This must match the name of the CI job that creates the build artifacts
36-
const RELEASE_CHANNEL =
37-
process.env.RELEASE_CHANNEL === 'experimental' ? 'experimental' : 'stable';
38-
const artifactsJobName =
39-
process.env.RELEASE_CHANNEL === 'experimental'
40-
? 'process_artifacts_experimental'
41-
: 'process_artifacts';
42-
43-
if (!existsSync('./build/bundle-sizes.json')) {
44-
// This indicates the build failed previously.
45-
// In that case, there's nothing for the Dangerfile to do.
46-
// Exit early to avoid leaving a redundant (and potentially confusing) PR comment.
47-
warn(
48-
'No bundle size information found. This indicates the build ' +
49-
'job failed.'
50-
);
51-
process.exit(0);
52-
}
53-
54-
const currentBuildResults = JSON.parse(
55-
readFileSync('./build/bundle-sizes.json')
56-
);
31+
const {readFileSync, readdirSync} = require('fs');
32+
const path = require('path');
5733

5834
/**
5935
* Generates a Markdown table
@@ -100,99 +76,23 @@ function setBoldness(row, isBold) {
10076
}
10177
}
10278

103-
/**
104-
* Gets the commit that represents the merge between the current branch
105-
* and master.
106-
*/
107-
function git(args) {
108-
return new Promise(res => {
109-
exec('git ' + args, (err, stdout, stderr) => {
110-
if (err) {
111-
throw err;
112-
} else {
113-
res(stdout.trim());
114-
}
115-
});
116-
});
117-
}
118-
119-
(async function() {
120-
// Use git locally to grab the commit which represents the place
121-
// where the branches differ
122-
const upstreamRepo = danger.github.pr.base.repo.full_name;
123-
if (upstreamRepo !== 'facebook/react') {
124-
// Exit unless we're running in the main repo
125-
return;
126-
}
127-
128-
markdown(`## Size changes (${RELEASE_CHANNEL})`);
129-
130-
const upstreamRef = danger.github.pr.base.ref;
131-
await git(`remote add upstream https://github.com/facebook/react.git`);
132-
await git('fetch upstream');
133-
const baseCommit = await git(`merge-base HEAD upstream/${upstreamRef}`);
134-
135-
let previousBuildResults = null;
136-
try {
137-
let baseCIBuildId = null;
138-
const statusesResponse = await fetch(
139-
`https://api.github.com/repos/facebook/react/commits/${baseCommit}/status`
140-
);
141-
const {statuses, state} = await statusesResponse.json();
142-
if (state === 'failure') {
143-
warn(`Base commit is broken: ${baseCommit}`);
144-
return;
79+
function getBundleSizes(pathToSizesDir) {
80+
const filenames = readdirSync(pathToSizesDir);
81+
let bundleSizes = [];
82+
for (let i = 0; i < filenames.length; i++) {
83+
const filename = filenames[i];
84+
if (filename.endsWith('.json')) {
85+
const json = readFileSync(path.join(pathToSizesDir, filename));
86+
bundleSizes.push(...JSON.parse(json).bundleSizes);
14587
}
146-
for (let i = 0; i < statuses.length; i++) {
147-
const status = statuses[i];
148-
if (status.context === `ci/circleci: ${artifactsJobName}`) {
149-
if (status.state === 'success') {
150-
baseCIBuildId = /\/facebook\/react\/([0-9]+)/.exec(
151-
status.target_url
152-
)[1];
153-
break;
154-
}
155-
if (status.state === 'pending') {
156-
warn(`Build job for base commit is still pending: ${baseCommit}`);
157-
return;
158-
}
159-
}
160-
}
161-
162-
if (baseCIBuildId === null) {
163-
warn(`Could not find build artifacts for base commit: ${baseCommit}`);
164-
return;
165-
}
166-
167-
const baseArtifactsInfoResponse = await fetch(
168-
`https://circleci.com/api/v1.1/project/github/facebook/react/${baseCIBuildId}/artifacts`
169-
);
170-
const baseArtifactsInfo = await baseArtifactsInfoResponse.json();
171-
172-
for (let i = 0; i < baseArtifactsInfo.length; i++) {
173-
const info = baseArtifactsInfo[i];
174-
if (info.path.endsWith('bundle-sizes.json')) {
175-
const resultsResponse = await fetch(info.url);
176-
previousBuildResults = await resultsResponse.json();
177-
break;
178-
}
179-
}
180-
} catch (error) {
181-
warn(`Failed to fetch build artifacts for base commit: ${baseCommit}`);
182-
return;
183-
}
184-
185-
if (previousBuildResults === null) {
186-
warn(`Could not find build artifacts for base commit: ${baseCommit}`);
187-
return;
18888
}
89+
return {bundleSizes};
90+
}
18991

92+
async function printResultsForChannel(baseResults, headResults) {
19093
// Take the JSON of the build response and
19194
// make an array comparing the results for printing
192-
const results = generateResultsArray(
193-
currentBuildResults,
194-
previousBuildResults
195-
);
95+
const results = generateResultsArray(baseResults, headResults);
19696

19797
const packagesToShow = results
19898
.filter(
@@ -281,9 +181,6 @@ function git(args) {
281181
<details>
282182
<summary>Details of bundled changes.</summary>
283183
284-
<p>Comparing: ${baseCommit}...${danger.github.pr.head.sha}</p>
285-
286-
287184
${allTables.join('\n')}
288185
289186
</details>
@@ -292,4 +189,35 @@ function git(args) {
292189
} else {
293190
markdown('No significant bundle size changes to report.');
294191
}
192+
}
193+
194+
(async function() {
195+
// Use git locally to grab the commit which represents the place
196+
// where the branches differ
197+
198+
const upstreamRepo = danger.github.pr.base.repo.full_name;
199+
if (upstreamRepo !== 'facebook/react') {
200+
// Exit unless we're running in the main repo
201+
return;
202+
}
203+
204+
markdown('## Size changes');
205+
206+
const headSha = (readFileSync('./build2/COMMIT_SHA') + '').trim();
207+
const headSizesStable = getBundleSizes('./build2/sizes-stable');
208+
const headSizesExperimental = getBundleSizes('./build2/sizes-experimental');
209+
210+
const baseSha = (readFileSync('./base-build/COMMIT_SHA') + '').trim();
211+
const baseSizesStable = getBundleSizes('./base-build/sizes-stable');
212+
const baseSizesExperimental = getBundleSizes(
213+
'./base-build/sizes-experimental'
214+
);
215+
216+
markdown(`<p>Comparing: ${baseSha}...${headSha}</p>`);
217+
218+
markdown('## Stable channel');
219+
printResultsForChannel(baseSizesStable, headSizesStable);
220+
221+
markdown('## Experimental channel');
222+
printResultsForChannel(baseSizesExperimental, headSizesExperimental);
295223
})();

0 commit comments

Comments
 (0)