Skip to content

Conversation

SyMind
Copy link
Member

@SyMind SyMind commented Sep 28, 2025

Summary

Related links

Checklist

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

@github-actions github-actions bot added release: performance release: performance related release(mr only) team The issue/pr is created by the member of Rspack. labels Sep 28, 2025
Copy link

netlify bot commented Sep 28, 2025

Deploy Preview for rspack canceled.

Name Link
🔨 Latest commit 6b73fc7
🔍 Latest deploy log https://app.netlify.com/projects/rspack/deploys/68d8eecae695c400085b630f

Copy link
Contributor

github-actions bot commented Sep 28, 2025

📦 Binary Size-limit

Comparing 6b73fc7 to perf: avoid unnecessary source map creation (#11773) by CPunisher

🎉 Size decreased by 384bytes from 47.89MB to 47.89MB (⬇️0.00%)

Copy link

codspeed-hq bot commented Sep 28, 2025

CodSpeed Performance Report

Merging #11772 will not alter performance

Comparing cc-try-improve-perf (6b73fc7) with main (f72a069)

🎉 Hooray! codspeed-rust just leveled up to 2.7.2!

A heads-up, this is a breaking change and it might affect your current performance baseline a bit. But here's the exciting part - it's packed with new, cool features and promises improved result stability 🥳!
Curious about what's new? Visit our releases page to delve into all the awesome details about this new version.

Summary

✅ 17 untouched

@SyMind SyMind force-pushed the cc-try-improve-perf branch from eadb12d to 79254ee Compare September 28, 2025 03:21
@web-infra-dev web-infra-dev deleted a comment from github-actions bot Sep 28, 2025
@SyMind SyMind force-pushed the cc-try-improve-perf branch from 79254ee to c4731b5 Compare September 28, 2025 03:29
@web-infra-dev web-infra-dev deleted a comment from github-actions bot Sep 28, 2025
@SyMind SyMind force-pushed the cc-try-improve-perf branch from c4731b5 to 6b73fc7 Compare September 28, 2025 08:16
@web-infra-dev web-infra-dev deleted a comment from github-actions bot Sep 28, 2025
@SyMind SyMind changed the title perf: ExternalItemFnCtx perf: context and dependency_type in ExternalItemFnCtx Sep 28, 2025
@SyMind SyMind marked this pull request as ready for review September 28, 2025 08:20
@Copilot Copilot AI review requested due to automatic review settings September 28, 2025 08:20
Copy link
Contributor

@Copilot 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 changes the type of context and dependency_type fields in ExternalItemFnCtx struct from strings to their respective typed equivalents (Context and DependencyCategory). This improves type safety and reduces unnecessary string conversions when creating the context object.

  • Changed context field from String to Context type
  • Changed dependency_type field from String to DependencyCategory enum
  • Updated all references to use the typed values directly instead of string comparisons

Reviewed Changes

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

File Description
crates/rspack_core/src/options/externals.rs Updated ExternalItemFnCtx struct to use typed fields
crates/rspack_plugin_externals/src/plugin.rs Removed string conversions when creating ExternalItemFnCtx
crates/rspack_plugin_externals/src/http_externals_plugin.rs Updated comparisons to use DependencyCategory enum variants
crates/rspack_binding_api/src/raw_options/raw_external.rs Updated RawExternalItemFnCtxInner to match new types

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

Comment on lines +93 to +94
context: Context,
dependency_type: DependencyCategory,
Copy link
Preview

Copilot AI Sep 28, 2025

Choose a reason for hiding this comment

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

The RawExternalItemFnCtxInner struct now uses typed fields but this change may break the JavaScript/TypeScript binding layer. The raw bindings typically need to convert between Rust types and JavaScript types, and changing from strings to typed enums could require additional conversion logic in the getter methods.

Copilot uses AI. Check for mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release: performance release: performance 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.

1 participant