Skip to content

Conversation

@lightninglu10
Copy link
Contributor

@lightninglu10 lightninglu10 commented Dec 3, 2025

Skip wrapping for slot/polymorphic component patterns (asChild/forwardedAs)

This PR updates the SWC plugin to avoid wrapping custom components when slot/polymorphic props are used, preventing breakage of composition patterns.

Key Changes:

  • Detects asChild and forwardedAs props on JSX opening elements.
  • Adds guard to exclude such components from the always-on wrapper logic.
  • Documents rationale for skipping to maintain parent-child relationships.

Review Notes:

  • Consider behavior when these props arrive via spread (may not be detected).
  • Ensure no regressions for block provider logic and the existing skip list.

@lightninglu10 lightninglu10 self-assigned this Dec 3, 2025
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

❇️ CodePress Review Summary

👋 Hey team,

Overall the changes look solid, but I spotted 1 must-fix issue and left 0 helpful notes inline.

Here's the quick rundown:

✅ Decision: APPROVE
The change is targeted, low-risk, and fixes breakage for popular composition patterns. No clear blocking issues identified; edge cases can be covered with follow-up tests.

🚧 Needs a bit of love

The change modifies core wrapper-insertion logic by exempting components with asChild or forwardedAs, but there is no test coverage validating these behaviors. Given this affects a widely used transform, the absence of tests creates a high risk of regressions. Targeted tests should verify the non-wrapping behavior for these props, cover interactions with the skip list and host elements, and document current handling of prop spreads. The current test suite contains no references to asChild or forwardedAs, highlighting the coverage gap that needs to be closed.

// - asChild: Radix UI, Ark UI slot composition
// - forwardedAs: styled-components polymorphic pattern
// These patterns rely on direct parent-child relationships that wrappers would break
let has_slot_prop = Self::has_attr_key(&node.opening.attrs, "asChild")
Copy link

Choose a reason for hiding this comment

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

🔴 REQUIRED: This change alters core wrapper-insertion behavior by skipping custom components with asChild/forwardedAs props, but there are no tests covering these attributes. Given this affects a widely-applied transform, it needs direct test coverage to prevent regressions.

  Please add tests that:
  • Verify components with asChild are not wrapped.
  • Verify components with forwardedAs are not wrapped.
  • Confirm interaction with the skip list and that host elements are unaffected.
  • Include cases with prop spreads to document current behavior/limitations.
  
  Evidence:
  • search_repo "asChild" in tests (test/, tests/) → 0 matches
  • search_repo "forwardedAs" in tests (test/, tests/) → 0 matches

@lightninglu10 lightninglu10 merged commit 7537115 into main Dec 5, 2025
8 checks passed
@lightninglu10 lightninglu10 deleted the aschild branch December 5, 2025 01:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants