From 98d9d1620ac2dd10d3562202ddd310d519593be7 Mon Sep 17 00:00:00 2001 From: Yasuhiro Yamamoto Date: Wed, 15 Jan 2025 16:16:34 +0900 Subject: [PATCH 1/5] fix: Properly parse multi-value shorthand with slash --- packages/shared/__tests__/split-value-test.js | 4 ++ packages/shared/src/utils/split-css-value.js | 42 ++++++++++++++++++- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/packages/shared/__tests__/split-value-test.js b/packages/shared/__tests__/split-value-test.js index ee3f38529..c4c4e1a18 100644 --- a/packages/shared/__tests__/split-value-test.js +++ b/packages/shared/__tests__/split-value-test.js @@ -44,4 +44,8 @@ describe('Ensure CSS values are split correctly', () => { splitValue('calc((100% - 50px) * 0.5) var(--rightpadding, 20px)'), ).toEqual(['calc((100% - 50px) * 0.5)', 'var(--rightpadding,20px)']); }); + + test('Splits a string of values with slash notation appropriately.', () => { + expect(splitValue('1px/2px 3px 4px 5px')).toEqual(['1px/2px', '3px', '4px', '5px']); + }); }); diff --git a/packages/shared/src/utils/split-css-value.js b/packages/shared/src/utils/split-css-value.js index f7c731fe8..15689ebfd 100644 --- a/packages/shared/src/utils/split-css-value.js +++ b/packages/shared/src/utils/split-css-value.js @@ -23,6 +23,44 @@ function printNode(node: PostCSSValueASTNode): string { } } +// Merges slash-separated values within nodes into single nodes. +function combineNodesWithSlash(nodes: PostCSSValueASTNode[]) { + const result = []; + + for (let i = 0; i < nodes.length; i++) { + const node = nodes[i]; + + if (node.type !== 'div' || node.value !== '/') { + result.push(node); + continue; + } + + if (i === 0 || i === nodes.length - 1) { + result.push(node); + continue; + } + + const prev = result.at(-1); + const next = nodes[i + 1]; + + if (!prev || !next || prev.type !== 'word' || next.type !== 'word') { + result.push(node); + continue; + } + + result.pop(); + const combinedNode = { + ...prev, + value: prev.value + node.value + next.value, + sourceEndIndex: next.sourceEndIndex, + }; + result.push(combinedNode); + i++; // 次の要素をスキップ + } + + return result; +} + // Using split(' ') Isn't enough because of values like calc. export default function splitValue( str: TStyleValue, @@ -38,8 +76,8 @@ export default function splitValue( const parsed = parser(str.trim()); - const nodes = parsed.nodes - .filter((node) => node.type !== 'space' && node.type !== 'div') + const nodes = combineNodesWithSlash(parsed.nodes.filter((node) => node.type !== 'space')) + .filter((node) => node.type !== 'div') .map(printNode); if ( From 4e1e0496fb9ce8a705dc071c0794d7ebfac55720 Mon Sep 17 00:00:00 2001 From: Yasuhiro Yamamoto Date: Wed, 15 Jan 2025 16:41:25 +0900 Subject: [PATCH 2/5] refactor: Remove unnecessary comments from code --- packages/shared/src/utils/split-css-value.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/shared/src/utils/split-css-value.js b/packages/shared/src/utils/split-css-value.js index 15689ebfd..07770a032 100644 --- a/packages/shared/src/utils/split-css-value.js +++ b/packages/shared/src/utils/split-css-value.js @@ -55,7 +55,7 @@ function combineNodesWithSlash(nodes: PostCSSValueASTNode[]) { sourceEndIndex: next.sourceEndIndex, }; result.push(combinedNode); - i++; // 次の要素をスキップ + i++; } return result; From f0364ecf15c1f2d8b567c4af2cbfafa34745b4ec Mon Sep 17 00:00:00 2001 From: Yasuhiro Yamamoto Date: Wed, 15 Jan 2025 16:50:58 +0900 Subject: [PATCH 3/5] refactor: Format with Prettier --- packages/shared/__tests__/split-value-test.js | 7 ++++++- packages/shared/src/utils/split-css-value.js | 16 +++++++++------- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/packages/shared/__tests__/split-value-test.js b/packages/shared/__tests__/split-value-test.js index c4c4e1a18..89303d8d1 100644 --- a/packages/shared/__tests__/split-value-test.js +++ b/packages/shared/__tests__/split-value-test.js @@ -46,6 +46,11 @@ describe('Ensure CSS values are split correctly', () => { }); test('Splits a string of values with slash notation appropriately.', () => { - expect(splitValue('1px/2px 3px 4px 5px')).toEqual(['1px/2px', '3px', '4px', '5px']); + expect(splitValue('1px/2px 3px 4px 5px')).toEqual([ + '1px/2px', + '3px', + '4px', + '5px', + ]); }); }); diff --git a/packages/shared/src/utils/split-css-value.js b/packages/shared/src/utils/split-css-value.js index 07770a032..756a31c55 100644 --- a/packages/shared/src/utils/split-css-value.js +++ b/packages/shared/src/utils/split-css-value.js @@ -50,12 +50,12 @@ function combineNodesWithSlash(nodes: PostCSSValueASTNode[]) { result.pop(); const combinedNode = { - ...prev, - value: prev.value + node.value + next.value, - sourceEndIndex: next.sourceEndIndex, - }; - result.push(combinedNode); - i++; + ...prev, + value: prev.value + node.value + next.value, + sourceEndIndex: next.sourceEndIndex, + }; + result.push(combinedNode); + i++; } return result; @@ -76,7 +76,9 @@ export default function splitValue( const parsed = parser(str.trim()); - const nodes = combineNodesWithSlash(parsed.nodes.filter((node) => node.type !== 'space')) + const nodes = combineNodesWithSlash( + parsed.nodes.filter((node) => node.type !== 'space'), + ) .filter((node) => node.type !== 'div') .map(printNode); From 78144fa1f190c0b0817a0fcaf31d366db86024a8 Mon Sep 17 00:00:00 2001 From: Yasuhiro Yamamoto Date: Sat, 18 Jan 2025 23:43:44 +0900 Subject: [PATCH 4/5] fix: Correct border-radius shorthand expansion logic --- packages/shared/__tests__/split-value-test.js | 27 ++++-- .../legacy-expand-shorthands.js | 2 +- packages/shared/src/utils/split-css-value.js | 87 +++++++++++-------- 3 files changed, 74 insertions(+), 42 deletions(-) diff --git a/packages/shared/__tests__/split-value-test.js b/packages/shared/__tests__/split-value-test.js index 89303d8d1..3bf2b1fcc 100644 --- a/packages/shared/__tests__/split-value-test.js +++ b/packages/shared/__tests__/split-value-test.js @@ -45,12 +45,27 @@ describe('Ensure CSS values are split correctly', () => { ).toEqual(['calc((100% - 50px) * 0.5)', 'var(--rightpadding,20px)']); }); - test('Splits a string of values with slash notation appropriately.', () => { - expect(splitValue('1px/2px 3px 4px 5px')).toEqual([ - '1px/2px', - '3px', - '4px', - '5px', + test('Expands a string of values with slash notation appropriately.', () => { + expect(splitValue('1px / 2px 3px 4px 5px', 'borderRadius')).toEqual([ + '1px 2px', + '1px 3px', + '1px 4px', + '1px 5px', ]); + expect(splitValue('1px 2px / 3px 4px', 'borderRadius')).toEqual([ + '1px 3px', + '2px 4px', + '1px 3px', + '2px 4px', + ]); + expect(splitValue('1px 2px / 3px 4px 5px', 'borderRadius')).toEqual([ + '1px 3px', + '2px 4px', + '1px 5px', + '2px 4px', + ]); + expect( + splitValue('1px 2px 3px 4px / 5px 6px 7px 8px', 'borderRadius'), + ).toEqual(['1px 5px', '2px 6px', '3px 7px', '4px 8px']); }); }); diff --git a/packages/shared/src/preprocess-rules/legacy-expand-shorthands.js b/packages/shared/src/preprocess-rules/legacy-expand-shorthands.js index 0f6770076..30f25cf77 100644 --- a/packages/shared/src/preprocess-rules/legacy-expand-shorthands.js +++ b/packages/shared/src/preprocess-rules/legacy-expand-shorthands.js @@ -162,7 +162,7 @@ const shorthands: $ReadOnly<{ [key: string]: (TStyleValue) => TReturn }> = { ], borderRadius: (rawValue: TStyleValue): TReturn => { - const [top, right = top, bottom = top, left = right] = splitValue(rawValue); + const [top, right = top, bottom = top, left = right] = splitValue(rawValue, 'borderRadius'); return [ ['borderTopStartRadius', top], diff --git a/packages/shared/src/utils/split-css-value.js b/packages/shared/src/utils/split-css-value.js index 756a31c55..e72a0d582 100644 --- a/packages/shared/src/utils/split-css-value.js +++ b/packages/shared/src/utils/split-css-value.js @@ -23,47 +23,47 @@ function printNode(node: PostCSSValueASTNode): string { } } -// Merges slash-separated values within nodes into single nodes. -function combineNodesWithSlash(nodes: PostCSSValueASTNode[]) { +// Splits PostCSS value nodes for border-radius into horizontal and vertical groups by slash. +function splitNodesBySlash(nodes: PostCSSValueASTNode[]): PostCSSValueASTNode[][] { const result = []; - - for (let i = 0; i < nodes.length; i++) { - const node = nodes[i]; - - if (node.type !== 'div' || node.value !== '/') { - result.push(node); - continue; - } - - if (i === 0 || i === nodes.length - 1) { - result.push(node); - continue; - } - - const prev = result.at(-1); - const next = nodes[i + 1]; - - if (!prev || !next || prev.type !== 'word' || next.type !== 'word') { - result.push(node); - continue; + let current = []; + + for (const node of nodes) { + const isSeparator = node.type === 'div' && node.value === '/'; + if (isSeparator) { + if (current.length > 0) { + result.push(current); + current = []; + } + } else { + current.push(node); } + } - result.pop(); - const combinedNode = { - ...prev, - value: prev.value + node.value + next.value, - sourceEndIndex: next.sourceEndIndex, - }; - result.push(combinedNode); - i++; + if (current.length > 0) { + result.push(current); } return result; } +// Expands a border-radius shorthand value to an array of four values. +function expandBorderRadiusShorthand(group: PostCSSValueASTNode[]) { + if (group.length === 2) return [group[0], group[1], group[0], group[1]]; + if (group.length === 3) return [group[0], group[1], group[2], group[1]]; + if (group.length === 4) return [group[0], group[1], group[2], group[3]]; + return Array(4).fill(group[0]); +} + +// Combines two arrays of border-radius values into a single formatted string. +function combineBorderRadiusValues(verticals: string[], horizontals: string[]) { + return verticals.map((value, i) => `${value} ${horizontals[i]}`); +} + // Using split(' ') Isn't enough because of values like calc. export default function splitValue( str: TStyleValue, + propertyName: string = '', ): $ReadOnlyArray { if (str == null || typeof str === 'number') { return [str]; @@ -76,11 +76,28 @@ export default function splitValue( const parsed = parser(str.trim()); - const nodes = combineNodesWithSlash( - parsed.nodes.filter((node) => node.type !== 'space'), - ) - .filter((node) => node.type !== 'div') - .map(printNode); + let nodes: string[] = []; + if (propertyName === 'borderRadius') { + const groups = splitNodesBySlash( + parsed.nodes.filter((node) => node.type !== 'space'), + ); + if (groups.length === 1) { + nodes = parsed.nodes.filter((node) => node.type !== 'div').map(printNode); + } else { + // edge case + const vertical = expandBorderRadiusShorthand( + groups[0].filter((node) => node.type !== 'div'), + ).map(printNode); + const horizontal = expandBorderRadiusShorthand( + groups[1].filter((node) => node.type !== 'div'), + ).map(printNode); + nodes = combineBorderRadiusValues(vertical, horizontal); + } + } else { + nodes = parsed.nodes + .filter((node) => node.type !== 'space' && node.type !== 'div') + .map(printNode); + } if ( nodes.length > 1 && From 7ec19e49a2d7fef09214c58349c8fcead6d221c9 Mon Sep 17 00:00:00 2001 From: Yasuhiro Yamamoto Date: Sat, 18 Jan 2025 23:49:45 +0900 Subject: [PATCH 5/5] chore: Format with Prettier --- .../shared/src/preprocess-rules/legacy-expand-shorthands.js | 5 ++++- packages/shared/src/utils/split-css-value.js | 4 +++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/shared/src/preprocess-rules/legacy-expand-shorthands.js b/packages/shared/src/preprocess-rules/legacy-expand-shorthands.js index 30f25cf77..33920930e 100644 --- a/packages/shared/src/preprocess-rules/legacy-expand-shorthands.js +++ b/packages/shared/src/preprocess-rules/legacy-expand-shorthands.js @@ -162,7 +162,10 @@ const shorthands: $ReadOnly<{ [key: string]: (TStyleValue) => TReturn }> = { ], borderRadius: (rawValue: TStyleValue): TReturn => { - const [top, right = top, bottom = top, left = right] = splitValue(rawValue, 'borderRadius'); + const [top, right = top, bottom = top, left = right] = splitValue( + rawValue, + 'borderRadius', + ); return [ ['borderTopStartRadius', top], diff --git a/packages/shared/src/utils/split-css-value.js b/packages/shared/src/utils/split-css-value.js index e72a0d582..602460505 100644 --- a/packages/shared/src/utils/split-css-value.js +++ b/packages/shared/src/utils/split-css-value.js @@ -24,7 +24,9 @@ function printNode(node: PostCSSValueASTNode): string { } // Splits PostCSS value nodes for border-radius into horizontal and vertical groups by slash. -function splitNodesBySlash(nodes: PostCSSValueASTNode[]): PostCSSValueASTNode[][] { +function splitNodesBySlash( + nodes: PostCSSValueASTNode[], +): PostCSSValueASTNode[][] { const result = []; let current = [];