Skip to content

Conversation

@magic-akari
Copy link
Member

@changeset-bot
Copy link

changeset-bot bot commented Jun 25, 2025

⚠️ No Changeset found

Latest commit: 414b2f1

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 Jun 25, 2025

CodSpeed Performance Report

Merging #10697 will not alter performance

Comparing magic-akari:fix/issue-10553 (414b2f1) with dev (ce2dc75)

Summary

✅ 140 untouched benchmarks

@magic-akari
Copy link
Member Author

magic-akari commented Jun 25, 2025

Required Documentation Updates:

  1. Custom JSX Transform Plugins (e.g., vue-jsx):
    When using custom JSX transformation plugins, users must disable SWC’s built-in JSX transform by configuring runtime: "preserve".

  2. Special Note for runtime: "preserve":
    If using runtime: "preserve", enable verbatim_module_syntax: true. This is required because TypeScript’s transform cannot resolve JSX-style pragma references – Without this, JSX pragmas appearing as imports will be treated as type-only imports and incorrectly removed.

@magic-akari
Copy link
Member Author

Since we've set automatic as the default, the classic JSX transform is disabled by default. This means with minimal configuration, plugins can still access JSX AST nodes.
However, explicitly declaring preserve remains the recommended approach for clarity.

@magic-akari magic-akari marked this pull request as ready for review June 25, 2025 09:34
@magic-akari magic-akari requested review from a team as code owners June 25, 2025 09:34
@kdy1 kdy1 added this to the v2 (API cleanup) milestone Jun 25, 2025
@Austaras
Copy link
Member

Would it not be easier if we just change how we compare idents in jsx transform code?

@magic-akari
Copy link
Member Author

Would it not be easier if we just change how we compare idents in jsx transform code?

  • The classic JSX transform must be applied before the TypeScript processing pass. Otherwise, we'd need to reimplement equivalent logic within the TypeScript pass.
  • If we place the JSX transform after the resolver, the required implementation would largely duplicate the resolver's existing logic.

@Austaras
Copy link
Member

  • If we place the JSX transform after the resolver, the required implementation would largely duplicate the resolver's existing logic.

Why since if I'm reading it right you're basically abandoning name resolve here.

@magic-akari
Copy link
Member Author

  • If we place the JSX transform after the resolver, the required implementation would largely duplicate the resolver's existing logic.

Why since if I'm reading it right you're basically abandoning name resolve here.

The details have been provided in the comments.

// Exception: Classic JSX transformation runs before the resolver
// because it's a context-free syntactic transformation that doesn't require
// scope information.
// Running resolver first would mark identifiers (like JSX pragma functions)
// with syntax contexts, making it harder for the JSX transform to
// handle pragma references that should resolve to local scope declarations.

@kdy1 kdy1 merged commit 7099ed9 into swc-project:dev Jun 26, 2025
168 checks passed
kdy1 pushed a commit that referenced this pull request Jun 26, 2025
kdy1 pushed a commit that referenced this pull request Jun 26, 2025
@magic-akari magic-akari deleted the fix/issue-10553 branch June 26, 2025 01:36
@mischnic
Copy link
Contributor

Isn't this a pretty breaking API change as well? Because users of the Rust API would have to change the pass order?

@magic-akari
Copy link
Member Author

Isn't this a pretty breaking API change as well? Because users of the Rust API would have to change the pass order?

It depends on how our Rust API users interact with SWC. To my knowledge, Rspack uses the build_as_input API – our changes are internal to this function. So if Rspack upgrades to this new SWC version, it will automatically pick up the changes without issues.

I checked Turbopack’s implementation: it calls more granular functions (including react::react), so Turbopack would need adjustments on their side to adopt this.

If backward compatibility is desired, we can preserve the old behavior. The current PR breaks compatibility, but we can revert it.

@magic-akari
Copy link
Member Author

The most breaking change is that we removed a parameter from react::react— the top_level_ctxt. This parameter was used to mark JSX factory references as top-level. Now, all contexts is marked as unresolved and are handled by the resolver pass.

To preserve the old behavior, we could reintroduce this parameter. Internally, SWC would pass unresolved_ctxt, while third-party Rust users during the transition period could continue passing top_level_ctxt.

The second change is that react::react now returns (impl Pass, impl Pass) (a tuple of passes) instead of a single impl Pass. However, since a tuple of passes still satisfies the Pass trait, this introduces no observable difference for consumers—except that we can now opt to destructure the tuple if needed.


That said, I lean towards keeping this breaking change. Why? Because it forces Rust’s compiler to flag all existing react::react call sites via errors—effectively surfacing every usage that needs upgrading. This makes the migration explicit and systematic.

@kdy1
Copy link
Member

kdy1 commented Jun 26, 2025

I also think we should keep this change, although I'm thinking about simply ignoring SyntaxContext from the classic JSX pass and running it after the resolver (and Wasm plugins).

kdy1 pushed a commit that referenced this pull request Jun 26, 2025
kdy1 pushed a commit that referenced this pull request Jul 1, 2025
magic-akari added a commit to magic-akari/swc that referenced this pull request Jul 23, 2025
@swc-project swc-project locked as resolved and limited conversation to collaborators Jul 26, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants