Skip to content

Commit ba4aafb

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 ba4aafb

File tree

2 files changed

+68
-178
lines changed

2 files changed

+68
-178
lines changed

.circleci/config.yml

Lines changed: 22 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,25 +190,17 @@ 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
@@ -225,6 +209,18 @@ jobs:
225209
- store_artifacts:
226210
path: ./build2.tgz
227211

212+
sizebot:
213+
docker: *docker
214+
environment: *environment
215+
steps:
216+
- checkout
217+
- attach_workspace:
218+
at: .
219+
- run: yarn workspaces info | head -n -1 > workspace_info.txt
220+
- *restore_node_modules
221+
- run:
222+
command: node ./scripts/tasks/danger
223+
228224
build_devtools_and_process_artifacts:
229225
docker: *docker
230226
environment: *environment
@@ -291,38 +287,6 @@ jobs:
291287
process_artifacts: *process_artifacts
292288
process_artifacts_experimental: *process_artifacts
293289

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-
326290
yarn_lint_build:
327291
docker: *docker
328292
environment: *environment
@@ -371,7 +335,7 @@ jobs:
371335
steps:
372336
- checkout
373337
- attach_workspace:
374-
at: build2
338+
at: .
375339
- run: yarn workspaces info | head -n -1 > workspace_info.txt
376340
- *restore_node_modules
377341
- run: yarn test --build <<parameters.args>> --ci
@@ -424,9 +388,6 @@ workflows:
424388
- process_artifacts:
425389
requires:
426390
- RELEASE_CHANNEL_stable_yarn_build
427-
- sizebot_stable:
428-
requires:
429-
- RELEASE_CHANNEL_stable_yarn_build
430391
- RELEASE_CHANNEL_stable_yarn_lint_build:
431392
requires:
432393
- RELEASE_CHANNEL_stable_yarn_build
@@ -443,9 +404,6 @@ workflows:
443404
- process_artifacts_experimental:
444405
requires:
445406
- yarn_build
446-
- sizebot_experimental:
447-
requires:
448-
- yarn_build
449407
- yarn_lint_build:
450408
requires:
451409
- yarn_build
@@ -528,6 +486,10 @@ workflows:
528486
- get_base_build:
529487
requires:
530488
- setup
489+
- sizebot:
490+
requires:
491+
- get_base_build
492+
- yarn_build_combined
531493
fuzz_tests:
532494
triggers:
533495
- 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)