Skip to content

Conversation

@chenjiahan
Copy link
Member

Summary

This PR simplifies the Compiler.close() method.

Use this.#instance?.close() instead of the this.#getInstance() method, because if the instance is not created, the this.#getInstance() method will attempt to create an instance twice and the error logs will be printed twice.

Related links

Checklist

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

@chenjiahan chenjiahan requested a review from hardfist as a code owner October 20, 2025 04:55
Copilot AI review requested due to automatic review settings October 20, 2025 04:55
@netlify
Copy link

netlify bot commented Oct 20, 2025

Deploy Preview for rspack canceled.

Name Link
🔨 Latest commit c0ebdfe
🔍 Latest deploy log https://app.netlify.com/projects/rspack/deploys/68f5c0dbf456d70008e50832

@github-actions github-actions bot added release: bug fix release: bug related release(mr only) team The issue/pr is created by the member of Rspack. labels Oct 20, 2025
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 simplifies the Compiler.close() method by using optional chaining to close the compiler instance rather than fetching it through #getInstance(), which prevents unnecessary instance creation and duplicate error logging when the instance doesn't exist.

Key changes:

  • Updated Compiler.close() to use this.#instance?.close() directly instead of calling this.#getInstance() followed by instance!.close()
  • Added test cases for builtin-swc-loader validation scenarios

Reviewed Changes

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

Show a summary per file
File Description
packages/rspack/src/Compiler.ts Simplified close() method to avoid unnecessary instance creation and duplicate error logs
tests/rspack-test/configCases/builtin-swc-loader/validate/raw-warning.err Removed expected error output file
tests/rspack-test/configCases/builtin-swc-loader/validate/raw-error.err Removed expected error output file
tests/rspack-test/configCases/builtin-swc-loader/invalid-unknown-option-error/rspack.config.js Added test configuration for unknown option error handling
tests/rspack-test/configCases/builtin-swc-loader/invalid-unknown-option-error/errors.js Added expected error assertions for unknown option test
tests/rspack-test/configCases/builtin-swc-loader/invalid-option-value-error/index.js Added test case for invalid option value scenarios

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@github-actions
Copy link
Contributor

📦 Binary Size-limit

Comparing c0ebdfe to test: add warn while closing compiler with failure (#11909) by harpsealjs

🙈 Size remains the same at 47.67MB

@codspeed-hq
Copy link

codspeed-hq bot commented Oct 20, 2025

CodSpeed Performance Report

Merging #11923 will not alter performance

Comparing close_compiler_1020 (c0ebdfe) with main (ceb5a7e)1

Summary

✅ 17 untouched

Footnotes

  1. No successful run was found on main (02ab3a4) during the generation of this report, so ceb5a7e was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@chenjiahan chenjiahan merged commit 58ba8a4 into main Oct 20, 2025
68 of 70 checks passed
@chenjiahan chenjiahan deleted the close_compiler_1020 branch October 20, 2025 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release: bug fix release: bug related release(mr only) team The issue/pr is created by the member of Rspack.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants