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.
facebook · Feb 3, 2021 · a85bff4 · a85bff4
1 parent f5d6d1c
commit a85bff4
Show file tree

Hide file tree

Showing 2 changed files with 89 additions and 179 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -170,17 +170,9 @@ jobs:
       - *restore_node_modules
       - run: yarn build-combined
       - persist_to_workspace:
-          root: build2
+          root: .
           paths:
-            - facebook-www
-            - facebook-react-native
-            - facebook-relay
-            - oss-stable
-            - oss-experimental
-            - react-native
-            - dist
-            - sizes-stable
-            - sizes-experimental
+            - build2
 
   get_base_build:
     docker: *docker
@@ -200,25 +192,17 @@ jobs:
               scripts/release/download-experimental-build.js --commit=6b8d4ca
               mv ./build2 ./base-build
       - persist_to_workspace:
-          root: base-build
+          root: .
           paths:
-            - facebook-www
-            - facebook-react-native
-            - facebook-relay
-            - oss-stable
-            - oss-experimental
-            - react-native
-            - dist
-            - sizes-stable
-            - sizes-experimental
+            - base-build
 
   process_artifacts_combined:
     docker: *docker
     environment: *environment
     steps:
       - checkout
       - attach_workspace:
-          at: build2
+          at: .
       - run: yarn workspaces info | head -n -1 > workspace_info.txt
       - *restore_node_modules
       - run: echo "<< pipeline.git.revision	>>" >> build2/COMMIT_SHA
@@ -227,6 +211,19 @@ jobs:
       - store_artifacts:
           path: ./build2.tgz
 
+  sizebot:
+    docker: *docker
+    environment: *environment
+    steps:
+      - checkout
+      - attach_workspace:
+          at: .
+      - run: echo "<< pipeline.git.revision	>>" >> build2/COMMIT_SHA
+      - run: yarn workspaces info | head -n -1 > workspace_info.txt
+      - *restore_node_modules
+      - run:
+          command: node ./scripts/tasks/danger
+
   build_devtools_and_process_artifacts:
     docker: *docker
     environment: *environment
@@ -293,38 +290,6 @@ jobs:
   process_artifacts: *process_artifacts
   process_artifacts_experimental: *process_artifacts
 
-  sizebot_stable:
-    docker: *docker
-    environment: *environment
-    steps:
-      - checkout
-      - attach_workspace: *attach_workspace
-      - run: yarn workspaces info | head -n -1 > workspace_info.txt
-      - *restore_node_modules
-      # This runs in the process_artifacts job, too, but it's faster to run
-      # this step in both jobs instead of running the jobs sequentially
-      - run: node ./scripts/rollup/consolidateBundleSizes.js
-      - run:
-          environment:
-            RELEASE_CHANNEL: stable
-          command: node ./scripts/tasks/danger
-
-  sizebot_experimental:
-    docker: *docker
-    environment: *environment
-    steps:
-      - checkout
-      - attach_workspace: *attach_workspace
-      - run: yarn workspaces info | head -n -1 > workspace_info.txt
-      - *restore_node_modules
-      # This runs in the process_artifacts job, too, but it's faster to run
-      # this step in both jobs instead of running the jobs sequentially
-      - run: node ./scripts/rollup/consolidateBundleSizes.js
-      - run:
-          environment:
-            RELEASE_CHANNEL: experimental
-          command: node ./scripts/tasks/danger
-
   yarn_lint_build:
     docker: *docker
     environment: *environment
@@ -373,7 +338,7 @@ jobs:
     steps:
       - checkout
       - attach_workspace:
-          at: build2
+          at: .
       - run: yarn workspaces info | head -n -1 > workspace_info.txt
       - *restore_node_modules
       - run: yarn test --build <<parameters.args>> --ci
@@ -426,9 +391,6 @@ workflows:
       - process_artifacts:
           requires:
             - RELEASE_CHANNEL_stable_yarn_build
-      - sizebot_stable:
-          requires:
-            - RELEASE_CHANNEL_stable_yarn_build
       - RELEASE_CHANNEL_stable_yarn_lint_build:
           requires:
             - RELEASE_CHANNEL_stable_yarn_build
@@ -445,9 +407,6 @@ workflows:
       - process_artifacts_experimental:
           requires:
             - yarn_build
-      - sizebot_experimental:
-          requires:
-            - yarn_build
       - yarn_lint_build:
           requires:
             - yarn_build
@@ -530,6 +489,10 @@ workflows:
       - get_base_build:
           requires:
             - setup
+      - sizebot:
+          requires:
+            - get_base_build
+            - yarn_build_combined
   fuzz_tests:
     triggers:
       - schedule:

diff --git a/dangerfile.js b/dangerfile.js
@@ -26,34 +26,10 @@
 // `DANGER_GITHUB_API_TOKEN=[ENV_ABOVE] yarn danger pr https://github.com/facebook/react/pull/11865
 
 const {markdown, danger, warn} = require('danger');
-const fetch = require('node-fetch');
 
 const {generateResultsArray} = require('./scripts/rollup/stats');
-const {existsSync, readFileSync} = require('fs');
-const {exec} = require('child_process');
-
-// This must match the name of the CI job that creates the build artifacts
-const RELEASE_CHANNEL =
-  process.env.RELEASE_CHANNEL === 'experimental' ? 'experimental' : 'stable';
-const artifactsJobName =
-  process.env.RELEASE_CHANNEL === 'experimental'
-    ? 'process_artifacts_experimental'
-    : 'process_artifacts';
-
-if (!existsSync('./build/bundle-sizes.json')) {
-  // This indicates the build failed previously.
-  // In that case, there's nothing for the Dangerfile to do.
-  // Exit early to avoid leaving a redundant (and potentially confusing) PR comment.
-  warn(
-    'No bundle size information found. This indicates the build ' +
-      'job failed.'
-  );
-  process.exit(0);
-}
-
-const currentBuildResults = JSON.parse(
-  readFileSync('./build/bundle-sizes.json')
-);
+const {readFileSync, readdirSync} = require('fs');
+const path = require('path');
 
 /**
  * Generates a Markdown table
@@ -100,99 +76,23 @@ function setBoldness(row, isBold) {
   }
 }
 
-/**
- * Gets the commit that represents the merge between the current branch
- * and master.
- */
-function git(args) {
-  return new Promise(res => {
-    exec('git ' + args, (err, stdout, stderr) => {
-      if (err) {
-        throw err;
-      } else {
-        res(stdout.trim());
-      }
-    });
-  });
-}
-
-(async function() {
-  // Use git locally to grab the commit which represents the place
-  // where the branches differ
-  const upstreamRepo = danger.github.pr.base.repo.full_name;
-  if (upstreamRepo !== 'facebook/react') {
-    // Exit unless we're running in the main repo
-    return;
-  }
-
-  markdown(`## Size changes (${RELEASE_CHANNEL})`);
-
-  const upstreamRef = danger.github.pr.base.ref;
-  await git(`remote add upstream https://github.com/facebook/react.git`);
-  await git('fetch upstream');
-  const baseCommit = await git(`merge-base HEAD upstream/${upstreamRef}`);
-
-  let previousBuildResults = null;
-  try {
-    let baseCIBuildId = null;
-    const statusesResponse = await fetch(
-      `https://api.github.com/repos/facebook/react/commits/${baseCommit}/status`
-    );
-    const {statuses, state} = await statusesResponse.json();
-    if (state === 'failure') {
-      warn(`Base commit is broken: ${baseCommit}`);
-      return;
-    }
-    for (let i = 0; i < statuses.length; i++) {
-      const status = statuses[i];
-      if (status.context === `ci/circleci: ${artifactsJobName}`) {
-        if (status.state === 'success') {
-          baseCIBuildId = /\/facebook\/react\/([0-9]+)/.exec(
-            status.target_url
-          )[1];
-          break;
-        }
-        if (status.state === 'pending') {
-          warn(`Build job for base commit is still pending: ${baseCommit}`);
-          return;
-        }
-      }
+function getBundleSizes(pathToSizesDir) {
+  const filenames = readdirSync(pathToSizesDir);
+  let bundleSizes = [];
+  for (let i = 0; i < filenames.length; i++) {
+    const filename = filenames[i];
+    if (filename.endsWith('.json')) {
+      const json = readFileSync(path.join(pathToSizesDir, filename));
+      bundleSizes.push(...JSON.parse(json).bundleSizes);
     }
-
-    if (baseCIBuildId === null) {
-      warn(`Could not find build artifacts for base commit: ${baseCommit}`);
-      return;
-    }
-
-    const baseArtifactsInfoResponse = await fetch(
-      `https://circleci.com/api/v1.1/project/github/facebook/react/${baseCIBuildId}/artifacts`
-    );
-    const baseArtifactsInfo = await baseArtifactsInfoResponse.json();
-
-    for (let i = 0; i < baseArtifactsInfo.length; i++) {
-      const info = baseArtifactsInfo[i];
-      if (info.path.endsWith('bundle-sizes.json')) {
-        const resultsResponse = await fetch(info.url);
-        previousBuildResults = await resultsResponse.json();
-        break;
-      }
-    }
-  } catch (error) {
-    warn(`Failed to fetch build artifacts for base commit: ${baseCommit}`);
-    return;
-  }
-
-  if (previousBuildResults === null) {
-    warn(`Could not find build artifacts for base commit: ${baseCommit}`);
-    return;
   }
+  return {bundleSizes};
+}
 
+async function printResultsForChannel(baseResults, headResults) {
   // Take the JSON of the build response and
   // make an array comparing the results for printing
-  const results = generateResultsArray(
-    currentBuildResults,
-    previousBuildResults
-  );
+  const results = generateResultsArray(baseResults, headResults);
 
   const packagesToShow = results
     .filter(
@@ -281,15 +181,62 @@ function git(args) {
   <details>
   <summary>Details of bundled changes.</summary>
 
-  <p>Comparing: ${baseCommit}...${danger.github.pr.head.sha}</p>
-
-
   ${allTables.join('\n')}
 
   </details>
   `;
-    markdown(summary);
+    return summary;
   } else {
-    markdown('No significant bundle size changes to report.');
+    return 'No significant bundle size changes to report.';
   }
+}
+
+(async function() {
+  // Use git locally to grab the commit which represents the place
+  // where the branches differ
+
+  const upstreamRepo = danger.github.pr.base.repo.full_name;
+  if (upstreamRepo !== 'facebook/react') {
+    // Exit unless we're running in the main repo
+    return;
+  }
+
+  let headSha;
+  let headSizesStable;
+  let headSizesExperimental;
+
+  let baseSha;
+  let baseSizesStable;
+  let baseSizesExperimental;
+
+  try {
+    headSha = (readFileSync('./build2/COMMIT_SHA') + '').trim();
+    headSizesStable = getBundleSizes('./build2/sizes-stable');
+    headSizesExperimental = getBundleSizes('./build2/sizes-experimental');
+
+    baseSha = (readFileSync('./base-build/COMMIT_SHA') + '').trim();
+    baseSizesStable = getBundleSizes('./base-build/sizes-stable');
+    baseSizesExperimental = getBundleSizes('./base-build/sizes-experimental');
+  } catch {
+    warn(
+      "Failed to read build artifacts. It's possible a build configuration " +
+        'has changed upstream. Try pulling the latest changes from the ' +
+        'main branch.'
+    );
+    return;
+  }
+
+  markdown(`
+## Size changes
+
+<p>Comparing: ${baseSha}...${headSha}</p>
+
+### Stable channel
+
+${await printResultsForChannel(baseSizesStable, headSizesStable)}
+
+### Experimental channel
+
+${await printResultsForChannel(baseSizesExperimental, headSizesExperimental)}
+`);
 })();