From 451236d2ccb4de432181a1c7002a69608fadcaea Mon Sep 17 00:00:00 2001 From: Hyo Date: Sun, 16 Nov 2025 01:24:19 +0900 Subject: [PATCH 1/2] fix: inline style override for padding/margin properties - Reverse style array order in Babel plugin optimization ([__ks.base, props.style]) - Expand shorthand properties (padding, margin) to longhand for proper override - Add tests for expandShorthandProperties function --- .../babel-plugin-kstyled/src/css-parser.ts | 20 ++- packages/babel-plugin-kstyled/src/index.ts | 5 +- packages/kstyled/src/css.tsx | 16 +- packages/kstyled/src/styled.tsx | 9 +- .../src/utils/__tests__/style-merger.test.ts | 147 ++++++++++++++++++ packages/kstyled/src/utils/style-merger.ts | 71 ++++++++- 6 files changed, 250 insertions(+), 18 deletions(-) create mode 100644 packages/kstyled/src/utils/__tests__/style-merger.test.ts diff --git a/packages/babel-plugin-kstyled/src/css-parser.ts b/packages/babel-plugin-kstyled/src/css-parser.ts index 1b43fce..244fa0a 100644 --- a/packages/babel-plugin-kstyled/src/css-parser.ts +++ b/packages/babel-plugin-kstyled/src/css-parser.ts @@ -212,16 +212,20 @@ function expandShorthand(property: string, value: string): Record | if (property === 'padding') { if (parts.length === 2) { // padding: vertical horizontal + // IMPORTANT: Use longhand properties for proper style override in React Native return { - paddingVertical: parts[0], - paddingHorizontal: parts[1], + paddingTop: parts[0], + paddingRight: parts[1], + paddingBottom: parts[0], + paddingLeft: parts[1], }; } else if (parts.length === 3) { // padding: top horizontal bottom return { paddingTop: parts[0], - paddingHorizontal: parts[1], + paddingRight: parts[1], paddingBottom: parts[2], + paddingLeft: parts[1], }; } else if (parts.length === 4) { // padding: top right bottom left @@ -235,16 +239,20 @@ function expandShorthand(property: string, value: string): Record | } else if (property === 'margin') { if (parts.length === 2) { // margin: vertical horizontal + // IMPORTANT: Use longhand properties for proper style override in React Native return { - marginVertical: parts[0], - marginHorizontal: parts[1], + marginTop: parts[0], + marginRight: parts[1], + marginBottom: parts[0], + marginLeft: parts[1], }; } else if (parts.length === 3) { // margin: top horizontal bottom return { marginTop: parts[0], - marginHorizontal: parts[1], + marginRight: parts[1], marginBottom: parts[2], + marginLeft: parts[1], }; } else if (parts.length === 4) { // margin: top right bottom left diff --git a/packages/babel-plugin-kstyled/src/index.ts b/packages/babel-plugin-kstyled/src/index.ts index 4e8b2a4..9a1c905 100644 --- a/packages/babel-plugin-kstyled/src/index.ts +++ b/packages/babel-plugin-kstyled/src/index.ts @@ -595,12 +595,13 @@ export default function babelPluginKStyled(): PluginObj { } } - // Create style expression: props.style ? [props.style, __ks.base] : __ks.base + // Create style expression: props.style ? [__ks.base, props.style] : __ks.base + // IMPORTANT: Base styles first, external styles last (external overrides base) const styleExpression = t.conditionalExpression( t.memberExpression(t.identifier('props'), t.identifier('style')), t.arrayExpression([ - t.memberExpression(t.identifier('props'), t.identifier('style')), t.memberExpression(t.identifier(styleId), t.identifier('base')), + t.memberExpression(t.identifier('props'), t.identifier('style')), ]), t.memberExpression(t.identifier(styleId), t.identifier('base')) ); diff --git a/packages/kstyled/src/css.tsx b/packages/kstyled/src/css.tsx index d92fcfa..61acba7 100644 --- a/packages/kstyled/src/css.tsx +++ b/packages/kstyled/src/css.tsx @@ -137,18 +137,24 @@ export const css: CssFactory = Object.assign( const parts = value.split(/\s+/); const prefix = camelProp; // 'padding' or 'margin' + // IMPORTANT: Always use longhand properties for proper override in React Native if (parts.length === 1) { const val = parseFloat(parts[0]); - styleObj[`${prefix}Vertical`] = val; - styleObj[`${prefix}Horizontal`] = val; + styleObj[`${prefix}Top`] = val; + styleObj[`${prefix}Right`] = val; + styleObj[`${prefix}Bottom`] = val; + styleObj[`${prefix}Left`] = val; } else if (parts.length === 2) { - styleObj[`${prefix}Vertical`] = parseFloat(parts[0]); - styleObj[`${prefix}Horizontal`] = parseFloat(parts[1]); + styleObj[`${prefix}Top`] = parseFloat(parts[0]); + styleObj[`${prefix}Right`] = parseFloat(parts[1]); + styleObj[`${prefix}Bottom`] = parseFloat(parts[0]); + styleObj[`${prefix}Left`] = parseFloat(parts[1]); } else if (parts.length === 3) { // top, horizontal, bottom styleObj[`${prefix}Top`] = parseFloat(parts[0]); - styleObj[`${prefix}Horizontal`] = parseFloat(parts[1]); + styleObj[`${prefix}Right`] = parseFloat(parts[1]); styleObj[`${prefix}Bottom`] = parseFloat(parts[2]); + styleObj[`${prefix}Left`] = parseFloat(parts[1]); } else if (parts.length === 4) { styleObj[`${prefix}Top`] = parseFloat(parts[0]); styleObj[`${prefix}Right`] = parseFloat(parts[1]); diff --git a/packages/kstyled/src/styled.tsx b/packages/kstyled/src/styled.tsx index 0a66768..eec93ef 100644 --- a/packages/kstyled/src/styled.tsx +++ b/packages/kstyled/src/styled.tsx @@ -120,10 +120,13 @@ function styledFunction, P = {}, AttrsP = {}>( mergedProps ); - // Build style array with correct priority + // Build style array with correct priority: + // 1. Static compiled styles (lowest) + // 2. Dynamic patch + // 3. External inline styles (highest - ensures override) const styles = buildStyleArray( - compiledStyles, - styleKeys, + mergedCompiledStyles, + mergedStyleKeys, dynamicPatch, externalStyle as StyleValue ); diff --git a/packages/kstyled/src/utils/__tests__/style-merger.test.ts b/packages/kstyled/src/utils/__tests__/style-merger.test.ts new file mode 100644 index 0000000..4f6203e --- /dev/null +++ b/packages/kstyled/src/utils/__tests__/style-merger.test.ts @@ -0,0 +1,147 @@ +import { expandShorthandProperties } from '../style-merger'; + +describe('expandShorthandProperties', () => { + describe('padding shorthand expansion', () => { + it('should expand padding to all four longhand properties', () => { + const input = { + padding: 0, + }; + + const result = expandShorthandProperties(input); + + expect(result).toEqual({ + paddingTop: 0, + paddingRight: 0, + paddingBottom: 0, + paddingLeft: 0, + }); + }); + + it('should expand paddingHorizontal to paddingLeft + paddingRight', () => { + const input = { + paddingHorizontal: 0, + }; + + const result = expandShorthandProperties(input); + + expect(result).toEqual({ + paddingLeft: 0, + paddingRight: 0, + }); + }); + + it('should expand paddingVertical to paddingTop + paddingBottom', () => { + const input = { + paddingVertical: 8, + }; + + const result = expandShorthandProperties(input); + + expect(result).toEqual({ + paddingTop: 8, + paddingBottom: 8, + }); + }); + }); + + describe('margin shorthand expansion', () => { + it('should expand margin to all four longhand properties', () => { + const input = { + margin: 4, + }; + + const result = expandShorthandProperties(input); + + expect(result).toEqual({ + marginTop: 4, + marginRight: 4, + marginBottom: 4, + marginLeft: 4, + }); + }); + + it('should expand marginHorizontal to marginLeft + marginRight', () => { + const input = { + marginHorizontal: 0, + }; + + const result = expandShorthandProperties(input); + + expect(result).toEqual({ + marginLeft: 0, + marginRight: 0, + }); + }); + + it('should expand marginVertical to marginTop + marginBottom', () => { + const input = { + marginVertical: 12, + }; + + const result = expandShorthandProperties(input); + + expect(result).toEqual({ + marginTop: 12, + marginBottom: 12, + }); + }); + }); + + describe('preserving other properties', () => { + it('should preserve non-shorthand properties', () => { + const input = { + padding: 8, + backgroundColor: 'red', + flexDirection: 'row', + }; + + const result = expandShorthandProperties(input); + + expect(result).toEqual({ + paddingTop: 8, + paddingRight: 8, + paddingBottom: 8, + paddingLeft: 8, + backgroundColor: 'red', + flexDirection: 'row', + }); + }); + + it('should handle mixed shorthand properties', () => { + const input = { + padding: 8, + margin: 4, + }; + + const result = expandShorthandProperties(input); + + expect(result).toEqual({ + paddingTop: 8, + paddingRight: 8, + paddingBottom: 8, + paddingLeft: 8, + marginTop: 4, + marginRight: 4, + marginBottom: 4, + marginLeft: 4, + }); + }); + }); + + describe('edge cases', () => { + it('should return null for null input', () => { + const result = expandShorthandProperties(null); + expect(result).toBeNull(); + }); + + it('should return undefined for undefined input', () => { + const result = expandShorthandProperties(undefined); + expect(result).toBeUndefined(); + }); + + it('should return empty object for empty input', () => { + const result = expandShorthandProperties({}); + expect(result).toEqual({}); + }); + }); +}); diff --git a/packages/kstyled/src/utils/style-merger.ts b/packages/kstyled/src/utils/style-merger.ts index b33c0d4..55ed498 100644 --- a/packages/kstyled/src/utils/style-merger.ts +++ b/packages/kstyled/src/utils/style-merger.ts @@ -2,6 +2,68 @@ import type { StyleMetadata, StyleArray, StyleValue, StyleObject, PropsWithTheme import type { CompiledStyles } from '../types'; import { normalizeStyleValue } from '../css-runtime-parser'; +/** + * Expand shorthand padding/margin properties to longhand + * This is necessary because base styles use longhand properties (paddingTop, paddingRight, etc.) + * and React Native gives longhand higher priority than shorthand (paddingHorizontal/paddingVertical) + */ +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function expandShorthandProperties(styleObj: any): any { + if (!styleObj || typeof styleObj !== 'object') { + return styleObj; + } + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const result: any = {}; + + for (const key in styleObj) { + if (Object.prototype.hasOwnProperty.call(styleObj, key)) { + const value = styleObj[key]; + + // Expand padding shorthand to all four sides + if (key === 'padding') { + result.paddingTop = value; + result.paddingRight = value; + result.paddingBottom = value; + result.paddingLeft = value; + } + // Expand margin shorthand to all four sides + else if (key === 'margin') { + result.marginTop = value; + result.marginRight = value; + result.marginBottom = value; + result.marginLeft = value; + } + // Expand paddingHorizontal to paddingLeft + paddingRight + else if (key === 'paddingHorizontal') { + result.paddingLeft = value; + result.paddingRight = value; + } + // Expand paddingVertical to paddingTop + paddingBottom + else if (key === 'paddingVertical') { + result.paddingTop = value; + result.paddingBottom = value; + } + // Expand marginHorizontal to marginLeft + marginRight + else if (key === 'marginHorizontal') { + result.marginLeft = value; + result.marginRight = value; + } + // Expand marginVertical to marginTop + marginBottom + else if (key === 'marginVertical') { + result.marginTop = value; + result.marginBottom = value; + } + // Keep all other properties as-is + else { + result[key] = value; + } + } + } + + return result; +} + /** * Build style array with correct priority order: * 1. Compiled static styles (lowest priority) @@ -29,18 +91,23 @@ export function buildStyleArray( } // Add external styles last (highest priority) + // IMPORTANT: Expand shorthand properties to match base style specificity if (externalStyle) { if (Array.isArray(externalStyle)) { // Handle nested arrays from StyleProp for (const style of externalStyle) { if (style) { + // Expand shorthand properties for proper override + const expanded = expandShorthandProperties(style); // eslint-disable-next-line @typescript-eslint/no-explicit-any - styles.push(style as any); + styles.push(expanded as any); } } } else { + // Expand shorthand properties for proper override + const expanded = expandShorthandProperties(externalStyle); // eslint-disable-next-line @typescript-eslint/no-explicit-any - styles.push(externalStyle as any); + styles.push(expanded as any); } } From 7e5f8bf9916e41f312bf36c7ddc5b3395d37d95f Mon Sep 17 00:00:00 2001 From: Hyo Date: Sun, 16 Nov 2025 01:28:25 +0900 Subject: [PATCH 2/2] fix: tests --- .../src/__tests__/transform.test.ts | 66 ++++++++++++------- packages/kstyled/package.json | 1 + pnpm-lock.yaml | 3 + 3 files changed, 47 insertions(+), 23 deletions(-) diff --git a/packages/babel-plugin-kstyled/src/__tests__/transform.test.ts b/packages/babel-plugin-kstyled/src/__tests__/transform.test.ts index 2f9771d..1004fdd 100644 --- a/packages/babel-plugin-kstyled/src/__tests__/transform.test.ts +++ b/packages/babel-plugin-kstyled/src/__tests__/transform.test.ts @@ -25,9 +25,11 @@ describe('babel-plugin-kstyled', () => { const output = transform(input); - // Should contain paddingVertical and paddingHorizontal - expect(output).toContain('paddingVertical'); - expect(output).toContain('paddingHorizontal'); + // Should expand to longhand properties (not paddingVertical/paddingHorizontal) + expect(output).toContain('paddingTop'); + expect(output).toContain('paddingRight'); + expect(output).toContain('paddingBottom'); + expect(output).toContain('paddingLeft'); // Should NOT contain the shorthand 'padding' property expect(output).not.toContain('padding:'); }); @@ -62,8 +64,11 @@ describe('babel-plugin-kstyled', () => { const output = transform(input); - expect(output).toContain('marginVertical'); - expect(output).toContain('marginHorizontal'); + // Should expand to longhand properties (not marginVertical/marginHorizontal) + expect(output).toContain('marginTop'); + expect(output).toContain('marginRight'); + expect(output).toContain('marginBottom'); + expect(output).toContain('marginLeft'); }); test('should expand multiple shorthand properties', () => { @@ -80,10 +85,15 @@ describe('babel-plugin-kstyled', () => { const output = transform(input); - expect(output).toContain('paddingVertical'); - expect(output).toContain('paddingHorizontal'); - expect(output).toContain('marginVertical'); - expect(output).toContain('marginHorizontal'); + // Should expand all shorthands to longhand + expect(output).toContain('paddingTop'); + expect(output).toContain('paddingRight'); + expect(output).toContain('paddingBottom'); + expect(output).toContain('paddingLeft'); + expect(output).toContain('marginTop'); + expect(output).toContain('marginRight'); + expect(output).toContain('marginBottom'); + expect(output).toContain('marginLeft'); expect(output).toContain('backgroundColor'); }); }); @@ -146,9 +156,11 @@ describe('babel-plugin-kstyled', () => { // Should have getDynamicPatch function expect(output).toContain('getDynamicPatch'); - // Should have static padding styles - expect(output).toContain('paddingVertical'); - expect(output).toContain('paddingHorizontal'); + // Should have static padding styles expanded to longhand + expect(output).toContain('paddingTop'); + expect(output).toContain('paddingRight'); + expect(output).toContain('paddingBottom'); + expect(output).toContain('paddingLeft'); // Dynamic function should be called with (p) expect(output).toMatch(/backgroundColor.*\(p\)/); }); @@ -214,9 +226,11 @@ describe('babel-plugin-kstyled', () => { const output = transform(input); - // Should expand padding shorthand - expect(output).toContain('paddingVertical'); - expect(output).toContain('paddingHorizontal'); + // Should expand padding shorthand to longhand + expect(output).toContain('paddingTop'); + expect(output).toContain('paddingRight'); + expect(output).toContain('paddingBottom'); + expect(output).toContain('paddingLeft'); // Should have attrs expect(output).toContain('attrs'); expect(output).toContain('placeholder'); @@ -266,9 +280,11 @@ describe('babel-plugin-kstyled', () => { const output = transform(input); - // Static styles - expect(output).toContain('paddingVertical'); - expect(output).toContain('paddingHorizontal'); + // Static styles - padding shorthand expanded to longhand + expect(output).toContain('paddingTop'); + expect(output).toContain('paddingRight'); + expect(output).toContain('paddingBottom'); + expect(output).toContain('paddingLeft'); expect(output).toContain('borderRadius'); expect(output).toContain('alignItems'); expect(output).toContain('marginVertical'); @@ -686,11 +702,15 @@ describe('babel-plugin-kstyled', () => { `; const output = transform(input); - // Should expand shorthand with mixed units - expect(output).toContain('paddingVertical'); - expect(output).toContain('paddingHorizontal'); - expect(output).toContain('marginVertical'); - expect(output).toContain('marginHorizontal'); + // Should expand shorthand to longhand + expect(output).toContain('paddingTop'); + expect(output).toContain('paddingRight'); + expect(output).toContain('paddingBottom'); + expect(output).toContain('paddingLeft'); + expect(output).toContain('marginTop'); + expect(output).toContain('marginRight'); + expect(output).toContain('marginBottom'); + expect(output).toContain('marginLeft'); expect(output).toContain('borderRadius'); expect(output).toContain('borderWidth'); }); diff --git a/packages/kstyled/package.json b/packages/kstyled/package.json index a9052ee..39ea8f9 100644 --- a/packages/kstyled/package.json +++ b/packages/kstyled/package.json @@ -46,6 +46,7 @@ "@babel/preset-react": "^7.26.0", "@babel/preset-typescript": "^7.26.0", "@types/babel__core": "^7.20.5", + "@types/jest": "^29.5.14", "@types/react": "~19.1.10", "@types/react-native": "^0.73.0", "@typescript-eslint/eslint-plugin": "^8.19.1", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index ca83928..3c29956 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -215,6 +215,9 @@ importers: '@types/babel__core': specifier: ^7.20.5 version: 7.20.5 + '@types/jest': + specifier: ^29.5.14 + version: 29.5.14 '@types/react': specifier: ~19.1.10 version: 19.1.17