feat: Expand bundle-analyzer options #163
Conversation
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found. Additional details and impacted files
☔ View full report in Codecov by Sentry. |
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
📢 Thoughts on this report? Let us know! |
Bundle ReportChanges will increase total bundle size by 30.42kB (0.53%) ⬆️. This is within the configured threshold ✅ Detailed changes
|
Bundle ReportChanges will increase total bundle size by 26.42kB ⬆️
|
suejung-sentry
force-pushed
the
sshin/2464
branch
from
43b71eb to
9701873
Compare
| @@ -5,6 +5,7 @@ import yargs from "yargs"; | |||
| import { hideBin } from "yargs/helpers"; | |||
| import { createAndUploadReport } from "./index.js"; | |||
| import { red, type Options } from "@codecov/bundler-plugin-core"; | |||
| import { type BundleAnalyzerOptions } from "./options"; | |||
There was a problem hiding this comment.
this file is tested by cli.test.ts (which runs via npx tsx)
| /** | ||
| * Additional directories to include in the build analysis. By default, it returns an empty list. | ||
| */ | ||
| additionalBuildDirectories?: string[]; |
There was a problem hiding this comment.
What do you think about changing the API of createAndUploadReport to accept an array? I feel like that would be easier to use than to have both buildDirectoryPath as the first argument, and then additionalBuildDirectories on top of that.
We could also make buildDirectoryPath of createAndUploadReport be able to take a glob string, so that it still is a string argument, but can be used for array-like queries.
There was a problem hiding this comment.
Makes sense! My original thought there was that the default usage in most cases would expect a single build directory. And then if people are doing something off the usual path or "fancier" could supply the additional paths optionally.
I went ahead and pulled them together to one array input to reduce the complexity.
Glob string is an interesting idea - I think I can add it on top of the array in the future if it seems users want to go in that direction
suejung-sentry
force-pushed
the
sshin/2464
branch
from
199bddb to
4825aa4
Compare
| * .then(() => console.log('Report successfully generated and uploaded.')) | ||
| * .catch((error) => console.error('Failed to generate or upload report:', error)); | ||
| */ | ||
| export const createAndUploadReport = async ( | ||
| buildDirectoryPath: string, | ||
| buildDirectoryPaths: string[], |
There was a problem hiding this comment.
What do you think about making this string | string[]?
Might be excess complexity though, I'm fine with sticking with the string[].
There was a problem hiding this comment.
Yeah, I had turned that over in my head, too... I think it would be easy to extend to allow string | string[] in the future so thought it'd be good to keep the simple case until can get some more feedback
There was a problem hiding this comment.
We'll need to update the README about the changes to createAndUploadReport and the new bundle analyzer options
There was a problem hiding this comment.
Thanks, I missed that spot!
This is the problem with code duplication in the docs :(
I'll think about that - maybe can lean more into Typedoc, which I'm generating with my annotations, or a reference to the examples in the same directory instead of spelling it out in the README.
We've also got extensive docs in https://docs.codecov.com/docs/javascript-bundle-analysis, which I can potentially write a page and point to instead.
Updated the README for now to have similar level of information to the other plugin packages in this repo.
Description
This PR handles follow-up functionality to add to the bundle-analyzer package.
Closes codecov/engineering-team#2464
i. Allow multiple paths for directory traversal to be included in the single bundle stats report
ii. Include an "ignore" pattern/list for which files to exclude
iii. Support an optional configuration option pathPattern such as [name]-[hash].[ext] instead of just the default of replacing anything "hashlike" with *. Has the limitation that the single pattern would get applies to all.
UPDATE 9/9 - also includes addressing this to better wrap the error from the report overrider (beforeReportUpload)