Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix content treated as string after JSX comment inside expression #895

Merged
merged 19 commits into from
Dec 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/lazy-beers-develop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@astrojs/compiler': patch
---

Fixes an issue where HTML and JSX comments lead to subsequent content being incorrectly treated as plain text when they have parent expressions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,5 @@ node_modules
*.wasm
/astro
debug.test
__debug_bin
packages/compiler/sourcemap.mjs
45 changes: 45 additions & 0 deletions internal/helpers/js_comment_utils.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package helpers

import (
"strings"
)

func peekIs(input string, cur int, assert byte) bool {
return cur+1 < len(input) && input[cur+1] == assert
}

// RemoveComments removes both block and inline comments from a string
func RemoveComments(input string) string {
var (
sb = strings.Builder{}
inComment = false
)
for cur := 0; cur < len(input); cur++ {
if input[cur] == '/' && !inComment {
if peekIs(input, cur, '*') {
inComment = true
cur++
} else if peekIs(input, cur, '/') {
// Skip until the end of line for inline comments
for cur < len(input) && input[cur] != '\n' {
cur++
}
continue
}
} else if input[cur] == '*' && inComment && peekIs(input, cur, '/') {
inComment = false
cur++
continue
}

if !inComment {
sb.WriteByte(input[cur])
}
}

if inComment {
return ""
}

return strings.TrimSpace(sb.String())
}
7 changes: 7 additions & 0 deletions internal/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -2785,6 +2785,13 @@ func inExpressionIM(p *parser) bool {
p.oe.pop()
p.im = textIM
return true
case CommentToken:
p.addChild(&Node{
Type: CommentNode,
Data: p.tok.Data,
Loc: p.generateLoc(),
})
return true
}
p.im = p.originalIM
p.originalIM = nil
Expand Down
27 changes: 18 additions & 9 deletions internal/printer/print-to-js.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

. "github.com/withastro/compiler/internal"
"github.com/withastro/compiler/internal/handler"
"github.com/withastro/compiler/internal/helpers"
"github.com/withastro/compiler/internal/js_scanner"
"github.com/withastro/compiler/internal/loc"
"github.com/withastro/compiler/internal/sourcemap"
Expand Down Expand Up @@ -87,13 +88,18 @@ func printToJs(p *printer, n *Node, cssLen int, opts transform.TransformOptions)
const whitespace = " \t\r\n\f"

// Returns true if the expression only contains a comment block (e.g. {/* a comment */})
func expressionOnlyHasCommentBlock(n *Node) bool {
clean, _ := removeComments(n.FirstChild.Data)
return n.FirstChild.NextSibling == nil &&
func expressionOnlyHasComment(n *Node) bool {
if n.FirstChild == nil {
return false
}
clean := helpers.RemoveComments(n.FirstChild.Data)
trimmedData := strings.TrimLeft(n.FirstChild.Data, whitespace)
result := n.FirstChild.NextSibling == nil &&
n.FirstChild.Type == TextNode &&
// removeComments iterates over text and most of the time we won't be parsing comments so lets check if text starts with /* before iterating
strings.HasPrefix(strings.TrimLeft(n.FirstChild.Data, whitespace), "/*") &&
// RemoveComments iterates over text and most of the time we won't be parsing comments so lets check if text starts with /* or // before iterating
(strings.HasPrefix(trimmedData, "/*") || strings.HasPrefix(trimmedData, "//")) &&
len(clean) == 0
return result
}

func emptyTextNodeWithoutSiblings(n *Node) bool {
Expand Down Expand Up @@ -311,7 +317,7 @@ func render1(p *printer, n *Node, opts RenderOptions) {
if n.Expression {
if n.FirstChild == nil {
p.print("${(void 0)")
} else if expressionOnlyHasCommentBlock(n) {
} else if expressionOnlyHasComment(n) {
// we do not print expressions that only contain comment blocks
return
} else {
Expand Down Expand Up @@ -604,9 +610,12 @@ func render1(p *printer, n *Node, opts RenderOptions) {
}
}

// Only slot ElementNodes or non-empty TextNodes!
// CommentNode and others should not be slotted
if c.Type == ElementNode || (c.Type == TextNode && !emptyTextNodeWithoutSiblings(c)) {
// Only slot ElementNodes (except expressions containing only comments) or non-empty TextNodes!
// CommentNode, JSX comments and others should not be slotted
if expressionOnlyHasComment(c) {
continue
}
if c.Type == ElementNode || c.Type == TextNode && !emptyTextNodeWithoutSiblings(c) {
slottedChildren[slotProp] = append(slottedChildren[slotProp], c)
}
}
Expand Down
7 changes: 4 additions & 3 deletions internal/printer/print-to-tsx.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
. "github.com/withastro/compiler/internal"
astro "github.com/withastro/compiler/internal"
"github.com/withastro/compiler/internal/handler"
"github.com/withastro/compiler/internal/helpers"
"github.com/withastro/compiler/internal/js_scanner"
"github.com/withastro/compiler/internal/loc"
"github.com/withastro/compiler/internal/sourcemap"
Expand Down Expand Up @@ -229,7 +230,7 @@ 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 expressionOnlyHasCommentBlock(n) {
} else if expressionOnlyHasComment(n) {
// we do not print expressions that only contain comment blocks
return
} else {
Expand Down Expand Up @@ -350,7 +351,7 @@ declare const Astro: Readonly<import('astro').AstroGlobal<%s, typeof %s`, propsI
p.print("}")
endLoc = a.KeyLoc.Start + len(a.Key) + 1
case astro.ShorthandAttribute:
withoutComments, _ := removeComments(a.Key)
withoutComments := helpers.RemoveComments(a.Key)
if len(withoutComments) == 0 {
return
}
Expand Down Expand Up @@ -421,7 +422,7 @@ declare const Astro: Readonly<import('astro').AstroGlobal<%s, typeof %s`, propsI
case astro.SpreadAttribute:
// noop
case astro.ShorthandAttribute:
withoutComments, _ := removeComments(a.Key)
withoutComments := helpers.RemoveComments(a.Key)
if len(withoutComments) == 0 {
return
}
Expand Down
5 changes: 3 additions & 2 deletions internal/printer/printer.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

astro "github.com/withastro/compiler/internal"
"github.com/withastro/compiler/internal/handler"
"github.com/withastro/compiler/internal/helpers"
"github.com/withastro/compiler/internal/js_scanner"
"github.com/withastro/compiler/internal/loc"
"github.com/withastro/compiler/internal/sourcemap"
Expand Down Expand Up @@ -337,7 +338,7 @@ func (p *printer) printAttributesToObject(n *astro.Node) {
p.addSourceMapping(loc.Loc{Start: a.KeyLoc.Start - 3})
p.print(`...(` + strings.TrimSpace(a.Key) + `)`)
case astro.ShorthandAttribute:
withoutComments, _ := removeComments(a.Key)
withoutComments := helpers.RemoveComments(a.Key)
if len(withoutComments) == 0 {
lastAttributeSkipped = true
continue
Expand Down Expand Up @@ -422,7 +423,7 @@ func (p *printer) printAttribute(attr astro.Attribute, n *astro.Node) {
p.printf(`,undefined,{"class":"astro-%s"})}`, p.opts.Scope)
}
case astro.ShorthandAttribute:
withoutComments, _ := removeComments(attr.Key)
withoutComments := helpers.RemoveComments(attr.Key)
if len(withoutComments) == 0 {
return
}
Expand Down
56 changes: 54 additions & 2 deletions internal/printer/printer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -811,6 +811,20 @@ const groups = [[0, 1, 2], [3, 4, 5]];
code: "${$$maybeRenderHead($$result)}<body><!-- \\`npm install astro\\` --></body>",
},
},
{
name: "HTML comment in component inside expression I",
source: "{(() => <Component><!--Hi--></Component>)}",
want: want{
code: "${(() => $$render`${$$renderComponent($$result,'Component',Component,{},{})}`)}",
natemoo-re marked this conversation as resolved.
Show resolved Hide resolved
},
},
{
name: "HTML comment in component inside expression II",
source: "{list.map(() => <Component><!--Hi--></Component>)}",
want: want{
code: "${list.map(() => $$render`${$$renderComponent($$result,'Component',Component,{},{})}`)}",
},
},
{
name: "nested expressions",
source: `<article>{(previous || next) && <aside>{previous && <div>Previous Article: <a rel="prev" href={new URL(previous.link, Astro.site).pathname}>{previous.text}</a></div>}{next && <div>Next Article: <a rel="next" href={new URL(next.link, Astro.site).pathname}>{next.text}</a></div>}</aside>}</article>`,
Expand Down Expand Up @@ -989,7 +1003,7 @@ const name = "world";
},
},
{
name: "head expression and conditional renderin of fragment",
name: "head expression and conditional rendering of fragment",
source: `---
const testBool = true;
---
Expand Down Expand Up @@ -2761,12 +2775,50 @@ const items = ["Dog", "Cat", "Platipus"];
},
},
{
name: "comment only expressions are removed",
name: "comment only expressions are removed I",
source: `{/* a comment 1 */}<h1>{/* a comment 2*/}Hello</h1>`,
want: want{
code: `${$$maybeRenderHead($$result)}<h1>Hello</h1>`,
},
},
{
name: "comment only expressions are removed II",
source: `{
list.map((i) => (
<Component>
{
// hello
}
</Component>
))
}`,
want: want{
code: `${
list.map((i) => (
$$render` + BACKTICK + `${$$renderComponent($$result,'Component',Component,{},{})}` + BACKTICK + `
))
}`,
},
},
{
name: "comment only expressions are removed III",
source: `{
list.map((i) => (
<Component>
{
/* hello */
}
</Component>
))
}`,
want: want{
code: `${
list.map((i) => (
$$render` + BACKTICK + `${$$renderComponent($$result,'Component',Component,{},{})}` + BACKTICK + `
))
}`,
},
},
{
name: "component with only a script",
source: "<script>console.log('hello world');</script>",
Expand Down
27 changes: 0 additions & 27 deletions internal/printer/utils.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package printer

import (
"errors"
"fmt"
"regexp"
"strings"
Expand Down Expand Up @@ -95,32 +94,6 @@ func encodeDoubleQuote(str string) string {
return strings.Replace(str, `"`, "&quot;", -1)
}

// Remove comment blocks from string (e.g. "/* a comment */aProp" => "aProp")
func removeComments(input string) (string, error) {
var (
sb = strings.Builder{}
inComment = false
)
for cur := 0; cur < len(input); cur++ {
peekIs := func(assert byte) bool { return cur+1 < len(input) && input[cur+1] == assert }
if input[cur] == '/' && !inComment && peekIs('*') {
inComment = true
cur++
} else if input[cur] == '*' && inComment && peekIs('/') {
inComment = false
cur++
} else if !inComment {
sb.WriteByte(input[cur])
}
}

if inComment {
return "", errors.New("unterminated comment")
}

return strings.TrimSpace(sb.String()), nil
}

func convertAttributeValue(n *astro.Node, attrName string) string {
expr := `""`
if transform.HasAttr(n, attrName) {
Expand Down
Loading