Codex/plan migration from styled components to stylex g54kic#5
Closed
Codex/plan migration from styled components to stylex g54kic#5
Conversation
skovhus
force-pushed
the
codex/plan-migration-from-styled-components-to-stylex-g54kic
branch
from
2a88d30 to
c1146fe
Compare
skovhus
deleted the
codex/plan-migration-from-styled-components-to-stylex-g54kic
branch
cursor Bot
pushed a commit
that referenced
this pull request
Feb 27, 2026
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>
Merged
skovhus
added a commit
that referenced
this pull request
Feb 28, 2026
…nt, helper sizing, prop forwarding Bug #5: Handle pseudoElementWithPseudo selectors (e.g. ::-webkit-slider-thumb:hover) in css-helper.ts selector dispatch. Fix process-rules.ts to distinguish AST nodes from pseudo maps using "default" key check instead of typeof object. Bug #4: In css-helper-conditional.ts, skip unsupported selectors (child/descendant) with continue instead of bailing with return null. Add retry logic in tryResolveDynamicPropertyNameTpl to try different ternary splits when the first attempt fails due to unresolvable expressions. Bug #2: In rule-interpolated-declaration.ts, trace local variables back to function parameters in helper calls (e.g. sizeMap[size] -> size). Generate style functions with derived expressions as call arguments and inferred number types. Fix property shorthand when key and value identifiers match. Bug #7: In emit-component.ts, skip forwarding props that are only used in style functions and not in .attrs() defaults. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
skovhus
added a commit
that referenced
this pull request
Mar 1, 2026
* Fix 4 orbiter codemod bugs: pseudo-element hover, conditional alignment, helper sizing, prop forwarding Bug #5: Handle pseudoElementWithPseudo selectors (e.g. ::-webkit-slider-thumb:hover) in css-helper.ts selector dispatch. Fix process-rules.ts to distinguish AST nodes from pseudo maps using "default" key check instead of typeof object. Bug #4: In css-helper-conditional.ts, skip unsupported selectors (child/descendant) with continue instead of bailing with return null. Add retry logic in tryResolveDynamicPropertyNameTpl to try different ternary splits when the first attempt fails due to unresolvable expressions. Bug #2: In rule-interpolated-declaration.ts, trace local variables back to function parameters in helper calls (e.g. sizeMap[size] -> size). Generate style functions with derived expressions as call arguments and inferred number types. Fix property shorthand when key and value identifiers match. Bug #7: In emit-component.ts, skip forwarding props that are only used in style functions and not in .attrs() defaults. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix theme value resolution in pseudo-element + pseudo-class combinations When a theme value interpolation (e.g., color("controlPrimaryHover")) appears inside a combined pseudo-element + pseudo-class selector like ::-webkit-slider-thumb:hover, delegate to applyResolvedPropValue instead of writing directly to perPropPseudo. This ensures the value is correctly scoped within the pseudo-element's nested selector bucket, producing the merged { default: ..., ":hover": ... } format inside "::-webkit-slider-thumb". Previously, the hover backgroundColor was dropped because tryHandleThemeValueInPseudo wrote to the top-level perPropPseudo map, ignoring the pseudoElement context entirely. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Bail on local helper calls with untraceable template expressions When tryHandleLocalHelperCall analyzes a helper function's return template literal, verify that every CSS property's expression can be traced back to the function parameter (either directly, via unit suffix, or through a local variable derived from the param). If any expression can't be traced, bail instead of silently generating incorrect code that passes the raw prop value as a CSS value. This fixes AvatarContainer where avatarSizeToCSS uses imperative mutation (let width = pixelSize; if (...) { width = ... }) which the single-level variable tracing couldn't follow, causing it to emit width: size (AvatarSize string) instead of a computed pixel number. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Address review comments: prop forwarding and helper-local scope - Forward style-function-only props to wrapped custom components (styled(Component) should forward all non-transient props like styled-components does). Only intrinsic element wrappers should filter style-only props. - Bail on derived call args that reference helper-local variables not in scope at the call site (e.g., const scale = 2; const px = sizeMap[size] * scale would produce invalid code referencing scale). - Refactor: extract resolveDerivedLocalVariable helper, replace inline containsParam with collectIdentifiers utility, eliminate non-null assertion. - Fix CI: add missing token names to tokens.stylex.ts, format plan. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Remove completed plan file Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix rendering CI: add conditional-alignChildSizing to expected failures The test case now includes a child selector (& > div) that StyleX cannot express, causing an expected visual mismatch. Also remove two keyframes entries that now pass. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Restore bail on unsupported selectors and fix direct param tracing - Revert `continue` to `return null` for non-`&` selectors in template-literals.ts and css-helper-conditional.ts — the `continue` approach silently dropped unsupported selector styling (lossy) - Revert conditional-alignChildSizing test case to remove the `& > div` child selector that triggered the lossy behavior - Restore EXPECTED_FAILURES in verify-storybook-rendering.mts to match main (conditional-alignChildSizing should not be expected to fail) - Fix direct param interpolation tracing: track CSS properties that directly reference the function parameter (with or without unit) so `opacity: ${value}` doesn't falsely bail - Add unit test for direct param usage without unit suffix Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix 3 codemod issues discovered during orbiter migration 1. no-shadow lint errors: cssPropertyToIdentifier now accepts avoidNames set built from importMap keys. When a generated style function parameter name (e.g., zIndex) conflicts with a top-level import, it is suffixed with "Value" (e.g., zIndexValue) to prevent shadowing. 2. Variant condition props forwarded to wrapped component: Track props added purely by variant conditions and pseudo-alias selectors in a styleOnlyConditionProps set. These are no longer forwarded to the base component unless it explicitly accepts them. 3. Event handler type annotations: Add post-processing pass that annotates unannotated event handler arrow function parameters at JSX usage sites of converted components with the appropriate React event type (e.g., onChange handler gets React.ChangeEvent annotation). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix intrinsic event handler annotations for converted components * Fix style function and inline style props forwarded to base component Props used only in CSS interpolations (styleFn) and inline style expressions were being forwarded as JSX attributes to the wrapped component, causing TypeScript errors like 'Property "position" does not exist on FlexProps'. Mark these props as style-only so they are not forwarded unless the base component explicitly accepts them. Also moves .attrs() static attrs after {...rest} spread to match styled-components override semantics. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix ref forwarding, warning messages, and PropsWithChildren in narrow types - Only include `ref` in narrow intrinsic types when the component actually forwards it (via `{...rest}` or explicit ref usage), preventing "ref declared but not forwarded" lint errors - Replace `React.PropsWithChildren<{...}>` with inline `{ ...; children?: React.ReactNode }` for cleaner, more explicit types - Remove interpolated function/prop names from bail warning messages so warnings can be grouped centrally - Bail on local helper functions that return untraceable CSS instead of silently dropping styles Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix WarningType union to match de-interpolated warning strings Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Address PR review feedback: prop forwarding, derived types, event annotations - Forward style-only condition props when base component type is unresolvable, matching styled-components semantics (all non-transient props are forwarded) - Use number | string for derived helper value style function params instead of hardcoded number, supporting both numeric and token-based lookup tables - Limit event handler type annotations to intrinsic-based components only, avoiding incorrect React.*Event annotations on custom component callbacks Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.