From f2e1ec5108b4b5d0b2ded5d1ef018b2e5e209487 Mon Sep 17 00:00:00 2001 From: Olivier Tassinari Date: Sat, 4 May 2024 16:55:27 +0200 Subject: [PATCH 1/5] [docs] Link to docs on PRs --- .circleci/config.yml | 15 +++++++++++++++ dangerfile.ts | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 47b978d259..5bb00d545c 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -405,6 +405,17 @@ jobs: - run: name: pnpm test:e2e command: pnpm test:e2e + test_bundle_size_monitor: + <<: *default-job + steps: + - checkout + - install_js + - run: + name: prepare danger on PRs + command: pnpm danger ci + environment: + DANGER_COMMAND: prepareBundleSizeReport + # TODO test bundle size https://github.com/mui/base-ui/issues/201 workflows: version: 2 pipeline: @@ -437,7 +448,11 @@ workflows: <<: *default-context requires: - checkout +<<<<<<< HEAD - test_e2e: +======= + - test_bundle_size_monitor: +>>>>>>> 17292f7a ([docs] Link to docs on PRs) <<: *default-context requires: - checkout diff --git a/dangerfile.ts b/dangerfile.ts index db9c801236..eec4d61b17 100644 --- a/dangerfile.ts +++ b/dangerfile.ts @@ -212,7 +212,7 @@ function addDeployPreviewUrls() { return url; } - const netlifyPreview = `https://deploy-preview-${danger.github.pr.number}--material-ui.netlify.app/`; + const netlifyPreview = `https://deploy-preview-${danger.github.pr.number}--base-ui.netlify.app/`; const files = [...danger.git.created_files, ...danger.git.modified_files]; From a077e0054c7ccbb69494bfaa6a6feff985d5154d Mon Sep 17 00:00:00 2001 From: Olivier Tassinari Date: Sat, 4 May 2024 17:15:14 +0200 Subject: [PATCH 2/5] simpler --- dangerfile.ts | 89 +++++++++++++-------------------------------------- 1 file changed, 23 insertions(+), 66 deletions(-) diff --git a/dangerfile.ts b/dangerfile.ts index eec4d61b17..dfba1e2018 100644 --- a/dangerfile.ts +++ b/dangerfile.ts @@ -1,8 +1,6 @@ -// inspire by reacts dangerfile +// Inspired by React dangerfile // danger has to be the first thing required! import { danger, markdown } from 'danger'; -// eslint-disable-next-line no-restricted-imports -import replaceUrl from '@mui/monorepo/packages/api-docs-builder/utils/replaceUrl'; import { exec } from 'child_process'; import { loadComparison } from './scripts/sizeSnapshot'; @@ -14,7 +12,7 @@ const parsedSizeChangeThreshold = 300; const gzipSizeChangeThreshold = 100; /** - * executes a git subcommand + * Executes a git subcommand. * @param {any} args */ function git(args: any) { @@ -41,8 +39,8 @@ async function reportBundleSizeCleanup() { } /** - * creates a callback for Object.entries(comparison).filter that excludes every - * entry that does not exceed the given threshold values for parsed and gzip size + * Creates a callback for Object.entries(comparison).filter that excludes every + * entry that does not exceed the given threshold values for parsed and gzip size. * @param {number} parsedThreshold * @param {number} gzipThreshold */ @@ -57,17 +55,7 @@ function createComparisonFilter(parsedThreshold: number, gzipThreshold: number) } /** - * checks if the bundle is of a package e.b. `@mui/material` but not - * `@mui/material/Paper` - * @param {[string, any]} comparisonEntry - */ -function isPackageComparison(comparisonEntry: [string, any]) { - const [bundleKey] = comparisonEntry; - return /^@[\w-]+\/[\w-]+$/.test(bundleKey); -} - -/** - * Generates a user-readable string from a percentage change + * Generates a user-readable string from a percentage change. * @param {number} change * @param {string} goodEmoji emoji on reduction * @param {string} badEmoji emoji on increase @@ -92,7 +80,7 @@ function generateEmphasizedChange([bundle, { parsed, gzip }]: [ } /** - * Puts results in different buckets wh + * Puts results in different buckets. * @param {*} results */ function sieveResults(results: Array<[string, T]>) { @@ -138,8 +126,7 @@ async function loadLastComparison( } async function reportBundleSize() { - // Use git locally to grab the commit which represents the place - // where the branches differ + // Use git locally to grab the commit which represents the place where the branches differ const upstreamRepo = danger.github.pr.base.repo.full_name; const upstreamRef = danger.github.pr.base.ref; try { @@ -162,12 +149,25 @@ async function reportBundleSize() { if (anyResultsChanges.length > 0) { const importantChanges = mainResults .filter(createComparisonFilter(parsedSizeChangeThreshold, gzipSizeChangeThreshold)) - .filter(isPackageComparison) + .sort(([, a], [, b]) => { + const aDiff = Math.abs(a.parsed.absoluteDiff) + Math.abs(a.gzip.absoluteDiff); + const bDiff = Math.abs(b.parsed.absoluteDiff) + Math.abs(b.gzip.absoluteDiff); + return bDiff - aDiff; + }) .map(generateEmphasizedChange); // have to guard against empty strings if (importantChanges.length > 0) { - markdown(importantChanges.join('\n')); + const maxVisible = 20; + + const lines = importantChanges.slice(0, maxVisible); + + const nrOfHiddenChanges = Math.max(0, importantChanges.length - maxVisible); + if (nrOfHiddenChanges > 0) { + lines.push(`and [${nrOfHiddenChanges} more changes](${detailedComparisonToolpadUrl})`); + } + + markdown(lines.join('\n')); } const details = `## Bundle size report @@ -185,55 +185,12 @@ async function reportBundleSize() { } function addDeployPreviewUrls() { - /** - * The incoming path from danger does not start with `/` - * e.g. ['docs/data/joy/components/button/button.md'] - */ - function formatFileToLink(path: string) { - let url = path.replace('docs/data', '').replace(/\.md$/, ''); - - const fragments = url.split('/').reverse(); - if (fragments[0] === fragments[1]) { - // check if the end of pathname is the same as the one before - // for example `/data/material/getting-started/overview/overview.md - url = fragments.slice(1).reverse().join('/'); - } - - if (url.startsWith('/material')) { - // needs to convert to correct material legacy folder structure to the existing url. - url = replaceUrl(url.replace('/material', ''), '/material-ui').replace(/^\//, ''); - } else { - url = url - .replace(/^\//, '') // remove initial `/` - .replace('joy/', 'joy-ui/') - .replace('components/', 'react-'); - } - - return url; - } - const netlifyPreview = `https://deploy-preview-${danger.github.pr.number}--base-ui.netlify.app/`; - const files = [...danger.git.created_files, ...danger.git.modified_files]; - - // limit to the first 5 docs - const docs = files - .filter((file) => file.startsWith('docs/data') && file.endsWith('.md')) - .slice(0, 5); - markdown(` ## Netlify deploy preview -${ - docs.length - ? docs - .map((path) => { - const formattedUrl = formatFileToLink(path); - return `- [${path}](${netlifyPreview}${formattedUrl})`; - }) - .join('\n') - : netlifyPreview -} +${netlifyPreview} `); } From d9b6aa78934b6aab4a0a6a1f8141d3d860549edf Mon Sep 17 00:00:00 2001 From: Olivier Tassinari Date: Sat, 4 May 2024 17:22:54 +0200 Subject: [PATCH 3/5] remove code for now --- dangerfile.ts | 197 -------------------------------------------------- 1 file changed, 197 deletions(-) diff --git a/dangerfile.ts b/dangerfile.ts index dfba1e2018..66ca44fbbf 100644 --- a/dangerfile.ts +++ b/dangerfile.ts @@ -1,188 +1,6 @@ // Inspired by React dangerfile // danger has to be the first thing required! import { danger, markdown } from 'danger'; -import { exec } from 'child_process'; -import { loadComparison } from './scripts/sizeSnapshot'; - -const circleCIBuildNumber = process.env.CIRCLE_BUILD_NUM; -const circleCIBuildUrl = `https://app.circleci.com/pipelines/github/mui/base-ui/jobs/${circleCIBuildNumber}`; -const dangerCommand = process.env.DANGER_COMMAND; - -const parsedSizeChangeThreshold = 300; -const gzipSizeChangeThreshold = 100; - -/** - * Executes a git subcommand. - * @param {any} args - */ -function git(args: any) { - return new Promise((resolve, reject) => { - exec(`git ${args}`, (err, stdout) => { - if (err) { - reject(err); - } else { - resolve(stdout.trim()); - } - }); - }); -} - -const UPSTREAM_REMOTE = 'danger-upstream'; - -/** - * This is mainly used for local development. It should be executed before the - * scripts exit to avoid adding internal remotes to the local machine. This is - * not an issue in CI. - */ -async function reportBundleSizeCleanup() { - await git(`remote remove ${UPSTREAM_REMOTE}`); -} - -/** - * Creates a callback for Object.entries(comparison).filter that excludes every - * entry that does not exceed the given threshold values for parsed and gzip size. - * @param {number} parsedThreshold - * @param {number} gzipThreshold - */ -function createComparisonFilter(parsedThreshold: number, gzipThreshold: number) { - return (comparisonEntry: any) => { - const [, snapshot] = comparisonEntry; - return ( - Math.abs(snapshot.parsed.absoluteDiff) >= parsedThreshold || - Math.abs(snapshot.gzip.absoluteDiff) >= gzipThreshold - ); - }; -} - -/** - * Generates a user-readable string from a percentage change. - * @param {number} change - * @param {string} goodEmoji emoji on reduction - * @param {string} badEmoji emoji on increase - */ -function addPercent(change: number, goodEmoji = '', badEmoji = ':small_red_triangle:') { - const formatted = (change * 100).toFixed(2); - if (/^-|^0(?:\.0+)$/.test(formatted)) { - return `${formatted}% ${goodEmoji}`; - } - return `+${formatted}% ${badEmoji}`; -} - -function generateEmphasizedChange([bundle, { parsed, gzip }]: [ - string, - { parsed: { relativeDiff: number }; gzip: { relativeDiff: number } }, -]) { - // increase might be a bug fix which is a nice thing. reductions are always nice - const changeParsed = addPercent(parsed.relativeDiff, ':heart_eyes:', ''); - const changeGzip = addPercent(gzip.relativeDiff, ':heart_eyes:', ''); - - return `**${bundle}**: parsed: ${changeParsed}, gzip: ${changeGzip}`; -} - -/** - * Puts results in different buckets. - * @param {*} results - */ -function sieveResults(results: Array<[string, T]>) { - const main: [string, T][] = []; - const pages: [string, T][] = []; - - results.forEach((entry) => { - const [bundleId] = entry; - - if (bundleId.startsWith('docs:')) { - pages.push(entry); - } else { - main.push(entry); - } - }); - - return { all: results, main, pages }; -} - -function prepareBundleSizeReport() { - markdown( - `## Bundle size report - -Bundle size will be reported once [CircleCI build #${circleCIBuildNumber}](${circleCIBuildUrl}) finishes.`, - ); -} - -// A previous build might have failed to produce a snapshot -// Let's walk up the tree a bit until we find a commit that has a successful snapshot -async function loadLastComparison( - upstreamRef: any, - n = 0, -): Promise>> { - const mergeBaseCommit = await git(`merge-base HEAD~${n} ${UPSTREAM_REMOTE}/${upstreamRef}`); - try { - return await loadComparison(mergeBaseCommit, upstreamRef); - } catch (err) { - if (n >= 5) { - throw err; - } - return loadLastComparison(upstreamRef, n + 1); - } -} - -async function reportBundleSize() { - // Use git locally to grab the commit which represents the place where the branches differ - const upstreamRepo = danger.github.pr.base.repo.full_name; - const upstreamRef = danger.github.pr.base.ref; - try { - await git(`remote add ${UPSTREAM_REMOTE} https://github.com/${upstreamRepo}.git`); - } catch (err) { - // ignore if it already exist for local testing - } - await git(`fetch ${UPSTREAM_REMOTE}`); - - const comparison = await loadLastComparison(upstreamRef); - - const detailedComparisonQuery = `circleCIBuildNumber=${circleCIBuildNumber}&baseRef=${danger.github.pr.base.ref}&baseCommit=${comparison.previous}&prNumber=${danger.github.pr.number}`; - const detailedComparisonToolpadUrl = `https://tools-public.onrender.com/prod/pages/h71gdad?${detailedComparisonQuery}`; - const detailedComparisonRoute = `/size-comparison?${detailedComparisonQuery}`; - const detailedComparisonUrl = `https://mui-dashboard.netlify.app${detailedComparisonRoute}`; - - const { all: allResults, main: mainResults } = sieveResults(Object.entries(comparison.bundles)); - const anyResultsChanges = allResults.filter(createComparisonFilter(1, 1)); - - if (anyResultsChanges.length > 0) { - const importantChanges = mainResults - .filter(createComparisonFilter(parsedSizeChangeThreshold, gzipSizeChangeThreshold)) - .sort(([, a], [, b]) => { - const aDiff = Math.abs(a.parsed.absoluteDiff) + Math.abs(a.gzip.absoluteDiff); - const bDiff = Math.abs(b.parsed.absoluteDiff) + Math.abs(b.gzip.absoluteDiff); - return bDiff - aDiff; - }) - .map(generateEmphasizedChange); - - // have to guard against empty strings - if (importantChanges.length > 0) { - const maxVisible = 20; - - const lines = importantChanges.slice(0, maxVisible); - - const nrOfHiddenChanges = Math.max(0, importantChanges.length - maxVisible); - if (nrOfHiddenChanges > 0) { - lines.push(`and [${nrOfHiddenChanges} more changes](${detailedComparisonToolpadUrl})`); - } - - markdown(lines.join('\n')); - } - - const details = `## Bundle size report - -[Details of bundle changes (Toolpad)](${detailedComparisonToolpadUrl}) -[Details of bundle changes](${detailedComparisonUrl})`; - - markdown(details); - } else { - markdown(`## Bundle size report - -[No bundle size changes (Toolpad)](${detailedComparisonToolpadUrl}) -[No bundle size changes](${detailedComparisonUrl})`); - } -} function addDeployPreviewUrls() { const netlifyPreview = `https://deploy-preview-${danger.github.pr.number}--base-ui.netlify.app/`; @@ -196,21 +14,6 @@ ${netlifyPreview} async function run() { addDeployPreviewUrls(); - - switch (dangerCommand) { - case 'prepareBundleSizeReport': - prepareBundleSizeReport(); - break; - case 'reportBundleSize': - try { - await reportBundleSize(); - } finally { - await reportBundleSizeCleanup(); - } - break; - default: - throw new TypeError(`Unrecognized danger command '${dangerCommand}'`); - } } run().catch((error) => { From 0dc2ca4a6ea5af5566ca2d1aa5370ed0aa85ef37 Mon Sep 17 00:00:00 2001 From: Olivier Tassinari Date: Wed, 8 May 2024 22:30:23 +0200 Subject: [PATCH 4/5] Michal's review --- .circleci/config.yml | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 5bb00d545c..98e3d657ae 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -411,10 +411,8 @@ jobs: - checkout - install_js - run: - name: prepare danger on PRs - command: pnpm danger ci - environment: - DANGER_COMMAND: prepareBundleSizeReport + name: Run danger on PRs + command: pnpm danger ci --fail-on-errors # TODO test bundle size https://github.com/mui/base-ui/issues/201 workflows: version: 2 @@ -448,11 +446,11 @@ workflows: <<: *default-context requires: - checkout -<<<<<<< HEAD - test_e2e: -======= + <<: *default-context + requires: + - checkout - test_bundle_size_monitor: ->>>>>>> 17292f7a ([docs] Link to docs on PRs) <<: *default-context requires: - checkout From 7fef0d15bbbe4b0797b5423fedba4fd603fcf987 Mon Sep 17 00:00:00 2001 From: Olivier Tassinari Date: Thu, 9 May 2024 19:11:42 +0200 Subject: [PATCH 5/5] Update dangerfile.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Michał Dudak Signed-off-by: Olivier Tassinari --- dangerfile.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/dangerfile.ts b/dangerfile.ts index 66ca44fbbf..4e033e0eef 100644 --- a/dangerfile.ts +++ b/dangerfile.ts @@ -1,4 +1,3 @@ -// Inspired by React dangerfile // danger has to be the first thing required! import { danger, markdown } from 'danger';