fix: make Vite and Rsbuild build-time metrics apples-to-apples

.changeset/tall-pugs-tickle.md

@@ -0,0 +1,11 @@
+---
+'agoda-devfeedback-common': minor
+'agoda-devfeedback-vite2': minor
+'agoda-devfeedback-rsbuild': minor
+---
+
+Make the Vite and Rsbuild build-time metrics apples-to-apples.
+
+- **Vite production builds now measure the whole build.** The reported `timeTaken` was `buildEnd - buildStart`, and Rollup's `buildEnd` fires when the module graph is complete — before `renderChunk`, `generateBundle` and `writeBundle`. Minification and emitting assets were excluded: 18% of the build under esbuild, 64% under terser, on a 301-module project. It is now measured through `closeBundle`, matching what Rsbuild's `stats.endTime - stats.startTime` already covered. The transform phase is still reported, as a new `transformTimeMs` field, instead of being the headline number.
+- **Rsbuild now emits `phase: 'clientready'` as a `command` event**, timed from dev server start so it shares an origin with Vite's. It previously existed only as a `clientReady` entry inside `devFeedback[]`, timed with `performance.now()` — i.e. from page navigation, excluding everything before the browser opened the page — which was not comparable with Vite's number. The browser-relative values are unchanged and still in `devFeedback[]`, and are also attached to the new event as `domContentLoadedMs` / `firstContentfulPaintMs`.
+- **Vite's `prebundled` flag can now actually report `true`.** It compared the mtime of `node_modules/.vite/deps/_metadata.json` at `listening`, but dependency prebundling is triggered by the first browser request and has not run at that point — so it reported `false` on a warm cache and nothing on a cold one, never observing the cost it exists to measure. It is now sampled on the `clientready` report, and moves from the `devserver` event to the `clientready` event with it.

CONTRIBUTING.md

@@ -44,6 +44,28 @@ Remember, we're all about that F5 Experience here. Our goal is to make the devel
 2. Increase the version numbers in any examples files and the README.md to the new version that this Pull Request would represent. We use SemVer, because we're not animals.
 3. You may merge the Pull Request in once you have the sign-off of two other developers, or if you don't have permission to do that, you may request the second reviewer to merge it for you. No lone wolves here!
 
+## Changesets
+
+Releases are driven by [changesets](https://github.com/changesets/changesets), not by hand-edited version numbers. If your PR changes anything a consumer of a published package can observe, it needs a changeset:
+
+```bash
+pnpm changeset
+```
+
+That prompts for the affected packages, a `major`/`minor`/`patch` bump for each, and a summary, then writes a markdown file into `.changeset/`. Commit it with your PR. On merge to `master` the Changeset workflow rolls every pending changeset into a "Version Packages" PR that bumps versions and writes the CHANGELOGs; merging **that** is what publishes to npm.
+
+So a release is two merges, not one. If your change is already on `master` but not on npm, the Version Packages PR is what you are waiting for.
+
+Nothing user-visible? Skip it — CI-only, test-only and internal refactor PRs don't need a changeset.
+
+### Why is my changeset called `tall-pugs-tickle.md`?
+
+Because changesets named it that. The filename is generated by [`human-id`](https://github.com/RienNeVaPlus/human-id), which picks one word from each of three built-in lists — 200 adjectives, 300 nouns, 250 verbs — for about 15 million combinations. As the changesets source puts it:
+
+> Worth understanding that the ID merely needs to be a unique hash to avoid git conflicts — experimenting with human readable ids to make finding changesets easier
+
+The name carries no meaning and never appears anywhere user-facing; only the file's contents reach the CHANGELOGs. Its whole job is to keep two people adding changesets on the same day from colliding on a filename. Renaming it to something descriptive is harmless if you prefer — changesets reads the directory, not the names.
+
 ## Code of Conduct
 
 In the interest of fostering an open and welcoming environment, we as contributors and maintainers pledge to making participation in our project and our community a harassment-free experience for everyone, regardless of age, body size, disability, ethnicity, gender identity and expression, level of experience, nationality, personal appearance, race, religion, or sexual identity and orientation.

README.md

@@ -2,6 +2,15 @@
 
 Welcome to agoda-devfeedback, the JavaScript/TypeScript package collection that's about to make your builds faster than a caffeinated squirrel on a sugar rush! We're here to collect metrics that relate to developers' experience, because who doesn't love a good statistic about how long they've been waiting for their build to finish?
 
+## The F5 Experience: Because Waiting is So Last Year
+
+What is the F5 Experience? Have a read [here](https://beerandserversdontmix.com/2024/08/15/an-introduction-to-the-f5-experience/)
+
+Remember, we're all about that F5 Experience here at agoda-devfeedback. Our goal is to make your development process smoother than a JavaScript promise chain. Here's what that means for you:
+
+1. **Setup Should Be a Breeze**: You should be able to install these packages and get metrics faster than you can say "npm install".
+2. **Fast Feedback Loop**: We want your builds to be so fast, you'll forget what you were working on by the time they finish. (Okay, maybe not that fast, but you get the idea.)
+
 ## Build Time (Compilation Time): Because Life's Too Short for Slow Builds
 
 This collection supports collecting build time (compilation time) metrics across multiple bundlers:
@@ -189,15 +198,6 @@ Telemetry that slows people down gets deleted from configs, so:
 - The spool is capped at 256 KB and events older than a week are dropped rather than accumulated.
 - Startup chatter is behind `DEVFEEDBACK_DEBUG=1`. At the default log level the lifecycle events print nothing at all.
 
-## The F5 Experience: Because Waiting is So Last Year
-
-What is the F5 Experience? Have a read [here](https://beerandserversdontmix.com/2024/08/15/an-introduction-to-the-f5-experience/)
-
-Remember, we're all about that F5 Experience here at agoda-devfeedback. Our goal is to make your development process smoother than a JavaScript promise chain. Here's what that means for you:
-
-1. **Setup Should Be a Breeze**: You should be able to install these packages and get metrics faster than you can say "npm install".
-2. **Fast Feedback Loop**: We want your builds to be so fast, you'll forget what you were working on by the time they finish. (Okay, maybe not that fast, but you get the idea.)
-
 ## Contributing
 
 We welcome contributions! Whether you're fixing bugs, improving documentation, or adding support for the next big JavaScript build tool, we appreciate your help in making agoda-devfeedback even better. Check out our [Contributing Guide](CONTRIBUTING.md) for more details on how to get started.

packages/common/src/lib/types.ts

@@ -54,6 +54,12 @@ export interface ViteBuildData extends CommonMetadata {
   viteVersion: string | null;
   bundleStats?: ViteBundleStats;
   file: string | null;
+  /**
+   * Production builds only: the transform phase alone (`buildStart` to `buildEnd`),
+   * where `timeTaken` covers the whole build through `closeBundle`. The difference
+   * between the two is render, minify and emit.
+   */
+  transformTimeMs?: number;
 }
 
 /** A phase of the local dev cycle that is measured as a single span. */
@@ -94,7 +100,11 @@ export interface CommandBuildData extends CommonMetadata {
   npmTimers?: Record<string, number>;
 
   // dev server specific, all optional
-  /** Vite only: did this start (re)run dependency prebundling? undefined when unknown */
+  /**
+   * Vite only: did this start (re)run dependency prebundling? undefined when unknown.
+   * Reported on the `clientready` event, not `devserver`: prebundling is triggered by
+   * the first browser request, so at `listening` it has not run yet.
+   */
   prebundled?: boolean;
 
   // client-ready specific, all optional

packages/rspack-plugin/package.json

@@ -1,6 +1,6 @@
 {
   "name": "agoda-devfeedback-rsbuild",
-  "version": "2.0.9",
+  "version": "2.1.0",
   "type": "module",
   "main": "./dist/index.cjs",
   "module": "./dist/index.js",

packages/rspack-plugin/src/lib/rsbuild-stats-plugin.spec.ts

@@ -3,6 +3,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest';
 import { RsbuildBuildStatsPlugin } from './rsbuild-stats-plugin';
 import { RsbuildPluginAPI } from '@rsbuild/core';
 import { Rspack } from '@rsbuild/core';
+import { WebSocket } from 'ws';
 import {
   getCommonMetadata,
   sendBuildData,
@@ -64,6 +65,25 @@ const createMockApi = (): Partial<RsbuildPluginAPI> => {
   };
 };
 
+// The port is only known at runtime, so the only way to reach the plugin's WebSocket
+// server is through the script it injects.
+const wsPortFromScript = (api: Partial<RsbuildPluginAPI>): number => {
+  const params = { headTags: [] as any[] };
+  (api.modifyHTMLTags as any).mock.calls[0][0](params);
+  const port = /ws:\/\/' \+ location\.hostname \+ ':(\d+)/.exec(
+    params.headTags[0]?.children ?? '',
+  )?.[1];
+  if (!port) throw new Error('client script carries no WebSocket port');
+  return Number(port);
+};
+
+const connectClient = (port: number): Promise<WebSocket> =>
+  new Promise((resolve, reject) => {
+    const socket = new WebSocket(`ws://127.0.0.1:${port}`);
+    socket.on('open', () => resolve(socket));
+    socket.on('error', reject);
+  });
+
 describe('RsbuildBuildStatsPlugin', () => {
   let mockApi: Partial<RsbuildPluginAPI>;
 
@@ -172,6 +192,57 @@ describe('RsbuildBuildStatsPlugin', () => {
       });
     });
 
+    it('reports client ready from dev server start, exactly once per run', async () => {
+      await RsbuildBuildStatsPlugin.setup(mockApi as RsbuildPluginAPI);
+      await new Promise((resolve) => setTimeout(resolve, 50));
+
+      (mockApi.onBeforeStartDevServer as any).mock.calls[0][0]();
+
+      const socket = await connectClient(wsPortFromScript(mockApi));
+      const message = JSON.stringify({
+        type: 'clientReady',
+        elapsedMs: 300,
+        domContentLoaded: 250,
+        firstContentfulPaint: 280,
+      });
+      // a page reload sends the same message again; it is not a new dev server start
+      socket.send(message);
+      socket.send(message);
+      await new Promise((resolve) => setTimeout(resolve, 50));
+      socket.close();
+
+      const clientReady = vi
+        .mocked(sendCommandData)
+        .mock.calls.filter((call) => call[0]?.phase === 'clientready');
+      expect(clientReady).toHaveLength(1);
+      expect(clientReady[0]?.[0]).toMatchObject({
+        type: 'command',
+        phase: 'clientready',
+        command: 'rsbuild dev',
+        success: true,
+        domContentLoadedMs: 250,
+        firstContentfulPaintMs: 280,
+      });
+      // the browser-relative numbers stay as detail on the next compile's payload
+      const onDevCompileDone = (mockApi.onDevCompileDone as any).mock.calls[0][0];
+      await onDevCompileDone({
+        stats: createMockStats({
+          startTime: Date.now(),
+          endTime: Date.now() + 10,
+          hash: 'devhash',
+          modules: [],
+        }),
+      });
+      const sent = mockedSendBuildData.mock.calls.at(-1)?.[0] as RspackBuildData;
+      expect(sent.devFeedback).toEqual(
+        expect.arrayContaining([
+          { type: 'clientReady', elapsedMs: 300 },
+          { type: 'domContentLoaded', elapsedMs: 250 },
+          { type: 'firstContentfulPaint', elapsedMs: 280 },
+        ]),
+      );
+    });
+
     it('injects a client script carrying the runtime WebSocket port', async () => {
       await RsbuildBuildStatsPlugin.setup(mockApi as RsbuildPluginAPI);
       // the listen callback is async, so wait a tick for the port to be assigned

packages/rspack-plugin/src/lib/rsbuild-stats-plugin.ts

@@ -23,12 +23,20 @@ import { Rspack, rspack } from '@rsbuild/core';
 /** client events that arrive between compiles must not grow without bound */
 const MAX_CARRIED_EVENTS = 100;
 
+interface ClientReadyMessage {
+  type: 'clientReady';
+  elapsedMs?: number;
+  domContentLoaded?: number;
+  firstContentfulPaint?: number;
+}
+
 export const RsbuildBuildStatsPlugin: RsbuildPlugin = {
   name: 'RsbuildBuildStatsPlugin',
   async setup(api: RsbuildPluginAPI) {
     const customIdentifier = process.env.npm_lifecycle_event;
     let devFeedbackBuffer: DevFeedbackEvent[] = [];
     let devServerStart = 0;
+    let clientReadyReported = false;
     let wsPort: number | undefined;
 
     // Retrieve the Rsbuild core version from the context
@@ -68,6 +76,7 @@ export const RsbuildBuildStatsPlugin: RsbuildPlugin = {
     api.onBeforeStartDevServer(() => {
       debugLog('[RsbuildBuildStatsPlugin] Development server is starting...');
       devServerStart = Date.now();
+      clientReadyReported = false;
       // A restart before the first compile completes would otherwise silently drop
       // every client event the previous server collected, so carry them forward.
       devFeedbackBuffer = devFeedbackBuffer.slice(-MAX_CARRIED_EVENTS);
@@ -143,17 +152,20 @@ export const RsbuildBuildStatsPlugin: RsbuildPlugin = {
         (() => {
           try {
             const socket = new WebSocket('ws://' + location.hostname + ':${port}');
-            const send = (type, elapsedMs) => {
-              if (typeof elapsedMs !== 'number') return;
-              try { socket.send(JSON.stringify({ type, elapsedMs })); } catch {}
-            };
             socket.addEventListener('open', () => {
               (('requestIdleCallback' in window) ? requestIdleCallback : setTimeout)(() => {
                 const nav = performance.getEntriesByType('navigation')[0];
                 const fcp = performance.getEntriesByName('first-contentful-paint')[0];
-                send('clientReady', performance.now());
-                send('domContentLoaded', nav && nav.domContentLoadedEventEnd);
-                send('firstContentfulPaint', fcp && fcp.startTime);
+                // one message, so the server can time the span from dev server start
+                // rather than from whenever the browser happened to open the page
+                try {
+                  socket.send(JSON.stringify({
+                    type: 'clientReady',
+                    elapsedMs: performance.now(),
+                    domContentLoaded: nav && nav.domContentLoadedEventEnd,
+                    firstContentfulPaint: fcp && fcp.startTime,
+                  }));
+                } catch {}
               }, 0);
             });
           } catch {}
@@ -207,8 +219,12 @@ export const RsbuildBuildStatsPlugin: RsbuildPlugin = {
     // Handle incoming WebSocket messages
     function handleIncomingWebSocketMessage(rawMsg: string) {
       try {
-        const parsed = JSON.parse(rawMsg) as DevFeedbackEvent;
-        devFeedbackBuffer.push(parsed);
+        const parsed = JSON.parse(rawMsg) as DevFeedbackEvent | ClientReadyMessage;
+        if (parsed.type === 'clientReady') {
+          handleClientReady(parsed as ClientReadyMessage);
+          return;
+        }
+        devFeedbackBuffer.push(parsed as DevFeedbackEvent);
         debugLog(
           `[DevFeedback] Client event: ${parsed.type}, elapsedMs=${parsed.elapsedMs}`,
         );
@@ -218,6 +234,41 @@ export const RsbuildBuildStatsPlugin: RsbuildPlugin = {
       }
     }
 
+    function bufferClientEvent(type: string, elapsedMs?: number) {
+      if (typeof elapsedMs !== 'number') return;
+      devFeedbackBuffer.push({ type, elapsedMs });
+    }
+
+    /**
+     * `clientReady` used to exist only as an entry in `devFeedback[]`, timed with
+     * `performance.now()` — i.e. from page navigation, excluding everything before the
+     * browser opened the page. That is not comparable with Vite's `clientready`, which
+     * runs from dev server start. Emit a first-class command event on the same origin
+     * as Vite's, and keep the browser-relative numbers as detail.
+     */
+    function handleClientReady(msg: ClientReadyMessage) {
+      bufferClientEvent('clientReady', msg.elapsedMs);
+      bufferClientEvent('domContentLoaded', msg.domContentLoaded);
+      bufferClientEvent('firstContentfulPaint', msg.firstContentfulPaint);
+      debugLog(`[DevFeedback] Client event: clientReady, elapsedMs=${msg.elapsedMs}`);
+
+      // one report per dev server run: a page reload is not a new dev server start,
+      // and a production build has no dev server to measure from
+      if (clientReadyReported || !devServerStart) return;
+      clientReadyReported = true;
+
+      void sendCommandData({
+        ...getCommonMetadata(Date.now() - devServerStart, customIdentifier),
+        type: 'command',
+        phase: 'clientready',
+        command: 'rsbuild dev',
+        exitCode: 0,
+        success: true,
+        domContentLoadedMs: msg.domContentLoaded,
+        firstContentfulPaintMs: msg.firstContentfulPaint,
+      });
+    }
+
     // Normalize file paths
     function normalizePath(filePath: string): string {
       return path.relative(process.cwd(), path.normalize(filePath));