Skip to content

Commit

Permalink
Prevent render tag for whitespace-only text nodes (#932)
Browse files Browse the repository at this point in the history
* don't print render for whitespace-only text nodes

* oops

* update whitespace in tests

* update whitespace in tests

* chore: changeset

* add missed updated test

* fix edge case

* chore: more descriptive comments

* chore: reformulate comments for clarity
  • Loading branch information
MoustaphaDev authored Jan 4, 2024
1 parent 9d74427 commit 24e2886
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 8 deletions.
5 changes: 5 additions & 0 deletions .changeset/hot-clocks-judge.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@astrojs/compiler': patch
---

Fixes a regression that caused whitespace between elements in an expression to result invalid code
15 changes: 13 additions & 2 deletions internal/printer/print-to-js.go
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,12 @@ func render1(p *printer, n *Node, opts RenderOptions) {
p.printTextWithSourcemap(c.Data, c.Loc[0])
continue
}
if c.PrevSibling == nil || c.PrevSibling.Type == TextNode {
// Print the opening of a tagged render function before
// a node, only when it meets either of these conditions:
// - It does not have a previous sibling.
// - It has a text node that contains more than just whitespace.
// - It is the first child of its parent expression.
if c.PrevSibling == nil || c.PrevSibling == n.FirstChild || (c.PrevSibling.Type == TextNode && strings.TrimSpace(c.PrevSibling.Data) != "") {
p.printTemplateLiteralOpen()
}
render1(p, c, RenderOptions{
Expand All @@ -346,7 +351,13 @@ func render1(p *printer, n *Node, opts RenderOptions) {
cssLen: opts.cssLen,
printedMaybeHead: opts.printedMaybeHead,
})
if c.NextSibling == nil || c.NextSibling.Type == TextNode {

// Print the closing of a tagged render function after
// a node, only when it meets either of these conditions:
// - It does not have a next sibling.
// - It has a text node that contains more than just whitespace.
// - It is the last child of its parent expression.
if c.NextSibling == nil || c.NextSibling == n.LastChild || (c.NextSibling.Type == TextNode && strings.TrimSpace(c.NextSibling.Data) != "") {
p.printTemplateLiteralClose()
}
}
Expand Down
14 changes: 9 additions & 5 deletions internal/printer/printer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -527,13 +527,17 @@ import type data from "test"
return (
$$render` + BACKTICK + `<p>
onlyp ${dummyKey}
</p><h2>
</p>
<h2>
onlyh2 ${dummyKey}
</h2><div>
</h2>
<div>
<h2>div+h2 ${dummyKey}</h2>
</div><p>
</p><h2>p+h2 ${dummyKey}</h2>` + BACKTICK + `
);
</div>
<p>
</p><h2>p+h2 ${dummyKey}</h2>
` + BACKTICK + `
);
})
}
</main>
Expand Down
3 changes: 2 additions & 1 deletion packages/compiler/test/tsx/comment-whitespace.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { convertToTSX } from '@astrojs/compiler';
import { test } from 'uvu';
import * as assert from 'uvu/assert';
import { TSXPrefix } from '../utils';

test('preverve whitespace around jsx comments', async () => {
const input = `{/* @ts-expect-error */}
Expand All @@ -20,7 +21,7 @@ test('preverve whitespace around jsx comments', async () => {
// @ts-expect-error
<Component prop="value"></Component>
}`;
const output = `<Fragment>
const output = `${TSXPrefix}<Fragment>
{/* @ts-expect-error */}
<Component prop="value"></Component>
Expand Down

0 comments on commit 24e2886

Please sign in to comment.