-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
feat(experimental): add printImportBreakdown.limit (fix: trim importDurations data)
#9401
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
base: main
Are you sure you want to change the base?
Conversation
…ollection with reasonable size
printImportBreakdown.limit and trim importDurations dataprintImportBreakdown.limit + fix: trim importDurations data
printImportBreakdown.limit + fix: trim importDurations dataprintImportBreakdown.limit (fix: trim importDurations data)
| for (const [filepath, { duration, selfTime, external, importer }] of entries) { | ||
| // Trim to top N by duration to reduce IPC payload | ||
| // Keep enough entries for UI "show more" and aggregation | ||
| const retention = Math.max(50, limit * 5) |
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.
I don't think this makes sense, to be honest. If there is a limit, then there is a limit. Maybe remove "show more" in UI
| } | ||
| resolved.experimental.printImportBreakdown ??= {} as any | ||
| resolved.experimental.printImportBreakdown.enabled ??= false | ||
| resolved.experimental.printImportBreakdown.limit ??= 10 |
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.
Shouldn't it be 0 by default then
I think we can just say "enable this option to see import breakdown" in the current popup
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.
Right, that works too. It looks like I was just over-complicating things.
Description
(Largely 🤖 Generated with Claude Code)
Addresses the data size concern from #9141 and #9262. The
importDurationsmetadata scales byO(test files × source files)but reporting only shows top N imports.Summary
Add
experimental.printImportBreakdown: { enabled?: boolean, limit?: number }configenabled(default:false): Enable import breakdown display in CLIlimit(default:10): Number of imports to show in CLI output.importDurationslimit = 0: Opt-out fromimportDurationscollection entirelyTrim
importDurationsat worker level ingetImportDurations():Math.max(50, limit * 5)entries per test file so that UI breakdown can still do "show 10 + more".TBD?
importDurationscollection available to be 50, which affects existing usage outside of base reporter. I think collecting all imports by default is largely unnecessary, so I think it's fine default.collectModuleDurationsDiagnostic()accumulates times across test files. Trimmed long-tail modules may show incomplete totals. To preserve the previous behavior, extension can overwrite limit to Infinity?Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
pnpm-lock.yamlunless you introduce a new test example.Tests
pnpm test:ci.Documentation
pnpm run docscommand.Changesets
feat:,fix:,perf:,docs:, orchore:.