Skip to content

fix(emotion): preserve dynamic css prop arrays#641

Merged
kdy1 merged 1 commit into
mainfrom
kdy1/fix-emotion-dynamic-css-prop-arrays
Jul 1, 2026
Merged

fix(emotion): preserve dynamic css prop arrays#641
kdy1 merged 1 commit into
mainfrom
kdy1/fix-emotion-dynamic-css-prop-arrays

Conversation

@kdy1

@kdy1 kdy1 commented Jul 1, 2026

Copy link
Copy Markdown
Member

Summary

Fixes #635.

This keeps dynamic JSX css prop arrays, such as css={[styles.row, {}]}, as raw arrays instead of rewriting them to css([...]). Emotion needs the raw array to reach the JSX runtime so theme callbacks can be evaluated with the active theme.

Changes

  • Restrict the JSX css prop rewrite to object expressions and fully static array expressions.
  • Leave dynamic arrays unwrapped when they contain identifiers, member expressions, calls, spreads, or other runtime expressions.
  • Add a regression fixture for the reported spread case and the same dynamic array without prop spread.
  • Add a WASM regression test to ensure dynamic arrays do not produce css([.
  • Add a patch changeset for @swc/plugin-emotion.

Validation

  • cargo test -p swc_emotion
  • cargo test -p swc_emotion --test fixture -- --include-ignored
  • RUSTFLAGS='--cfg swc_ast_unknown' cargo build -p swc_plugin_emotion --target wasm32-wasip1
  • node_modules/.bin/vitest run packages/emotion/__tests__/wasm.test.ts --testTimeout=0

Note: pnpm -F @swc/plugin-emotion test was blocked in this environment before running by pnpm build-script approval (ERR_PNPM_IGNORED_BUILDS), so I ran the package script's build and vitest steps directly.

@changeset-bot

changeset-bot Bot commented Jul 1, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 3298736

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@swc/plugin-emotion Patch

Not sure what this means? Click here to learn what changesets are.

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

@kdy1 kdy1 enabled auto-merge (squash) July 1, 2026 03:08
@claude

claude Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review: fix(emotion): preserve dynamic css prop arrays

Thanks for the fix — I traced through the logic and it's a well-targeted, conservative solution to #635. The core insight is right: wrapping css={[styles.row, {}]} into css([styles.row, {}]) evaluates the array at module scope, where the styles.row theme callback never receives the active theme. Leaving the array raw lets Emotion's JSX runtime resolve callbacks with the theme in context. Good call keeping the recursion conservative (identifiers/members/calls/spreads/computed keys → treated as dynamic), since the transform can't statically know whether an identifier is a theme callback.

Correctness ✔

  • Verified the existing static-array fixture issues/160 (css={[{color:"blue"}, {background:"white"}]}) still passes can_rewrite_css_prop_array and continues to emit css([...]) — no snapshot regression.
  • The #[cfg(swc_ast_unknown)] _ => false arm in can_rewrite_static_css_prop_object correctly handles the unknown-variant build (matches the RUSTFLAGS='--cfg swc_ast_unknown' path in your validation), while the other two matchers rely on their existing _ => false catch-all. Consistent with the codebase pattern.
  • Shorthand props ({ display }), method/getter/setter props, spreads, and computed keys all fall through to false, so a dynamic object nested in a static-looking array is correctly left unwrapped. Nice.

Minor points (non-blocking)

  1. break vs continue inconsistency (lib.rs:521-522): a dynamic array breaks the attr loop while other dynamic css values (identifier, call, template) still continue. Since there's only one css prop per element these are functionally equivalent, but the asymmetry reads oddly. Consider using the same terminator for both, or a short comment noting they're equivalent because there's at most one css attr.

  2. Object css props remain unconditionally wrapped (lib.rs:519): Expr::Object(_) => {} wraps regardless of static/dynamic content, whereas arrays now require a static check. This is intentional and correct for the array-runtime theme-resolution case (objects don't have the [callback, ...] shape Emotion resolves at runtime), but a one-line comment explaining why objects skip the static analysis would save the next reader the head-scratch.

  3. Over-conservative literals (can_rewrite_static_css_prop_expr): negative numbers (-5) are Expr::Unary, not Expr::Lit, so an array containing one is treated as dynamic and left unwrapped. Harmless (Emotion handles it) and rare, just noting it.

Test coverage ✔

The WASM test plus the issues/635 fixture (covering both the spread and non-spread cases) exercise the new path well, and the fixture is auto-discovered by the tests/fixture/**/input.tsx glob. The negative assertion not.toContain("css([") is a good guard. Optionally, a fixture asserting a nested dynamic array (e.g. css={[[styles.row]]}) stays unwrapped would lock down the recursion, but the current coverage is sufficient.

Overall this looks good to merge. 👍

@kdy1 kdy1 merged commit b6b2355 into main Jul 1, 2026
11 checks passed
@kdy1 kdy1 deleted the kdy1/fix-emotion-dynamic-css-prop-arrays branch July 1, 2026 03:12
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-emotion ≥ 14.8.0 wraps array css props in css() when the element spreads props, dropping theme-function styles

1 participant