Skip to content

fix(cli): apply default lazy compilation value based on user config #12583

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.

Sign up for GitHub

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

Jump to bottom
Merged
stormslowly merged 2 commits into from
Dec 30, 2025
Merged

fix(cli): apply default lazy compilation value based on user config #12583

stormslowly merged 2 commits into from
Dec 30, 2025

Conversation

@stormslowly
Copy link
Contributor

@stormslowly stormslowly commented Dec 30, 2025 •
edited by chenjiahan
Loading

Summary

The compiler.options is already set to default by rspack/core. We should load the user configuration in the CLI for future inspection.

Related links

Checklist

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

Copilot AI review requested due to automatic review settings December 30, 2025 06:40
@netlify
Copy link

netlify bot commented Dec 30, 2025 •
edited
Loading

✅ Deploy Preview for rspack canceled.

Name Link
🔨 Latest commit 0758066
🔍 Latest deploy log https://app.netlify.com/projects/rspack/deploys/6953767d5ec4b200085a4ee9

@github-actions github-actions bot added the release: bug fix release: bug related release(mr only) label Dec 30, 2025
@stormslowly stormslowly force-pushed the fix/lazy_compilation_overrides_by_cor branch from fcfe403 to 1d9f372 Compare December 30, 2025 06:41
Copilot AI reviewed Dec 30, 2025
View reviewed changes
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 refactors the CLI to read user configuration before creating the compiler, allowing for proper inspection of user-provided settings. Previously, compiler.options was set to default values by rspack/core, making it difficult to distinguish between user-specified and default values.

Key changes:

  • Split createCompiler method into buildCompilerConfig and createCompiler to separate config building from compiler creation
  • Moved lazyCompilation default logic from rspack/core to rspack-cli's serve command, enabling conditional defaults based on user config
  • Changed default value of lazyCompilation from conditional to false in rspack/core

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/rspack-cli/src/cli.ts Refactored to separate config building from compiler creation for better config inspection
packages/rspack-cli/src/commands/build.ts Updated to use new two-step API: build config then create compiler
packages/rspack-cli/src/commands/serve.ts Updated to use new API and handle lazyCompilation defaults conditionally based on user config and target platform
packages/rspack/src/config/defaults.ts Simplified lazyCompilation default to always be false, moving conditional logic to CLI
tests/rspack-test/defaultsCases/default/base.js Updated test expectations to reflect new lazyCompilation default value
tests/rspack-test/defaultsCases/target_/*.js Updated test expectations for various target configurations
tests/e2e/cases/chunk/recover-error/rspack.config.js Added explicit lazyCompilation setting for test case

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

@stormslowly stormslowly force-pushed the fix/lazy_compilation_overrides_by_cor branch from 1d9f372 to 4b0147a Compare December 30, 2025 06:43
chenjiahan
chenjiahan reviewed Dec 30, 2025
View reviewed changes
@github-actions
Copy link
Contributor

github-actions bot commented Dec 30, 2025

Rsdoctor Bundle Diff Analysis

Found 5 projects in monorepo, 0 projects with changes.

📊 Quick Summary
Project Total Size Change
react-10k 5.7 MB 0
react-1k 823.4 KB 0
react-5k 2.7 MB 0
rome 984.4 KB 0
ui-components 2.1 MB 0

Generated by Rsdoctor GitHub Action

@github-actions
Copy link
Contributor

github-actions bot commented Dec 30, 2025

📦 Binary Size-limit

Comparing 0758066 to chore: bump swc_core from 52.0.0 to 54.0.0 and swc_experimental from 0.3.5 to 0.4.0 (#12582) by CPunisher

🙈 Size remains the same at 47.85MB

@codspeed-hq
Copy link

codspeed-hq bot commented Dec 30, 2025

CodSpeed Performance Report

Merging #12583 will not alter performance

Comparing fix/lazy_compilation_overrides_by_cor (0758066) with main (4a0f619)

Summary

✅ 16 untouched
⏩ 1 skipped1

Footnotes

  1. 1 benchmark was skipped, so the baseline result was used instead. If it was deleted from the codebase, click here and archive it to remove it from the performance reports. ↩

@chenjiahan chenjiahan changed the title fix(cli):should read user config from cli fix(cli): apply default lazy compilation value based on user config Dec 30, 2025
@stormslowly stormslowly merged commit f2efb67 into main Dec 30, 2025
105 of 111 checks passed
@stormslowly stormslowly deleted the fix/lazy_compilation_overrides_by_cor branch December 30, 2025 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@chenjiahan chenjiahan chenjiahan approved these changes

@hardfist hardfist Awaiting requested review from hardfist hardfist is a code owner

Assignees

No one assigned

Labels

release: bug fix release: bug related release(mr only)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@stormslowly @chenjiahan