Skip to content

Conversation

@kdy1
Copy link
Member

@kdy1 kdy1 commented Feb 9, 2026

Summary

This PR fixes the issue where the @swc/plugin-loadable-components plugin would not transform loadable() calls when imported from vendored or aliased packages.

Changes

  • Made the from field in the Signature struct optional (Option<Wtf8Atom>)
  • Updated visit_mut_import_decl to skip source checking when from is None
  • Added test cases for vendored/aliased loadable imports
  • Updated documentation with usage examples

Usage

To enable aliased imports, configure with a signature that omits the from field:

["loadable-components", { "signatures": [
    {
        "name": "loadable"
    }
]}]

This matches the behavior of the official Babel plugin as documented in the Loadable documentation.

Fixes #572


Generated with Claude Code) | View branch | [View job run](https://github.com/swc-project/plugins/actions/runs/21084641572

Make the 'from' field in Signature optional to allow matching any
import with a specific name regardless of source package. This enables
support for vendored or aliased loadable implementations.

- Made Signature.from an Option<Wtf8Atom>
- Updated visit_mut_import_decl to skip source check when from is None
- Added test case for vendored loadable imports
- Updated documentation with usage examples

Fixes #572

Co-authored-by: Donny/강동윤 <kdy1@users.noreply.github.com>
@changeset-bot
Copy link

changeset-bot bot commented Feb 9, 2026

⚠️ No Changeset found

Latest commit: 05fc137

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.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

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

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@kdy1 kdy1 self-assigned this Feb 9, 2026
@claude
Copy link

claude bot commented Feb 9, 2026

Code Review

Summary

This PR successfully addresses issue #572 by adding support for aliased/vendored imports. The implementation allows the plugin to transform calls regardless of their import source when configured with a signature that omits the from field.

Code Quality ✅

Strengths:

  • Clean implementation with minimal changes to existing logic
  • The use of Option<Wtf8Atom> for the from field is idiomatic Rust and maintains backward compatibility
  • The #[serde(default)] attribute ensures existing configurations continue to work
  • Code follows the existing patterns in the codebase
  • Default implementations properly wrap values in Some()

Observations:

  1. Logic flow in visit_mut_import_decl (src/lib.rs:636-669): The restructured logic is more readable. The early continue when from doesn't match is cleaner than the previous nested structure.

  2. Consistent updates: All three Signature constructors (default, default_lazy, and test fixtures) were properly updated to use Some().

Testing ✅

Strengths:

  • New test case fixture_aliased_import properly validates the feature
  • Unit test should_support_signatures_without_from validates config parsing
  • Test fixtures include both input and expected output
  • The output correctly matches the expected transformation format

Suggestions:

  1. Additional test case: Consider adding a test that combines both aliased (with from: None) and specific (with from: Some(...)) signatures in the same configuration to ensure they can coexist:
vec![
    Signature { name: "default".into(), from: None },
    Signature { name: "lazy".into(), from: Some("@loadable/component".into()) }
]
  1. Negative test case: Consider a test that verifies functions named loadable that are NOT imported are still ignored (local function definitions).

Security Considerations ✅

No security concerns identified:

  • No unsafe code introduced
  • No external input processing changes
  • The change only affects pattern matching logic for import statements
  • Transformation output remains the same format

Performance Considerations ✅

Analysis:

  • The change adds a single if let Some(ref from) check per signature per import declaration
  • This is negligible overhead and only occurs during compilation
  • The early continue actually improves performance when sources don't match
  • No allocations or cloning added beyond what existed

Potential Issues & Edge Cases

  1. Overly broad matching (Minor concern): When from: None is configured, ANY import named loadable will be transformed, including potential false positives from unrelated packages. This is by design per the Babel plugin behavior, but users should be aware.

    • Mitigation: Documentation clearly explains this behavior ✅
  2. Namespace imports: The code handles ImportSpecifier::Default and ImportSpecifier::Named, but ImportSpecifier::Namespace is explicitly ignored with the _ => () pattern. This seems intentional but worth noting.

    • Example that won't be transformed: import * as loadable from 'pkg'
  3. Early return in named specifier handling (src/lib.rs:659): There's a return statement when imported.sym == signature.name. This exits the entire import declaration processing, which means if multiple specifiers match different signatures, only the first will be registered. However, this appears to be existing behavior, not introduced by this PR.

Documentation ✅

Strengths:

  • README updates clearly explain both use cases (with and without from)
  • Examples are clear and show the practical difference
  • Documentation correctly notes that omitting from matches any import source

Minor suggestion:
Consider adding a note about precedence/specificity if both aliased and specific signatures might apply to the same import.

Best Practices ✅

  • Backward compatibility maintained
  • Serde defaults properly configured
  • Test coverage added
  • Documentation updated
  • Follows Rust idioms
  • Changes are focused and minimal

Recommendation: APPROVE

This is a well-implemented feature that:

The implementation is production-ready. The minor suggestions above are optional enhancements, not blockers.

@claude
Copy link

claude bot commented Feb 9, 2026

Code Review

Summary

This PR successfully addresses issue #572 by adding support for aliased/vendored loadable imports. The implementation allows the plugin to transform loadable() calls regardless of their import source when configured with a signature that omits the from field.

Code Quality ✅

Strengths:

  • Clean implementation with minimal changes to existing logic
  • The use of Option for the from field is idiomatic Rust and maintains backward compatibility
  • The #[serde(default)] attribute ensures existing configurations continue to work
  • Code follows the existing patterns in the codebase
  • Default implementations properly wrap values in Some()

Observations:

  1. Logic flow in visit_mut_import_decl (src/lib.rs:636-669): The restructured logic is more readable. The early continue when from doesn't match is cleaner than the previous nested structure.

  2. Consistent updates: All three Signature constructors (default, default_lazy, and test fixtures) were properly updated to use Some().

Testing ✅

Strengths:

  • New test case fixture_aliased_import properly validates the feature
  • Unit test should_support_signatures_without_from validates config parsing
  • Test fixtures include both input and expected output
  • The output correctly matches the expected transformation format

Suggestions for additional test coverage:

  1. Mixed signatures: Consider adding a test that combines both aliased (with from: None) and specific (with from: Some(...)) signatures in the same configuration to ensure they can coexist properly.

  2. Negative test case: Consider a test that verifies functions named loadable that are NOT imported are still ignored (local function definitions).

Security Considerations ✅

No security concerns identified:

  • No unsafe code introduced
  • No external input processing changes
  • The change only affects pattern matching logic for import statements
  • Transformation output remains the same format

Performance Considerations ✅

Analysis:

  • The change adds a single if let Some(ref from) check per signature per import declaration
  • This is negligible overhead and only occurs during compilation
  • The early continue actually improves performance when sources don't match
  • No allocations or cloning added beyond what existed

Potential Issues & Edge Cases

  1. Overly broad matching (Minor concern): When from: None is configured, ANY import named loadable will be transformed, including potential false positives from unrelated packages. This is by design per the Babel plugin behavior, but users should be aware.

    • Mitigation: Documentation clearly explains this behavior ✅
  2. Namespace imports: The code handles ImportSpecifier::Default and ImportSpecifier::Named, but ImportSpecifier::Namespace is explicitly ignored with the _ => () pattern. This seems intentional but worth noting.

    • Example that won't be transformed: import * as loadable from 'pkg'
  3. Early return in named specifier handling (src/lib.rs:659): There's a return statement when imported.sym == signature.name. This exits the entire import declaration processing, which means if multiple specifiers match different signatures, only the first will be registered. However, this appears to be existing behavior, not introduced by this PR.

Documentation ✅

Strengths:

  • README updates clearly explain both use cases (with and without from)
  • Examples are clear and show the practical difference
  • Documentation correctly notes that omitting from matches any import source

Minor suggestion:
Consider adding a note about precedence/specificity if both aliased and specific signatures might apply to the same import.

Best Practices ✅

  • Backward compatibility maintained
  • Serde defaults properly configured
  • Test coverage added
  • Documentation updated
  • Follows Rust idioms
  • Changes are focused and minimal

Recommendation: APPROVE

This is a well-implemented feature that:

The implementation is production-ready. The minor suggestions above are optional enhancements, not blockers.

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.

@swc/plugin-loadable-components doesn't support aliased loadable imports

2 participants