-
-
Couldn't load subscription status.
- Fork 32.7k
[ci] Use dangerJS to report bundle size changes #14587
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
Changes from all commits
Commits
Show all changes
35 commits
Select commit
Hold shift + click to select a range
945a7ab
[core] Track size via build artifacts
eps1lon 05735c0
[core] Fix size snapshot failing on ci
eps1lon d644e17
[ci] Fix size snapshot not being saved
eps1lon 65a785d
[ci] Decouple size snapshot from test pipeline
eps1lon c6aacb5
[ci] Persist snapshot in s3 bucket
eps1lon 322c77e
[ci] Add dangerfile
eps1lon 2f94161
[ci] Fix pull requests not being recognized
eps1lon e9f0293
[ci] Proper dangerJS report
eps1lon b1aebd5
[ci] Silence non existing snapshots
eps1lon fac7fca
[ci] Print commit range to CI
eps1lon 4d7ab85
[ci] Fix violation passed to danger js
eps1lon 0bd59e4
[docs] explain scripts folder structure
eps1lon 7b2eafb
[ci] Remove debug tasks
eps1lon c3c13dc
[ci] Report even with no changes
eps1lon d1b729c
[ci] Fix danger js violation if no important change
eps1lon 04d3bb6
[ci] Fix no change being considered any change
eps1lon a389123
[ci] Improve review expierence
eps1lon d8294b4
[core] Remove rollup snapshot
eps1lon ba46066
[ci] improve config docs
eps1lon d4e50c8
[ci] Fix emoji in bundle changes details
eps1lon 1a817d5
[ci] cleanup danger async runner
eps1lon e0a2ad2
[ci] reintriduce umd smoke test
eps1lon 4249451
[ci] Fix lint errors
eps1lon c97eb00
[ci] Refactor code
eps1lon a1ec9ea
[ci] Use +-inf the mark added/removed snapshots
eps1lon 1e02dfe
[ci] Fix new bundles being considered as reducing size
eps1lon 7e23fa7
[ci] Consider non-existing as zero
eps1lon dad906d
[ci] Place change emoji after percent
eps1lon 76579aa
[ci] Fix increase being displayed like reduction (triangle down)
eps1lon a24b8d3
[ci] Fix outdated code comments
eps1lon e056f26
[ci] Use upercase snakecase for constants
eps1lon 1b2579a
[core] Remove legacy scripts
eps1lon 006c7da
[ci] Improve destructuring semantics
eps1lon 5eee75f
[ci] Exit with non-zero for any error in danger
eps1lon 24a3f80
[scripts] Remove utils in favor of lodash
eps1lon File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,3 +20,4 @@ | |
| build | ||
| node_modules | ||
| package-lock.json | ||
| size-snapshot.json | ||
This file was deleted.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,190 @@ | ||
| /* eslint-disable no-console */ | ||
| // inspire by reacts dangerfile | ||
| // danger has to be the first thing required! | ||
| const { danger, markdown } = require('danger'); | ||
| const { exec } = require('child_process'); | ||
| const { loadComparison } = require('./scripts/sizeSnapshot'); | ||
|
|
||
| const parsedSizeChangeThreshold = 300; | ||
| const gzipSizeChangeThreshold = 100; | ||
|
|
||
| /** | ||
| * executes a git subcommand | ||
| * @param {any} args | ||
| */ | ||
| function git(args) { | ||
| 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 cleanup() { | ||
| 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, gzipThreshold) { | ||
| return comparisonEntry => { | ||
| const [, snapshot] = comparisonEntry; | ||
| return ( | ||
| Math.abs(snapshot.parsed.absoluteDiff) >= parsedThreshold || | ||
| Math.abs(snapshot.gzip.absoluteDiff) >= gzipThreshold | ||
| ); | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * checks if the bundle is of a package e.b. `@material-ui/core` but not | ||
| * `@material-ui/core/Paper` | ||
| * @param {[string, any]} comparisonEntry | ||
| */ | ||
| function isPackageComparison(comparisonEntry) { | ||
| const [bundleKey] = comparisonEntry; | ||
| return /^@[\w-]+\/[\w-]+$/.test(bundleKey); | ||
| } | ||
|
|
||
| /** | ||
| * Generates a user-readable string from a percentage change | ||
| * @param {number} change | ||
| * @param {string} goodEmoji emoji on reduction | ||
| * @param {string} badEmooji emoji on increase | ||
| */ | ||
| function addPercent(change, goodEmoji = '', badEmooji = ':small_red_triangle:') { | ||
| const formatted = (change * 100).toFixed(2); | ||
| if (/^-|^0(?:\.0+)$/.test(formatted)) { | ||
| return `${formatted}%${goodEmoji}`; | ||
| } | ||
| return `+${formatted}%${badEmooji}`; | ||
| } | ||
|
|
||
| /** | ||
| * Generates a Markdown table | ||
| * @param {string[]} headers | ||
| * @param {string[][]} body | ||
| * @returns {string} | ||
| */ | ||
| 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')}`; | ||
| } | ||
|
|
||
| function generateEmphasizedChange([bundle, { parsed, gzip }]) { | ||
| // 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}`; | ||
| } | ||
|
|
||
| async function run() { | ||
| // 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 mergeBaseCommit = await git(`merge-base HEAD ${UPSTREAM_REMOTE}/${upstreamRef}`); | ||
|
|
||
| const commitRange = `${mergeBaseCommit}...${danger.github.pr.head.sha}`; | ||
|
|
||
| const comparison = await loadComparison(mergeBaseCommit, upstreamRef); | ||
| const results = Object.entries(comparison.bundles); | ||
| const anyResultsChanges = results.filter(createComparisonFilter(1, 1)); | ||
|
|
||
| if (anyResultsChanges.length > 0) { | ||
| markdown('This PR introduced some changes to the bundle size.'); | ||
|
|
||
| const importantChanges = results | ||
| .filter(createComparisonFilter(parsedSizeChangeThreshold, gzipSizeChangeThreshold)) | ||
| .filter(isPackageComparison) | ||
| .map(generateEmphasizedChange); | ||
|
|
||
| // have to guard against empty strings | ||
| if (importantChanges.length > 0) { | ||
| markdown(importantChanges.join('\n')); | ||
| } | ||
|
|
||
| const detailsTable = generateMDTable( | ||
| [ | ||
| 'bundle', | ||
| 'parsed diff', | ||
| 'gzip diff', | ||
| 'prev parsed', | ||
| 'current parsed', | ||
| 'prev gzip', | ||
| 'current gzip', | ||
| ], | ||
| results.map(([bundle, { parsed, gzip }]) => { | ||
| return [ | ||
| bundle, | ||
| addPercent(parsed.relativeDiff), | ||
| addPercent(gzip.relativeDiff), | ||
| parsed.previous, | ||
| parsed.current, | ||
| gzip.previous, | ||
| gzip.current, | ||
| ]; | ||
| }), | ||
| ); | ||
|
|
||
| const details = ` | ||
| <details> | ||
| <summary>Details of bundle changes.</summary> | ||
|
|
||
| <p>Comparing: ${commitRange}</p> | ||
|
|
||
| ${detailsTable} | ||
|
|
||
| </details>`; | ||
|
|
||
| markdown(details); | ||
| } else { | ||
| // this can later be removed to reduce PR noise. It is kept for now for debug | ||
| // purposes only. DangerJS will swallow console.logs if completes successfully | ||
| markdown(`No bundle size changes comparing ${commitRange}`); | ||
| } | ||
| } | ||
|
|
||
| (async () => { | ||
| let exitCode = 0; | ||
| try { | ||
| await run(); | ||
| } catch (err) { | ||
| console.error(err); | ||
| exitCode = 1; | ||
| } | ||
|
|
||
| try { | ||
| await cleanup(); | ||
| } catch (err) { | ||
| console.error(err); | ||
| exitCode = 1; | ||
| } | ||
|
|
||
| process.exit(exitCode); | ||
| })(); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,8 +40,7 @@ | |
| "lint:fix": "eslint . --cache --fix && echo \"eslint: no lint errors\"", | ||
| "prettier": "yarn babel-node ./scripts/prettier.js", | ||
| "prettier:all": "yarn babel-node ./scripts/prettier.js write", | ||
| "size": "size-limit", | ||
| "size:overhead:why": "size-limit --why ./test/size/overhead.js", | ||
| "size:snapshot": "node scripts/createSizeSnapshot", | ||
| "size:why": "size-limit --why packages/material-ui/build/index.js", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The best command ever! We might be able to kill 1KB by removing the usage of |
||
| "start": "yarn docs:dev", | ||
| "test": "yarn lint && yarn typescript && yarn test:coverage", | ||
|
|
@@ -103,6 +102,7 @@ | |
| "cross-env": "^5.1.1", | ||
| "css-loader": "^2.0.0", | ||
| "css-mediaquery": "^0.1.2", | ||
| "danger": "^7.0.12", | ||
| "date-fns": "2.0.0-alpha.21", | ||
| "doctrine": "^3.0.0", | ||
| "downshift": "^3.0.0", | ||
|
|
@@ -140,6 +140,7 @@ | |
| "karma-sourcemap-loader": "^0.3.7", | ||
| "karma-webpack": "^3.0.0", | ||
| "lerna": "^3.4.3", | ||
| "lodash": "^4.17.11", | ||
| "lz-string": "^1.4.4", | ||
| "markdown-to-jsx": "^6.8.3", | ||
| "material-ui-pickers": "^2.0.2", | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note for self: we could have used this abstraction the API markdown generation logic.