Skip to content

Conversation

@quininer
Copy link
Contributor

Description:

This is a follow up to #11100, which will result in plugin abi break.

It currently passes the plugin tests, but I still need to do some code and configuration cleanup, and add a regression test.

plugin tests

BREAKING CHANGE:

Since we switch serialization schemes, this will result in breaking changes on the plugin abi and api.

Related issue (if exists):

@changeset-bot
Copy link

changeset-bot bot commented Oct 28, 2025

⚠️ No Changeset found

Latest commit: f735956

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@codspeed-hq
Copy link

codspeed-hq bot commented Oct 28, 2025

CodSpeed Performance Report

Merging #11198 will not alter performance

Comparing quininer:r/switch-wasm-abi (f735956) with main (6223100)

Summary

✅ 138 untouched

@quininer quininer force-pushed the r/switch-wasm-abi branch 4 times, most recently from f02f7d4 to 0f85c06 Compare October 29, 2025 09:03
@socket-security
Copy link

socket-security bot commented Oct 29, 2025

No dependency changes detected. Learn more about Socket for GitHub.

👍 No dependency changes detected in pull request

@quininer quininer force-pushed the r/switch-wasm-abi branch 2 times, most recently from d043f9f to 119000c Compare October 29, 2025 09:26
@kdy1 kdy1 added this to the Planned milestone Oct 29, 2025
@quininer quininer force-pushed the r/switch-wasm-abi branch 2 times, most recently from eda5d89 to f756aac Compare October 30, 2025 02:46
@quininer quininer force-pushed the r/switch-wasm-abi branch 3 times, most recently from bf5f8ae to b82f905 Compare October 30, 2025 08:24
@quininer quininer marked this pull request as ready for review October 30, 2025 08:25
@quininer quininer requested a review from a team as a code owner October 30, 2025 08:25
Copilot AI review requested due to automatic review settings October 30, 2025 08:25
@quininer quininer requested review from a team as code owners October 30, 2025 08:25
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 migrates the serialization layer from rkyv to cbor4ii encoding for the SWC plugin system. The changes include:

  • Replacing rkyv-impl feature flags with encoding-impl across multiple crates
  • Adding cbor4ii as a dependency for serialization/deserialization
  • Updating field encoding attributes from rkyv-specific to cbor4ii-specific implementations
  • Modifying CSS AST enum variants from tuple-style to named field syntax (e.g., Token::Dimension(v)Token::Dimension { dimension: v })
  • Removing deprecated CSS plugin transform functionality
  • Updating visitor code generation to handle both tuple and named variant syntax

Reviewed Changes

Copilot reviewed 144 out of 154 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tools/generate-code/src/generators/visitor.rs Enhanced visitor code generation to support both tuple and named field enum variants
crates/swc_xml_ast/src/*.rs Added encoding-impl attributes for Option fields using cbor4ii::core::types::Maybe
crates/swc_plugin_runner/src/*.rs Replaced __rkyv feature with encoding-impl and updated serialization logic
crates/swc_plugin_proxy/src/*.rs Migrated from rkyv to cbor4ii serialization traits and simplified memory interop
crates/swc_ecma_ast/src/*.rs Added encoding-impl attributes for TypeScript/JSX optional fields
crates/swc_css_parser/tests/**/*.{json,stderr} Updated test snapshots to reflect new Token::Dimension structure
crates/swc_plugin_macro/src/lib.rs Removed css_plugin_transform macro

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

@github-actions
Copy link
Contributor

github-actions bot commented Oct 30, 2025

Binary Sizes

File Size
swc.linux-x64-gnu.node 31M (31930760 bytes)

Commit: c6826db

@quininer
Copy link
Contributor Author

rkyv:
rkyv
cbor (with unknown):
cbor

I ran swc_plugin_backend_tests/benches/ecma_invoke.rs, and the new scheme is about 10% to 25% slower than rkyv scheme.

@kdy1 kdy1 self-assigned this Oct 30, 2025
Copilot AI review requested due to automatic review settings October 30, 2025 10:09
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

Copilot reviewed 144 out of 154 changed files in this pull request and generated no new comments.


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

Copilot AI review requested due to automatic review settings November 3, 2025 01:55
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

Copilot reviewed 45 out of 51 changed files in this pull request and generated 3 comments.


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

/// it from the old Wasm plugin (v1), serialize it back as old data (v1), and
/// deserialize it back from latest runtime (v2).
#[test]
fn bakward_compatible() {
Copy link

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

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

Corrected spelling of 'bakward' to 'backward'.

Copilot uses AI. Check for mistakes.
///
/// This is a test for Wasm plugin tests.
///
/// This test simulates serilizing from the latest runtime (v2) and deserialize
Copy link

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

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

Corrected spelling of 'serilizing' to 'serializing'.

Copilot uses AI. Check for mistakes.
name: &"Context",
found: 0,
})?;
let len = std::cmp::min(len, 4 * 1024);
Copy link

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

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

The magic number 4 * 1024 should be extracted as a named constant (e.g., const MAX_CONTEXT_ENTRIES: usize = 4096;) to improve code readability and maintainability.

Copilot uses AI. Check for mistakes.
@@ -0,0 +1,4 @@
[target.wasm32-wasip1]
Copy link
Member

Choose a reason for hiding this comment

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

Can you update the plugin template to have this?

Copy link
Member

Choose a reason for hiding this comment

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

Copilot AI review requested due to automatic review settings November 3, 2025 03:49
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

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


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

Copilot AI review requested due to automatic review settings November 3, 2025 06:17
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

Copilot reviewed 48 out of 54 changed files in this pull request and generated 2 comments.


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

Copilot AI review requested due to automatic review settings November 3, 2025 06:22
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

Copilot reviewed 48 out of 54 changed files in this pull request and generated 3 comments.


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

@kdy1
Copy link
Member

kdy1 commented Nov 3, 2025

Can you reflect some change requests about panic from @Copilot and fix the CI?

@quininer
Copy link
Contributor Author

quininer commented Nov 3, 2025

Copilot provides an illusion-based patch, which we need to revert.

@kdy1
Copy link
Member

kdy1 commented Nov 3, 2025

Thank you so much for the hard work on the idea and implementing it!
I'll think about the way to merge this PR cleanly

@kdy1 kdy1 merged commit e5feaf1 into swc-project:main Nov 3, 2025
318 of 369 checks passed
@quininer quininer deleted the r/switch-wasm-abi branch November 3, 2025 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants