Don't ignore whitespace in expressions (#930)

* add tests * do print expressions with comments only * don't remove whitespace in expressions * fix whitespace in tests * test to ignore empty expressions with whitespace * ignore empty expressions with whitespace * remove test relating to an unaffected code path * chore: changeset
withastro · Jan 2, 2024 · 4de359b · 4de359b
1 parent 9ff6342
commit 4de359b
Show file tree

Hide file tree

Showing 7 changed files with 70 additions and 14 deletions.
diff --git a/.changeset/poor-cheetahs-talk.md b/.changeset/poor-cheetahs-talk.md
@@ -0,0 +1,5 @@
+---
+'@astrojs/compiler': patch
+---
+
+Preserve whitespace in expressions
diff --git a/internal/parser.go b/internal/parser.go
@@ -342,11 +342,6 @@ func (p *parser) addText(text string) {
 		return
 	}
 
-	// Inside of expressions we can skip whitespace
-	if p.top().Expression && strings.TrimSpace(text) == "" {
-		return
-	}
-
 	t := p.top()
 	if n := t.LastChild; n != nil && n.Type == TextNode {
 		n.Data += text

diff --git a/internal/printer/print-to-js.go b/internal/printer/print-to-js.go
@@ -316,7 +316,11 @@ func render1(p *printer, n *Node, opts RenderOptions) {
 
 	// Tip! Comment this block out to debug expressions
 	if n.Expression {
-		if n.FirstChild == nil {
+		clean := ""
+		if n.FirstChild != nil {
+			clean = strings.TrimSpace(n.FirstChild.Data)
+		}
+		if n.FirstChild == nil || clean == "" {
 			p.print("${(void 0)")
 		} else if expressionOnlyHasComment(n) {
 			// we do not print expressions that only contain comment blocks

diff --git a/internal/printer/print-to-tsx.go b/internal/printer/print-to-tsx.go
@@ -262,9 +262,6 @@ declare const Astro: Readonly<import('astro').AstroGlobal<%s, typeof %s`, propsI
 		p.addSourceMapping(n.Loc[0])
 		if n.FirstChild == nil {
 			p.print("{(void 0)")
-		} else if expressionOnlyHasComment(n) {
-			// we do not print expressions that only contain comment blocks
-			return
 		} else {
 			p.print("{")
 		}

diff --git a/internal/printer/printer_test.go b/internal/printer/printer_test.go
@@ -821,7 +821,8 @@ const groups = [[0, 1, 2], [3, 4, 5]];
 			items.map(item => {
 				return %s<li>${item}</li>%s;
 			})
-		}</ul>%s})}
+		}</ul>%s
+	})}
 </div>`, "$$render"+BACKTICK, "$$render"+BACKTICK, BACKTICK, BACKTICK),
 			},
 		},
@@ -871,7 +872,7 @@ const groups = [[0, 1, 2], [3, 4, 5]];
 			name:   "nested expressions IV",
 			source: `<div>{() => { if (value > 0.25) { return <span>Default</span> } else if (value > 0.5) { return <span>Another</span> } else if (value > 0.75) { return <span>Other</span> } return <span>Yet Other</span> }}</div>`,
 			want: want{
-				code: "${$$maybeRenderHead($$result)}<div>${() => { if (value > 0.25) { return $$render`<span>Default</span>`} else if (value > 0.5) { return $$render`<span>Another</span>`} else if (value > 0.75) { return $$render`<span>Other</span>`} return $$render`<span>Yet Other</span>`}}</div>",
+				code: "${$$maybeRenderHead($$result)}<div>${() => { if (value > 0.25) { return $$render`<span>Default</span>` } else if (value > 0.5) { return $$render`<span>Another</span>` } else if (value > 0.75) { return $$render`<span>Other</span>` } return $$render`<span>Yet Other</span>` }}</div>",
 			},
 		},
 		{
@@ -921,10 +922,10 @@ const items = ['red', 'yellow', 'blue'];
 				code: `${$$maybeRenderHead($$result)}<div>
   ${items.map((item) => (
     // foo < > < }
-$$render` + "`" + `<div${$$addAttribute(color, "id")}>color</div>` + "`" + `
+    $$render` + "`" + `<div${$$addAttribute(color, "id")}>color</div>` + "`" + `
   ))}
   ${items.map((item) => (
-    /* foo < > < } */$$render` + "`" + `<div${$$addAttribute(color, "id")}>color</div>` + "`" + `
+    /* foo < > < } */ $$render` + "`" + `<div${$$addAttribute(color, "id")}>color</div>` + "`" + `
   ))}
 </div>`,
 			},
@@ -2311,6 +2312,13 @@ const items = ["Dog", "Cat", "Platipus"];
 				code: `${$$maybeRenderHead($$result)}<body>(${(void 0)})</body>`,
 			},
 		},
+		{
+			name:   "Empty expression with whitespace",
+			source: "<body>({   })</body>",
+			want: want{
+				code: `${$$maybeRenderHead($$result)}<body>(${(void 0)   })</body>`,
+			},
+		},
 		{
 			name:   "Empty attribute expression",
 			source: "<body attr={}></body>",

diff --git a/packages/compiler/test/basic/expression-then-node.ts b/packages/compiler/test/basic/expression-then-node.ts
@@ -38,7 +38,7 @@ test('expression followed by node', () => {
     result.code,
     `yield '
 ';
-}`,
+		}`,
     'Expected output to properly handle expression!'
   );
 });

diff --git a/packages/compiler/test/tsx/comment-whitespace.ts b/packages/compiler/test/tsx/comment-whitespace.ts
@@ -0,0 +1,47 @@
+import { convertToTSX } from '@astrojs/compiler';
+import { test } from 'uvu';
+import * as assert from 'uvu/assert';
+
+test('preverve whitespace around jsx comments', async () => {
+  const input = `{/* @ts-expect-error */}
+<Component prop="value"></Component>
+
+{
+// @ts-expect-error
+}
+<Component prop="value"></Component>
+
+{
+/* @ts-expect-error */
+<Component prop="value"></Component>
+}
+
+{
+// @ts-expect-error
+<Component prop="value"></Component>
+}`;
+  const output = `<Fragment>
+{/* @ts-expect-error */}
+<Component prop="value"></Component>
+
+{
+// @ts-expect-error
+}
+<Component prop="value"></Component>
+
+{
+/* @ts-expect-error */
+<Fragment><Component prop="value"></Component></Fragment>
+}
+
+{
+// @ts-expect-error
+<Fragment><Component prop="value"></Component></Fragment>
+}
+</Fragment>
+export default function __AstroComponent_(_props: Record<string, any>): any {}\n`;
+  const { code } = await convertToTSX(input, { sourcemap: 'external' });
+  assert.snapshot(code, output, `expected code to match snapshot`);
+});
+
+test.run();
-Original file line number
+Diff line change
@@ Expand Up / @@ -38,7 +38,7 @@ test('expression followed by node', () => { @@
         result.code,
         `yield '
     ';
-    }`,
+    		}`,
         'Expected output to properly handle expression!'
       );
     });
@@ Expand Down @@