Conversation
skovhus
force-pushed
the
implement-gpt5.2
branch
from
8bb1eb9 to
0567224
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| style: styleFromSingleDeclaration(node.css.property, alt), | ||
| }, | ||
| ], | ||
| }; |
There was a problem hiding this comment.
Handler returns malformed variant data instead of null
The conditionalValueHandler has a fallback return at lines 100-114 that produces malformed splitVariants results instead of returning null. When testPath is null (test expression isn't a MemberExpression), both variants get when: "" (empty string). When testPath has length > 1 (nested access like props.foo.bar), only the first segment is used, producing semantically incorrect conditions. Downstream in emit-wrappers.ts, this causes j.identifier("") to be called with an empty string, producing invalid JavaScript AST. The fallback block should return null to indicate the handler cannot process this pattern.
| const defaultMarkerCall = j.callExpression( | ||
| j.memberExpression(j.identifier("stylex"), j.identifier("defaultMarker")), | ||
| [], | ||
| ); |
There was a problem hiding this comment.
Same AST node reused across multiple locations in tree
The defaultMarkerCall AST node is created once at line 25-28, then the same node instance is pushed into call.arguments for every matching JSX element at line 83. Sharing AST node references across multiple locations in the tree violates AST invariants and can cause recast/jscodeshift to produce corrupted output or throw errors during printing. Each insertion point needs a fresh AST node, which can be created by calling j.callExpression(...) inside the loop or by cloning the node.
Additional Locations (1)
Finding #1 (MODERATE): canSafelyInline and resolvePerSiteProps missed JSXSelfClosingElement nodes (<Foo />) — consumed props on self-closing elements were not checked for safety or variant creation. Fixed by extracting findAllJsxSites() helper that searches both JSXElement and JSXSelfClosingElement, used by all JSX scanning functions. Finding #2 (MODERATE): attrsAsTag (.attrs({ as: SomeComponent })) was not checked — component-ref overrides were silently ignored. Fixed by bailing extractStaticPropsFromAttrs when attrsAsTag is set. Finding #3 (MODERATE): $-prefixed non-consumed transient props would leak to the DOM in the wrapper path for inlined intrinsic elements. Fixed by setting shouldForwardProp.dropPrefix to "$" in applyResolution. Finding #4 (LOW): findVaryingProps had an always-true condition (!allSame || hasAnyValue where hasAnyValue was guaranteed true). Simplified: track all consumed props present at any JSX site. Finding #5 (LOW): Covered by Finding #3's dropPrefix fix. Finding #6 (LOW): Inaccurate comment in applyResolution. Updated. Finding #7 (LOW): Static property inheritance lost for resolved components. Fixed by saving originalBaseIdent before overwriting base. Added 3 regression tests: - attrsAsTag component ref bail - self-closing element with dynamic consumed prop (canSafelyInline) - self-closing element with static consumed prop (variant creation) Co-authored-by: Kenneth Skovhus <skovhus@users.noreply.github.com>
1 participant
GPT5.2 version
Note
Implements a production-ready transform from styled-components to StyleX with a minimal public API.
internal/*) with rule lowering, JSX rewrites, wrappers, keyframes parsing, CSS var rewriting, and output formatting/safetydefineAdapter+ unifiedresolveValueand optional custom handlers; adds built-in dynamic handlersdefineAdapterandrunTransform; updatesexportsand README with usage, options, and advanced customizationsrc/__tests__, adds e2e (run.e2e.test.ts) and fixture adapters; normalizes comparisons viaoxfmtstylis, coverage plugin) and lockfile updatesWritten by Cursor Bugbot for commit d3d4edd. This will update automatically on new commits. Configure here.