-
-
Notifications
You must be signed in to change notification settings - Fork 234
feat: improve build time printing #6607
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
Conversation
✅ Deploy Preview for rsbuild ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for rsbuild ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const printTime = (index: number) => { | ||
| if (startTime === null) { | ||
| return; | ||
| } | ||
|
|
||
| const { name } = context.environmentList[index]; | ||
| const time = Date.now() - startTime; | ||
| context.buildState.time[name] = time; | ||
|
|
||
| // For multi compiler, print name to distinguish different environments | ||
| const suffix = isMultiCompiler ? color.dim(` (${name})`) : ''; | ||
| logger.ready(`built in ${prettyTime(time / 1000)}${suffix}`); | ||
| }; | ||
|
|
||
| if (isMultiCompiler) { | ||
| (compiler as Rspack.MultiCompiler).compilers.forEach((item, index) => { | ||
| item.hooks.done.tap(HOOK_NAME, () => { | ||
| printTime(index); | ||
| }); |
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.
Avoid logging build time when the environment fails
The new printTime helper is invoked from both multi-compiler child hooks and the top-level done hook without checking whether the corresponding compiler finished with errors. Previously, timing information was printed only when hasErrors was false. With this change the CLI will emit logger.ready('built in …') messages even when a compilation fails, which presents the build as successful and stores a duration in context.buildState.time for environments that actually errored. This makes error runs harder to diagnose and exposes misleading telemetry. Consider checking stats.hasErrors() (available in the child done hook and from statsInstance in the single-compiler path) before logging or storing the time.
Useful? React with 👍 / 👎.
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 will improve the time logs when having build errors.
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.
Pull Request Overview
This PR implements per-environment build time tracking by storing timing data in the internal context object and outputting build times immediately after each environment completes, rather than waiting for all environments to finish building.
Key Changes:
- Adds
time: Record<string, number>field toBuildStateto track build duration for each environment - Replaces stats-based timing with direct
Date.now()measurement for more accurate control - Outputs build times immediately when each environment's compilation completes in multi-compiler scenarios
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
packages/core/src/types/context.ts |
Adds time field to BuildState type to store per-environment build durations |
packages/core/src/types/rsbuild.ts |
Removes time from RsbuildStatsItem as timing is now tracked separately |
packages/core/src/createContext.ts |
Initializes the time object as an empty record in build state |
packages/core/src/provider/createCompiler.ts |
Implements timing logic using Date.now() and prints times immediately per environment |
packages/core/src/helpers/stats.ts |
Removes timings: true from stats options as time is now tracked independently |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>

Summary
Checklist