Skip to content

Commit 6e3615c

Browse files
committed
refactor: deduplicate shared patterns across codebase
- Extract isValidIdentifierName to string-utils.ts (was defined 6 times) - Consolidate resolveThemeAst in value-patterns.ts to reuse resolveThemeFromPropsMember from template-literals.ts - Extract resolveTplBranch and applyResolvedEntries helpers in css-helper-conditional.ts to deduplicate template literal branch handling https://claude.ai/code/session_01CEoafpHoAaRMVmm1WtcXu1
1 parent de4ba42 commit 6e3615c

File tree

9 files changed

+68
-111
lines changed

9 files changed

+68
-111
lines changed

src/internal/emit-wrappers/emit-intrinsic-should-forward-prop.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import type { StyledDecl } from "../transform-types.js";
88
import { getBridgeClassVar } from "../utilities/bridge-classname.js";
99
import { buildStyleFnConditionExpr } from "../utilities/jscodeshift-utils.js";
10+
import { isValidIdentifierName } from "../utilities/string-utils.js";
1011
import { type ExpressionKind, type InlineStyleProp, type WrapperPropDefaults } from "./types.js";
1112
import { SX_PROP_TYPE_TEXT, type JsxAttr, type StatementKind } from "./wrapper-emitter.js";
1213
import { emitStyleMerging } from "./style-merger.js";
@@ -150,10 +151,9 @@ export function emitShouldForwardPropWrappers(ctx: EmitIntrinsicContext): void {
150151
!!dropPrefixFromFilter &&
151152
(usedAttrs.has("*") ||
152153
[...usedAttrs].some((n) => n.startsWith(dropPrefixFromFilter) && !extraProps.has(n)));
153-
const isValidIdentifier = (name: string): boolean => /^[$A-Z_][0-9A-Z_$]*$/i.test(name);
154154
const knownPrefixProps = dropPrefixFromFilter
155155
? [...extraProps].filter(
156-
(p: string) => p.startsWith(dropPrefixFromFilter) && isValidIdentifier(p),
156+
(p: string) => p.startsWith(dropPrefixFromFilter) && isValidIdentifierName(p),
157157
)
158158
: [];
159159
const knownPrefixPropsSet = new Set(knownPrefixProps);
@@ -183,7 +183,7 @@ export function emitShouldForwardPropWrappers(ctx: EmitIntrinsicContext): void {
183183
const staticVariantPropTypes = buildStaticVariantPropTypes(d);
184184
const lines: string[] = [];
185185
for (const p of extraProps) {
186-
if (!isValidIdentifier(p)) {
186+
if (!isValidIdentifierName(p)) {
187187
continue;
188188
}
189189
const variantType = variantDimByProp.get(p);
@@ -219,7 +219,7 @@ export function emitShouldForwardPropWrappers(ctx: EmitIntrinsicContext): void {
219219
const staticVariantPropTypes = buildStaticVariantPropTypes(d);
220220
const lines: string[] = [];
221221
for (const p of extraProps) {
222-
if (!isValidIdentifier(p) || explicitPropNames.has(p) || compoundWhenKeys.has(p)) {
222+
if (!isValidIdentifierName(p) || explicitPropNames.has(p) || compoundWhenKeys.has(p)) {
223223
continue;
224224
}
225225
const variantType = variantDimByProp.get(p);

src/internal/emit-wrappers/emit-intrinsic-simple.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import type { JSCodeshift } from "jscodeshift";
88
import type { StyledDecl } from "../transform-types.js";
99
import { getBridgeClassVar } from "../utilities/bridge-classname.js";
10+
import { isValidIdentifierName } from "../utilities/string-utils.js";
1011
import {
1112
collectInlineStylePropNames,
1213
type ExpressionKind,
@@ -650,11 +651,10 @@ export function emitSimpleExportedIntrinsicWrappers(ctx: EmitIntrinsicContext):
650651
for (const k of compoundVariantWhenKeys) {
651652
keys.delete(k);
652653
}
653-
const isValidIdentifier = (name: string): boolean => /^[$A-Z_][0-9A-Z_$]*$/i.test(name);
654654
const filtered = [...keys].filter(
655655
(k) =>
656656
k &&
657-
isValidIdentifier(k) &&
657+
isValidIdentifierName(k) &&
658658
k !== "children" &&
659659
k !== "className" &&
660660
k !== "style" &&

src/internal/emit-wrappers/variant-condition.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
*/
99
import type { JSCodeshift } from "jscodeshift";
1010
import type { ExpressionKind } from "./types.js";
11+
import { isValidIdentifierName } from "../utilities/string-utils.js";
1112

1213
export type LogicalExpressionOperand = Parameters<JSCodeshift["logicalExpression"]>[1];
1314

@@ -17,8 +18,6 @@ type VariantConditionResult = {
1718
isBoolean: boolean;
1819
};
1920

20-
const isValidIdentifier = (name: string): boolean => /^[$A-Z_][0-9A-Z_$]*$/i.test(name);
21-
2221
/**
2322
* Parse a variant "when" string into an AST condition expression.
2423
*
@@ -43,7 +42,7 @@ export function parseVariantWhenToAst(
4342
.split(".")
4443
.map((part) => part.trim())
4544
.filter(Boolean);
46-
if (parts.length < 2 || parts.some((part) => !isValidIdentifier(part))) {
45+
if (parts.length < 2 || parts.some((part) => !isValidIdentifierName(part))) {
4746
return null;
4847
}
4948
return parts
@@ -65,13 +64,13 @@ export function parseVariantWhenToAst(
6564
.map((part) => part.trim())
6665
.filter(Boolean);
6766
const last = parts[parts.length - 1];
68-
if (!last || !isValidIdentifier(last)) {
67+
if (!last || !isValidIdentifierName(last)) {
6968
return { propName: null, expr: j.identifier(trimmedRaw) };
7069
}
7170
const root = parts[0];
7271
if (root === "props" || root === "p") {
7372
const propRoot = parts[1];
74-
if (!propRoot || !isValidIdentifier(propRoot)) {
73+
if (!propRoot || !isValidIdentifierName(propRoot)) {
7574
return { propName: null, expr: j.identifier(trimmedRaw) };
7675
}
7776
const expr = parts

src/internal/lower-rules/css-helper-conditional.ts

Lines changed: 33 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -1636,16 +1636,8 @@ export function createCssHelperConditionalHandler(ctx: CssHelperConditionalConte
16361636
continue;
16371637
}
16381638

1639-
const consResolved = resolveTemplateLiteralBranch(tplCtx, {
1640-
node: split.consTpl as Parameters<typeof resolveTemplateLiteralBranch>[1]["node"],
1641-
paramName,
1642-
bindings,
1643-
});
1644-
const altResolved = resolveTemplateLiteralBranch(tplCtx, {
1645-
node: split.altTpl as Parameters<typeof resolveTemplateLiteralBranch>[1]["node"],
1646-
paramName,
1647-
bindings,
1648-
});
1639+
const consResolved = resolveTplBranch(split.consTpl as ExpressionKind);
1640+
const altResolved = resolveTplBranch(split.altTpl as ExpressionKind);
16491641
if (!consResolved || !altResolved) {
16501642
skipIndices.add(split.ternaryIdx);
16511643
continue;
@@ -1703,97 +1695,72 @@ export function createCssHelperConditionalHandler(ctx: CssHelperConditionalConte
17031695
return false;
17041696
};
17051697

1698+
// Helper to resolve a template literal branch and apply its entries
1699+
const resolveTplBranch = (
1700+
node: ExpressionKind,
1701+
): ReturnType<typeof resolveTemplateLiteralBranch> =>
1702+
resolveTemplateLiteralBranch(tplCtx, { node: node as any, paramName, bindings });
1703+
1704+
// Helper to apply all entries (style, dynamic, inline) from a resolved branch
1705+
const applyResolvedEntries = (
1706+
resolved: NonNullable<ReturnType<typeof resolveTemplateLiteralBranch>>,
1707+
info: TestInfo,
1708+
when: string,
1709+
): void => {
1710+
if (Object.keys(resolved.style).length > 0) {
1711+
applyVariant(info, resolved.style);
1712+
}
1713+
if (resolved.dynamicEntries.length > 0) {
1714+
applyDynamicEntries(resolved.dynamicEntries, when);
1715+
}
1716+
if (resolved.inlineEntries.length > 0) {
1717+
applyInlineEntries(resolved.inlineEntries, when);
1718+
}
1719+
};
1720+
17061721
// Check altIsEmpty BEFORE altIsTpl since empty templates are also template literals
17071722
// and the altIsEmpty case doesn't require invertWhen (which fails for compound conditions)
17081723
if (consIsTpl && altIsEmpty) {
17091724
dropAllTestInfoProps(testInfo);
1710-
const consResolved = resolveTemplateLiteralBranch(tplCtx, {
1711-
node: cons as any,
1712-
paramName,
1713-
bindings,
1714-
});
1725+
const consResolved = resolveTplBranch(cons);
17151726
if (!consResolved) {
17161727
// Fallback: try splitting dynamic property name ternaries
17171728
if (tryResolveDynamicPropertyNameTpl(cons)) {
17181729
return true;
17191730
}
17201731
return false;
17211732
}
1722-
if (Object.keys(consResolved.style).length > 0) {
1723-
applyVariant(testInfo, consResolved.style);
1724-
}
1725-
if (consResolved.dynamicEntries.length > 0) {
1726-
applyDynamicEntries(consResolved.dynamicEntries, testInfo.when);
1727-
}
1728-
if (consResolved.inlineEntries.length > 0) {
1729-
applyInlineEntries(consResolved.inlineEntries, testInfo.when);
1730-
}
1733+
applyResolvedEntries(consResolved, testInfo, testInfo.when);
17311734
return true;
17321735
}
17331736

17341737
if (consIsTpl && altIsTpl) {
17351738
dropAllTestInfoProps(testInfo);
1736-
const consResolved = resolveTemplateLiteralBranch(tplCtx, {
1737-
node: cons as any,
1738-
paramName,
1739-
bindings,
1740-
});
1741-
const altResolved = resolveTemplateLiteralBranch(tplCtx, {
1742-
node: alt as any,
1743-
paramName,
1744-
bindings,
1745-
});
1739+
const consResolved = resolveTplBranch(cons);
1740+
const altResolved = resolveTplBranch(alt);
17461741
if (!consResolved || !altResolved) {
17471742
return false;
17481743
}
17491744
const invertedWhen = invertWhen(testInfo.when);
17501745
if (!invertedWhen) {
17511746
return false;
17521747
}
1753-
if (Object.keys(consResolved.style).length > 0) {
1754-
applyVariant(testInfo, consResolved.style);
1755-
}
1756-
if (Object.keys(altResolved.style).length > 0) {
1757-
applyVariant({ ...testInfo, when: invertedWhen }, altResolved.style);
1758-
}
1759-
if (consResolved.dynamicEntries.length > 0) {
1760-
applyDynamicEntries(consResolved.dynamicEntries, testInfo.when);
1761-
}
1762-
if (altResolved.dynamicEntries.length > 0) {
1763-
applyDynamicEntries(altResolved.dynamicEntries, invertedWhen);
1764-
}
1765-
if (consResolved.inlineEntries.length > 0) {
1766-
applyInlineEntries(consResolved.inlineEntries, testInfo.when);
1767-
}
1768-
if (altResolved.inlineEntries.length > 0) {
1769-
applyInlineEntries(altResolved.inlineEntries, invertedWhen);
1770-
}
1748+
applyResolvedEntries(consResolved, testInfo, testInfo.when);
1749+
applyResolvedEntries(altResolved, { ...testInfo, when: invertedWhen }, invertedWhen);
17711750
return true;
17721751
}
17731752

17741753
if (consIsEmpty && altIsTpl) {
17751754
dropAllTestInfoProps(testInfo);
1776-
const altResolved = resolveTemplateLiteralBranch(tplCtx, {
1777-
node: alt as any,
1778-
paramName,
1779-
bindings,
1780-
});
1755+
const altResolved = resolveTplBranch(alt);
17811756
if (!altResolved) {
17821757
return false;
17831758
}
17841759
const invertedWhen = invertWhen(testInfo.when);
17851760
if (!invertedWhen) {
17861761
return false;
17871762
}
1788-
if (Object.keys(altResolved.style).length > 0) {
1789-
applyVariant({ ...testInfo, when: invertedWhen }, altResolved.style);
1790-
}
1791-
if (altResolved.dynamicEntries.length > 0) {
1792-
applyDynamicEntries(altResolved.dynamicEntries, invertedWhen);
1793-
}
1794-
if (altResolved.inlineEntries.length > 0) {
1795-
applyInlineEntries(altResolved.inlineEntries, invertedWhen);
1796-
}
1763+
applyResolvedEntries(altResolved, { ...testInfo, when: invertedWhen }, invertedWhen);
17971764
return true;
17981765
}
17991766

src/internal/lower-rules/import-resolution.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
isFunctionNode,
1515
isIdentifierNode,
1616
} from "../utilities/jscodeshift-utils.js";
17+
import { isValidIdentifierName } from "../utilities/string-utils.js";
1718

1819
export const buildSafeIndexedParamName = (
1920
preferred: string,
@@ -284,5 +285,3 @@ export const createImportResolver = (args: {
284285
resolveImportForExpr,
285286
};
286287
};
287-
288-
const isValidIdentifierName = (name: string): boolean => /^[$A-Z_][0-9A-Z_$]*$/i.test(name);

src/internal/lower-rules/template-literals.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import {
2929
type ArrowFnParamBindings,
3030
type CallExpressionNode,
3131
} from "../utilities/jscodeshift-utils.js";
32+
import { isValidIdentifierName } from "../utilities/string-utils.js";
3233
import { parseCssTemplateToRules } from "./css-helper.js";
3334
import { extractStaticParts } from "./interpolations.js";
3435
import { buildTemplateWithStaticParts } from "./inline-styles.js";
@@ -703,7 +704,7 @@ function resolveDynamicTemplateExpr(args: {
703704
return null;
704705
}
705706

706-
function resolveThemeFromPropsMember(args: {
707+
export function resolveThemeFromPropsMember(args: {
707708
expr: Expression;
708709
paramName: string;
709710
filePath: string;
@@ -853,7 +854,7 @@ function buildPropAccessExpr(
853854
propName: string,
854855
defaultValue?: unknown,
855856
): ExpressionKind {
856-
const isIdent = /^[$A-Z_][0-9A-Z_$]*$/i.test(propName);
857+
const isIdent = isValidIdentifierName(propName);
857858
const baseExpr = isIdent
858859
? (j.identifier(propName) as ExpressionKind)
859860
: (j.memberExpression(j.identifier("props"), j.literal(propName), true) as ExpressionKind);

src/internal/lower-rules/value-patterns.ts

Lines changed: 10 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ import {
1414
resolveStaticExpressionValue,
1515
setIdentifierTypeAnnotation,
1616
} from "../utilities/jscodeshift-utils.js";
17+
import { isValidIdentifierName } from "../utilities/string-utils.js";
1718
import { buildSafeIndexedParamName } from "./import-resolution.js";
19+
import { resolveThemeFromPropsMember } from "./template-literals.js";
1820
import { cssValueToJs, styleKeyWithSuffix } from "../transform/helpers.js";
1921
import { cssPropertyToIdentifier, makeCssProperty } from "./shared.js";
2022
import type { LowerRulesState } from "./state.js";
@@ -278,29 +280,14 @@ export const createValuePatternHandlers = (ctx: ValuePatternContext) => {
278280
if (hasLocalThemeBinding) {
279281
return null;
280282
}
281-
const path = getMemberPathFromIdentifier(node as any, paramName);
282-
if (!path || path[0] !== "theme") {
283-
return null;
284-
}
285-
const themePath = path.slice(1).join(".");
286-
if (!themePath) {
287-
return null;
288-
}
289-
const resolved = resolveValue({
290-
kind: "theme",
291-
path: themePath,
283+
return resolveThemeFromPropsMember({
284+
expr: node,
285+
paramName: paramName!,
292286
filePath,
293-
loc: getNodeLocStart(node) ?? undefined,
287+
parseExpr,
288+
resolveValue,
289+
resolverImports,
294290
});
295-
if (!resolved) {
296-
return null;
297-
}
298-
registerImports(resolved.imports, resolverImports);
299-
const exprAst = parseExpr(resolved.expr);
300-
if (!exprAst) {
301-
return null;
302-
}
303-
return exprAst as ExpressionKind;
304291
};
305292

306293
const readPropAccess = (node: any): string | null => {
@@ -334,7 +321,7 @@ export const createValuePatternHandlers = (ctx: ValuePatternContext) => {
334321
const altTheme = resolveThemeAst(body.alternate);
335322

336323
const buildPropAccess = (prop: string): ExpressionKind => {
337-
const isIdent = /^[$A-Z_][0-9A-Z_$]*$/i.test(prop);
324+
const isIdent = isValidIdentifierName(prop);
338325
return isIdent
339326
? j.memberExpression(j.identifier("props"), j.identifier(prop))
340327
: j.memberExpression(j.identifier("props"), j.literal(prop), true);
@@ -382,7 +369,7 @@ export const createValuePatternHandlers = (ctx: ValuePatternContext) => {
382369
styleFnDecls.set(fnKey, j.arrowFunctionExpression([param], bodyExpr));
383370
}
384371
if (!styleFnFromProps.some((p) => p.fnKey === fnKey)) {
385-
const isIdent = /^[$A-Z_][0-9A-Z_$]*$/i.test(nullishPropName);
372+
const isIdent = isValidIdentifierName(nullishPropName);
386373
const baseArg = isIdent ? j.identifier(nullishPropName) : buildPropAccess(nullishPropName);
387374
const callArg = j.logicalExpression("??", baseArg, fallbackTheme);
388375
styleFnFromProps.push({

src/internal/transform-steps/analyze-before-emit.ts

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import {
1919
isFunctionNode,
2020
literalToStaticValue,
2121
} from "../utilities/jscodeshift-utils.js";
22-
import { escapeRegex } from "../utilities/string-utils.js";
22+
import { escapeRegex, isValidIdentifierName } from "../utilities/string-utils.js";
2323
import type { PromotedStyleEntry } from "../transform-types.js";
2424
import { parseVariantWhenToAst } from "../emit-wrappers/variant-condition.js";
2525
import { BLOCKED_INTRINSIC_ATTR_RENAMES } from "../emit-wrappers/types.js";
@@ -2041,12 +2041,6 @@ type PromotedParamType = "number" | "string" | "numberOrString";
20412041
const LENGTH_LIKE_CSS_PROP_RE =
20422042
/^(top|right|bottom|left|width|height|minWidth|maxWidth|minHeight|maxHeight|margin|padding|gap|inset|translate|fontSize|letterSpacing|lineHeight|borderWidth|borderRadius|outline)/;
20432043

2044-
const IDENTIFIER_NAME_RE = /^[$A-Z_][0-9A-Z_$]*$/i;
2045-
2046-
function isValidIdentifierName(name: string): boolean {
2047-
return IDENTIFIER_NAME_RE.test(name);
2048-
}
2049-
20502044
function coerceToStringForStyleX(cssProp: string, value: unknown): unknown {
20512045
if (STYLEX_STRING_ONLY_CSS_PROPS.has(cssProp) && typeof value === "number") {
20522046
return String(value);

src/internal/utilities/string-utils.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,16 @@
66
* Capitalizes the first character of a string.
77
* @example capitalize("hello") => "Hello"
88
*/
9+
/**
10+
* Checks whether a string is a valid JavaScript identifier name.
11+
* @example isValidIdentifierName("foo") => true
12+
* @example isValidIdentifierName("$bar") => true
13+
* @example isValidIdentifierName("some-prop") => false
14+
*/
15+
export function isValidIdentifierName(name: string): boolean {
16+
return /^[$A-Z_][0-9A-Z_$]*$/i.test(name);
17+
}
18+
919
export function capitalize(s: string): string {
1020
return s.charAt(0).toUpperCase() + s.slice(1);
1121
}

0 commit comments

Comments
 (0)