From 4dd07a237321093d8d8c3175458344060ac88d33 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Sun, 18 Jan 2026 21:22:55 +0100 Subject: [PATCH 1/2] Rewriter: Preserve interpolated ERB class names in tailwind rewriter --- .../src/built-ins/tailwind-class-sorter.ts | 70 ++++++++++++++++ .../built-ins/tailwind-class-sorter.test.ts | 81 +++++++++++++++++++ 2 files changed, 151 insertions(+) diff --git a/javascript/packages/rewriter/src/built-ins/tailwind-class-sorter.ts b/javascript/packages/rewriter/src/built-ins/tailwind-class-sorter.ts index 156710bf0..0a0daf4b5 100644 --- a/javascript/packages/rewriter/src/built-ins/tailwind-class-sorter.ts +++ b/javascript/packages/rewriter/src/built-ins/tailwind-class-sorter.ts @@ -135,7 +135,77 @@ class TailwindClassSorterVisitor extends Visitor { return isLiteralNode(node) && !node.content.trim() } + private containsStringInterpolation(nodes: Node[]): boolean { + for (let i = 0; i < nodes.length - 1; i++) { + const current = nodes[i] + const next = nodes[i + 1] + + if (isLiteralNode(current) && !isLiteralNode(next)) { + const literalContent = current.content + + if (literalContent.length > 0 && !/\s$/.test(literalContent)) { + for (let j = i + 1; j < nodes.length; j++) { + const lookAhead = nodes[j] + + if (isLiteralNode(lookAhead)) { + const lookAheadContent = lookAhead.content + + if (lookAheadContent.length > 0 && !/^\s/.test(lookAheadContent)) { + return true + } + + break + } + } + } + } + + // "<%= prefix %>-blue-500" + if (i === 0 && !isLiteralNode(current) && isLiteralNode(next)) { + const nextContent = next.content + + if (nextContent.length > 0 && !/^\s/.test(nextContent)) { + return true + } + } + + // "bg-<%= suffix %>" + if (isLiteralNode(current) && !isLiteralNode(next)) { + const literalContent = current.content + + if (literalContent.length > 0 && /-$/.test(literalContent)) { + let hasLiteralAfter = false + + for (let j = i + 1; j < nodes.length; j++) { + const node = nodes[j] + + if (isLiteralNode(node) && node.content.trim()) { + hasLiteralAfter = true + break + } + } + + if (!hasLiteralAfter) { + return true + } + } + } + } + + return false + } + private formatNodes(nodes: Node[], isNested: boolean): Node[] { + if (this.containsStringInterpolation(nodes)) { + for (const node of nodes) { + if (!isLiteralNode(node)) { + this.visit(node) + } + } + + return nodes + } + const { classLiterals, others } = this.partitionNodes(nodes) const preserveLeadingSpace = isNested || this.startsWithClassLiteral(nodes) diff --git a/javascript/packages/rewriter/test/built-ins/tailwind-class-sorter.test.ts b/javascript/packages/rewriter/test/built-ins/tailwind-class-sorter.test.ts index 9c0502c70..a42de2953 100644 --- a/javascript/packages/rewriter/test/built-ins/tailwind-class-sorter.test.ts +++ b/javascript/packages/rewriter/test/built-ins/tailwind-class-sorter.test.ts @@ -115,6 +115,87 @@ describe("tailwind-class-sorter", () => { }) }) + describe("ERB string interpolation within class names", () => { + test("preserves interpolation in middle of class name (issue #879)", async () => { + await expectNoTransform( + `
` + ) + }) + + test("preserves interpolation at start of class name", async () => { + await expectNoTransform( + `
` + ) + }) + + test("preserves interpolation at end of class name", async () => { + await expectNoTransform( + `
` + ) + }) + + test("preserves interpolation with multiple ERB tags in middle", async () => { + await expectNoTransform( + `
` + ) + }) + + test("preserves multiple ERB interpolations building one class name", async () => { + await expectNoTransform( + `
` + ) + }) + + test("preserves complex interpolation with prefix, middle, and suffix", async () => { + await expectNoTransform( + `
` + ) + }) + + test("preserves interpolation with static prefix and multiple dynamic parts", async () => { + await expectNoTransform( + `
` + ) + }) + + test("preserves interpolation with dynamic prefix and static suffix", async () => { + await expectNoTransform( + `
` + ) + }) + + test("preserves fully dynamic class with hyphens", async () => { + await expectNoTransform( + `
` + ) + }) + + test("preserves multiple interpolated class names", async () => { + await expectNoTransform( + `
` + ) + }) + + test("preserves multiple different interpolation patterns in same attribute", async () => { + await expectNoTransform( + `
` + ) + }) + + test("preserves class with interpolation mixed with static classes", async () => { + await expectNoTransform( + `
` + ) + }) + + test("still sorts classes inside conditionals when interpolation is in main attribute", async () => { + await expectTransform( + `
`, + `
` + ) + }) + }) + describe("ERB nodes in class attributes", () => { test("moves ERB output node from middle to end", async () => { await expectTransform( From 44a5d0ad1914a8a351c01b9a66f341635a2bfd20 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Sun, 18 Jan 2026 22:54:30 +0100 Subject: [PATCH 2/2] Try formatting them in groups --- .../src/built-ins/tailwind-class-sorter.ts | 311 +++++++++++------- .../built-ins/tailwind-class-sorter.test.ts | 23 +- .../packages/rewriter/test/rewrite.test.ts | 9 +- 3 files changed, 211 insertions(+), 132 deletions(-) diff --git a/javascript/packages/rewriter/src/built-ins/tailwind-class-sorter.ts b/javascript/packages/rewriter/src/built-ins/tailwind-class-sorter.ts index 0a0daf4b5..c907f96ea 100644 --- a/javascript/packages/rewriter/src/built-ins/tailwind-class-sorter.ts +++ b/javascript/packages/rewriter/src/built-ins/tailwind-class-sorter.ts @@ -45,7 +45,23 @@ class TailwindClassSorterVisitor extends Visitor { const attributeName = getStaticAttributeName(node.name) if (attributeName !== "class") return - this.visit(node.value) + const classAttributeSorter = new ClassAttributeSorter(this.sorter) + + classAttributeSorter.visit(node.value) + } +} + +/** + * Visitor that sorts classes within a single class attribute value. + * Only operates on the content of a class attribute, not the full document. + */ +class ClassAttributeSorter extends Visitor { + private sorter: TailwindClassSorter + + constructor(sorter: TailwindClassSorter) { + super() + + this.sorter = sorter } visitHTMLAttributeValueNode(node: HTMLAttributeValueNode): void { @@ -127,175 +143,244 @@ class TailwindClassSorterVisitor extends Visitor { }) } - private startsWithClassLiteral(nodes: Node[]): boolean { - return nodes.length > 0 && isLiteralNode(nodes[0]) && !!nodes[0].content.trim() - } - private isWhitespaceLiteral(node: Node): boolean { return isLiteralNode(node) && !node.content.trim() } - private containsStringInterpolation(nodes: Node[]): boolean { - for (let i = 0; i < nodes.length - 1; i++) { - const current = nodes[i] - const next = nodes[i + 1] + private splitLiteralsAtWhitespace(nodes: Node[]): Node[] { + const result: Node[] = [] + + for (const node of nodes) { + if (isLiteralNode(node)) { + const parts = node.content.match(/(\S+|\s+)/g) || [] + + for (const part of parts) { + result.push(new LiteralNode({ + type: "AST_LITERAL_NODE", + content: part, + errors: [], + location: node.location + })) + } + } else { + result.push(node) + } + } + + return result + } + + private groupNodesByClass(nodes: Node[]): Node[][] { + if (nodes.length === 0) return [] - if (isLiteralNode(current) && !isLiteralNode(next)) { - const literalContent = current.content + const groups: Node[][] = [] + let currentGroup: Node[] = [] - if (literalContent.length > 0 && !/\s$/.test(literalContent)) { - for (let j = i + 1; j < nodes.length; j++) { - const lookAhead = nodes[j] + for (let i = 0; i < nodes.length; i++) { + const node = nodes[i] + const previousNode = i > 0 ? nodes[i - 1] : null - if (isLiteralNode(lookAhead)) { - const lookAheadContent = lookAhead.content + let startNewGroup = false - if (lookAheadContent.length > 0 && !/^\s/.test(lookAheadContent)) { - return true - } + if (currentGroup.length === 0) { + startNewGroup = false + } else if (isLiteralNode(node)) { + if (/^\s/.test(node.content)) { + startNewGroup = true + } else if (/^-/.test(node.content)) { + startNewGroup = false + } else if (previousNode && !isLiteralNode(previousNode)) { + startNewGroup = true + } - break - } + } else { + if (previousNode && isLiteralNode(previousNode)) { + if (/\s$/.test(previousNode.content)) { + startNewGroup = true + } else if (/-$/.test(previousNode.content)) { + startNewGroup = false + } else { + startNewGroup = true } + + } else if (previousNode && !isLiteralNode(previousNode)) { + startNewGroup = false } } - // "<%= prefix %>-blue-500" - if (i === 0 && !isLiteralNode(current) && isLiteralNode(next)) { - const nextContent = next.content + if (startNewGroup && currentGroup.length > 0) { + groups.push(currentGroup) - if (nextContent.length > 0 && !/^\s/.test(nextContent)) { - return true - } + currentGroup = [] } - // "bg-<%= suffix %>" - if (isLiteralNode(current) && !isLiteralNode(next)) { - const literalContent = current.content + currentGroup.push(node) + } - if (literalContent.length > 0 && /-$/.test(literalContent)) { - let hasLiteralAfter = false + if (currentGroup.length > 0) { + groups.push(currentGroup) + } - for (let j = i + 1; j < nodes.length; j++) { - const node = nodes[j] + return groups + } - if (isLiteralNode(node) && node.content.trim()) { - hasLiteralAfter = true - break - } - } + private isInterpolatedGroup(group: Node[]): boolean { + return group.some(node => !isLiteralNode(node)) + } - if (!hasLiteralAfter) { - return true - } - } - } - } + private isWhitespaceGroup(group: Node[]): boolean { + return group.every(node => this.isWhitespaceLiteral(node)) + } - return false + private getStaticClassContent(group: Node[]): string { + return group + .filter(node => isLiteralNode(node)) + .map(node => (node as LiteralNode).content) + .join("") } private formatNodes(nodes: Node[], isNested: boolean): Node[] { - if (this.containsStringInterpolation(nodes)) { - for (const node of nodes) { - if (!isLiteralNode(node)) { - this.visit(node) - } - } + if (nodes.length === 0) return nodes + if (nodes.every(n => this.isWhitespaceLiteral(n))) return nodes - return nodes - } + const splitNodes = this.splitLiteralsAtWhitespace(nodes) + const groups = this.groupNodesByClass(splitNodes) - const { classLiterals, others } = this.partitionNodes(nodes) - const preserveLeadingSpace = isNested || this.startsWithClassLiteral(nodes) + const staticClasses: string[] = [] + const interpolationGroups: Node[][] = [] + const standaloneERBNodes: Node[] = [] - return this.formatSortedClasses(classLiterals, others, preserveLeadingSpace, isNested) - } + for (const group of groups) { + if (this.isWhitespaceGroup(group)) { + continue + } - private partitionNodes(nodes: Node[]): { classLiterals: LiteralNode[], others: Node[] } { - const classLiterals: LiteralNode[] = [] - const others: Node[] = [] + if (this.isInterpolatedGroup(group)) { + const hasAttachedLiteral = group.some(node => isLiteralNode(node) && node.content.trim()) - for (const node of nodes) { - if (isLiteralNode(node)) { - if (node.content.trim()) { - classLiterals.push(node) + if (hasAttachedLiteral) { + for (const node of group) { + if (!isLiteralNode(node)) { + this.visit(node) + } + } + + interpolationGroups.push(group) } else { - others.push(node) + for (const node of group) { + if (!isLiteralNode(node)) { + this.visit(node) + standaloneERBNodes.push(node) + } + } } } else { - this.visit(node) - others.push(node) + const content = this.getStaticClassContent(group).trim() + + if (content) { + staticClasses.push(content) + } } } - return { classLiterals, others } - } + const allStaticContent = staticClasses.join(" ") + let sortedContent = allStaticContent + + if (allStaticContent) { + try { + sortedContent = this.sorter.sortClasses(allStaticContent) + } catch { + // Keep original on error + } + } - private formatSortedClasses(literals: LiteralNode[], others: Node[], preserveLeadingSpace: boolean, isNested: boolean): Node[] { - if (literals.length === 0 && others.length === 0) return [] - if (literals.length === 0) return others + let addedLeadingSpace = false - const fullContent = literals.map(n => n.content).join("") - const trimmedClasses = fullContent.trim() + const result: Node[] = [] + const hasContent = sortedContent || interpolationGroups.length > 0 || standaloneERBNodes.length > 0 + const needsLeadingSpace = isNested && hasContent - if (!trimmedClasses) return others.length > 0 ? others : [] + if (sortedContent) { + const literal = new LiteralNode({ + type: "AST_LITERAL_NODE", + content: (needsLeadingSpace ? " " : "") + sortedContent, + errors: [], + location: Location.zero + }) - try { - const sortedClasses = this.sorter.sortClasses(trimmedClasses) + result.push(literal) + + addedLeadingSpace = !!needsLeadingSpace + } - if (others.length === 0) { - return this.formatSortedLiteral(literals[0], fullContent, sortedClasses, trimmedClasses) + for (const group of interpolationGroups) { + if (result.length > 0) { + result.push(this.spaceLiteral) + } else if (needsLeadingSpace && !addedLeadingSpace) { + result.push(this.spaceLiteral) + addedLeadingSpace = true } - return this.formatSortedLiteralWithERB(literals[0], fullContent, sortedClasses, others, preserveLeadingSpace, isNested) - } catch (error) { - return [...literals, ...others] - } - } + const trimmedGroup = this.trimGroupWhitespace(group) - private formatSortedLiteral(literal: LiteralNode, fullContent: string, sortedClasses: string, trimmedClasses: string): Node[] { - const leadingSpace = fullContent.match(/^\s*/)?.[0] || "" - const trailingSpace = fullContent.match(/\s*$/)?.[0] || "" - const alreadySorted = sortedClasses === trimmedClasses + result.push(...trimmedGroup) + } - const sortedContent = alreadySorted ? fullContent : (leadingSpace + sortedClasses + trailingSpace) + for (const node of standaloneERBNodes) { + if (result.length > 0) { + result.push(this.spaceLiteral) + } else if (needsLeadingSpace && !addedLeadingSpace) { + result.push(this.spaceLiteral) + addedLeadingSpace = true + } + result.push(node) + } - asMutable(literal).content = sortedContent + if (isNested && result.length > 0) { + result.push(this.spaceLiteral) + } - return [literal] + return result } - private formatSortedLiteralWithERB(literal: LiteralNode, fullContent: string, sortedClasses: string, others: Node[], preserveLeadingSpace: boolean, isNested: boolean): Node[] { - const leadingSpace = fullContent.match(/^\s*/)?.[0] || "" - const trailingSpace = fullContent.match(/\s*$/)?.[0] || "" + private trimGroupWhitespace(group: Node[]): Node[] { + if (group.length === 0) return group - const leading = preserveLeadingSpace ? leadingSpace : "" - const firstIsWhitespace = this.isWhitespaceLiteral(others[0]) - const spaceBetween = firstIsWhitespace ? "" : " " + const result = [...group] - asMutable(literal).content = leading + sortedClasses + spaceBetween + if (isLiteralNode(result[0])) { + const first = result[0] as LiteralNode + const trimmed = first.content.trimStart() - const othersWithWhitespace = this.addSpacingBetweenERBNodes(others, isNested, trailingSpace) + if (trimmed !== first.content) { + result[0] = new LiteralNode({ + type: "AST_LITERAL_NODE", + content: trimmed, + errors: [], + location: first.location + }) + } + } - return [literal, ...othersWithWhitespace] - } + const lastIndex = result.length - 1 - private addSpacingBetweenERBNodes(nodes: Node[], isNested: boolean, trailingSpace: string): Node[] { - return nodes.flatMap((node, index) => { - const isLast = index >= nodes.length - 1 + if (isLiteralNode(result[lastIndex])) { + const last = result[lastIndex] as LiteralNode + const trimmed = last.content.trimEnd() - if (isLast) { - return isNested && trailingSpace ? [node, this.spaceLiteral] : [node] + if (trimmed !== last.content) { + result[lastIndex] = new LiteralNode({ + type: "AST_LITERAL_NODE", + content: trimmed, + errors: [], + location: last.location + }) } + } - const currentIsWhitespace = this.isWhitespaceLiteral(node) - const nextIsWhitespace = this.isWhitespaceLiteral(nodes[index + 1]) - const needsSpace = !currentIsWhitespace && !nextIsWhitespace - - return needsSpace ? [node, this.spaceLiteral] : [node] - }) + return result } + } /** diff --git a/javascript/packages/rewriter/test/built-ins/tailwind-class-sorter.test.ts b/javascript/packages/rewriter/test/built-ins/tailwind-class-sorter.test.ts index a42de2953..1b3cd5d11 100644 --- a/javascript/packages/rewriter/test/built-ins/tailwind-class-sorter.test.ts +++ b/javascript/packages/rewriter/test/built-ins/tailwind-class-sorter.test.ts @@ -182,9 +182,17 @@ describe("tailwind-class-sorter", () => { ) }) - test("preserves class with interpolation mixed with static classes", async () => { - await expectNoTransform( - `
` + test("sorts static classes and moves multiple interpolations to end", async () => { + await expectTransform( + `
`, + `
` + ) + }) + + test("sorts static classes and moves interpolation to end", async () => { + await expectTransform( + `
`, + `
` ) }) @@ -326,14 +334,7 @@ describe("tailwind-class-sorter", () => { rounded"> `, - dedent` -
-
- ` + `
\n
` ) }) diff --git a/javascript/packages/rewriter/test/rewrite.test.ts b/javascript/packages/rewriter/test/rewrite.test.ts index 54c1d6ade..266be5184 100644 --- a/javascript/packages/rewriter/test/rewrite.test.ts +++ b/javascript/packages/rewriter/test/rewrite.test.ts @@ -128,15 +128,8 @@ describe("rewrite", () => { rounded"> ` - const expected = dedent` -
-
- ` + const expected = `
\n
` const output = rewriteString(Herb, template, [sorter]) expect(output).toBe(expected)