Skip to content

Conversation

@yifancong
Copy link
Contributor

Summary

Related links

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).

Copilot AI review requested due to automatic review settings November 20, 2025 12:57
@netlify
Copy link

netlify bot commented Nov 20, 2025

Deploy Preview for rspack canceled.

Name Link
🔨 Latest commit 4cbf2b1
🔍 Latest deploy log https://app.netlify.com/projects/rspack/deploys/691f107de3110f0008d6a478

@yifancong yifancong force-pushed the chore/add-rsdoctor-diff-actoin branch from 90a810c to 4cbf2b1 Compare November 20, 2025 12:58
@yifancong yifancong changed the base branch from main to chore/diff-v2 November 20, 2025 12:58
Copilot finished reviewing on behalf of yifancong November 20, 2025 12:58
Copy link
Contributor

Copilot AI left a 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 adds Rsdoctor diff action functionality to the reusable build-test workflow to track and report build size changes.

  • Integrates rsdoctor-action to monitor bundle size differences
  • Clones an external demo repository for analysis
  • Configures automated reporting on pull requests

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

run: pnpm run build

- name: Report Compressed Size
uses: web-infra-dev/rsdoctor-action@main
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using @main for the action version is a security and stability risk. The action could change unexpectedly, breaking the workflow or introducing malicious code.

Pin to a specific version or commit SHA:

uses: web-infra-dev/[email protected]  # or specific commit SHA
Suggested change
uses: web-infra-dev/rsdoctor-action@main
uses: web-infra-dev/rsdoctor-action@v1.0.0

Copilot uses AI. Check for mistakes.
with:
github_token: ${{ secrets.GITHUB_TOKEN }}
file_path: "rspack-diff-minimal/dist/rsdoctor-data.json"
target_branch: "chore/diff-v2"
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The target branch "chore/diff-v2" is hardcoded, which means this workflow will always compare against the same branch regardless of the actual PR base branch. This will produce incorrect comparisons for PRs targeting different branches.

Consider making this configurable or using a dynamic value like ${{ github.base_ref }} to automatically use the PR's target branch.

Suggested change
target_branch: "chore/diff-v2"
target_branch: ${{ github.base_ref }}

Copilot uses AI. Check for mistakes.
Comment on lines 146 to 162
- name: Clone rspack-sourcemap-demo
run: git clone [email protected]:yifancong/rspack-diff-minimal.git

- name: Install dependencies
working-directory: rspack-diff-minimal
run: pnpm i

- name: Build project
working-directory: rspack-diff-minimal
run: pnpm run build

- name: Report Compressed Size
uses: web-infra-dev/rsdoctor-action@main
with:
github_token: ${{ secrets.GITHUB_TOKEN }}
file_path: "rspack-diff-minimal/dist/rsdoctor-data.json"
target_branch: "chore/diff-v2"
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These rsdoctor diff steps run unconditionally for all test matrix combinations (different Node versions and targets). This means the same rsdoctor analysis will be executed multiple times unnecessarily, wasting CI resources and potentially causing conflicts.

Add a condition to run only once, for example:

- name: Clone rspack-diff-minimal
  if: ${{ matrix.node == '20' && inputs.target == 'x86_64-unknown-linux-gnu' }}
  run: git clone https://github.com/yifancong/rspack-diff-minimal.git

Apply the same condition to all rsdoctor-related steps (lines 149-162).

Copilot uses AI. Check for mistakes.

### Rsdoctor Diff Action
- name: Clone rspack-sourcemap-demo
run: git clone [email protected]:yifancong/rspack-diff-minimal.git
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The git clone command uses SSH protocol ([email protected]:), which will fail in GitHub Actions without SSH key configuration. There's no SSH key setup in the workflow, so this step will fail with an authentication error.

Replace with HTTPS protocol:

run: git clone https://github.com/yifancong/rspack-diff-minimal.git
Suggested change
run: git clone git@github.com:yifancong/rspack-diff-minimal.git
run: git clone https://github.com/yifancong/rspack-diff-minimal.git

Copilot uses AI. Check for mistakes.
@github-actions
Copy link
Contributor

github-actions bot commented Nov 20, 2025

📦 Binary Size-limit

Comparing 5661b90 to feat: support SRI with experiments.css and CssExtractRspackPlugin (#12239) by harpsealjs

🙈 Size remains the same at 47.63MB

@codspeed-hq
Copy link

codspeed-hq bot commented Nov 20, 2025

CodSpeed Performance Report

Merging #12256 will degrade performances by 20.12%

Comparing chore/add-rsdoctor-diff-actoin (5661b90) with main (5322cc8)1

Summary

❌ 1 regression
✅ 16 untouched

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
js@record chunk group 83.3 µs 104.3 µs -20.12%

Footnotes

  1. No successful run was found on chore/diff-v2 (e488009) during the generation of this report, so main (5322cc8) was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@yifancong yifancong force-pushed the chore/add-rsdoctor-diff-actoin branch from 3850784 to 10c9699 Compare November 20, 2025 13:50
@yifancong yifancong force-pushed the chore/add-rsdoctor-diff-actoin branch from 09834f9 to 7caec74 Compare November 21, 2025 03:22
@yifancong yifancong removed the request for review from stormslowly November 21, 2025 03:23
@yifancong yifancong force-pushed the chore/add-rsdoctor-diff-actoin branch from db8515a to f8aab99 Compare November 21, 2025 04:01
@yifancong yifancong force-pushed the chore/add-rsdoctor-diff-actoin branch from 0af480d to 9719312 Compare November 21, 2025 04:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants