Skip to content

Commit 418558c

Browse files
MoustaphaDevMoustaphaDev
authored
Fix slot regression when there are multiple expressions (#952)
* localize relevant state in expression * reuse default slot constant * more granular use of `mergeSlots` * skip expressions with comments-only earlier * test: edit test to reflect changes * refactor mergeSlots printing * simplify condition * remove wrong assumption * chore: one-liner * chore: changeset * add test case that failed in core smoke tests * correctly determine the end of slot chains * simplify code
1 parent 1457326 commit 418558c

File tree

3 files changed

+110
-61
lines changed

3 files changed

+110
-61
lines changed

‎.changeset/dull-socks-provide.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@astrojs/compiler': patch
3+
---
4+
5+
Fixes an issue where a slotted element in an expression would cause subsequent ones to be incorrectly printed

‎internal/printer/print-to-js.go

Lines changed: 74 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,7 @@ func render1(p *printer, n *Node, opts RenderOptions) {
432432
p.printAttributesToObject(n)
433433
} else if isSlot {
434434
if len(n.Attr) == 0 {
435-
p.print(`"default"`)
435+
p.print(DEFAULT_SLOT_PROP)
436436
} else {
437437
slotted := false
438438
for _, a := range n.Attr {
@@ -454,7 +454,7 @@ func render1(p *printer, n *Node, opts RenderOptions) {
454454
}
455455
}
456456
if !slotted {
457-
p.print(`"default"`)
457+
p.print(DEFAULT_SLOT_PROP)
458458
}
459459
}
460460
p.print(`]`)
@@ -559,7 +559,7 @@ func render1(p *printer, n *Node, opts RenderOptions) {
559559
switch true {
560560
case n.CustomElement:
561561
p.print(`,({`)
562-
p.print(fmt.Sprintf(`"%s": () => `, "default"))
562+
p.print(fmt.Sprintf(`%s: () => `, DEFAULT_SLOT_PROP))
563563
p.printTemplateLiteralOpen()
564564
for c := n.FirstChild; c != nil; c = c.NextSibling {
565565
render1(p, c, RenderOptions{
@@ -638,6 +638,8 @@ func render1(p *printer, n *Node, opts RenderOptions) {
638638
}
639639
}
640640

641+
const DEFAULT_SLOT_PROP = `"default"`
642+
641643
// Section 12.1.2, "Elements", gives this list of void elements. Void elements
642644
// are those that can't have any contents.
643645
// nolint
@@ -662,12 +664,14 @@ var voidElements = map[string]bool{
662664
func handleSlots(p *printer, n *Node, opts RenderOptions, depth int) {
663665
p.print(`,`)
664666
slottedChildren := make(map[string][]*Node)
665-
hasAnyDynamicSlots := false
667+
hasAnyNestedDynamicSlot := false
666668
nestedSlotChildren := make([]*NestedSlotChild, 0)
667-
numberOfNestedSlots := 0
669+
670+
// the highest number of nested slots in an expression
671+
maxNestedSlotsCount := 0
668672

669673
for c := n.FirstChild; c != nil; c = c.NextSibling {
670-
slotProp := `"default"`
674+
slotProp := DEFAULT_SLOT_PROP
671675
for _, a := range c.Attr {
672676
if a.Key == "slot" {
673677
if a.Type == QuotedAttribute {
@@ -686,82 +690,79 @@ func handleSlots(p *printer, n *Node, opts RenderOptions, depth int) {
686690
}
687691
}
688692
if c.Expression {
689-
nestedSlotsCount := 0
690-
var firstNestedSlotProp string
693+
// Only slot ElementNodes (except expressions containing only comments) or non-empty TextNodes!
694+
// CommentNode, JSX comments and others should not be slotted
695+
if expressionOnlyHasComment(c) {
696+
continue
697+
}
698+
nestedSlotsInExprCount := 0
699+
hasAnyDynamicSlotsInExpr := false
700+
var slotProp = DEFAULT_SLOT_PROP
691701
for c1 := c.FirstChild; c1 != nil; c1 = c1.NextSibling {
692-
var slotProp = ""
693702
for _, a := range c1.Attr {
694703
if a.Key == "slot" {
695704
if a.Type == QuotedAttribute {
696705
slotProp = fmt.Sprintf(`"%s"`, escapeDoubleQuote(a.Val))
697706
} else if a.Type == ExpressionAttribute {
698707
slotProp = fmt.Sprintf(`[%s]`, a.Val)
699-
hasAnyDynamicSlots = true
708+
hasAnyNestedDynamicSlot, hasAnyDynamicSlotsInExpr = true, true
700709
} else if a.Type == TemplateLiteralAttribute {
701710
slotProp = fmt.Sprintf(`[%s%s%s]`, BACKTICK, a.Val, BACKTICK)
702-
hasAnyDynamicSlots = true
711+
hasAnyNestedDynamicSlot, hasAnyDynamicSlotsInExpr = true, true
703712
} else {
704713
panic(`unknown slot attribute type`)
705714
}
706715
}
707-
if firstNestedSlotProp == "" && slotProp != "" {
708-
firstNestedSlotProp = slotProp
709-
}
710716
}
711-
if firstNestedSlotProp != "" {
712-
nestedSlotsCount++
717+
if c1.Type == ElementNode {
718+
nestedSlotsInExprCount++
713719
}
714720
}
715721

716-
if nestedSlotsCount == 1 && !hasAnyDynamicSlots {
717-
slottedChildren[firstNestedSlotProp] = append(slottedChildren[firstNestedSlotProp], c)
722+
if nestedSlotsInExprCount == 1 && !hasAnyDynamicSlotsInExpr {
723+
slottedChildren[slotProp] = append(slottedChildren[slotProp], c)
718724
continue
719-
} else if nestedSlotsCount > 1 || hasAnyDynamicSlots {
725+
} else if nestedSlotsInExprCount > 1 || hasAnyDynamicSlotsInExpr {
726+
if nestedSlotsInExprCount > maxNestedSlotsCount {
727+
maxNestedSlotsCount = nestedSlotsInExprCount
728+
}
720729
child_loop:
721730
for c1 := c.FirstChild; c1 != nil; c1 = c1.NextSibling {
722731
foundNamedSlot := false
732+
isFirstInGroup := c1 == c.FirstChild
723733
for _, a := range c1.Attr {
724734
if a.Key == "slot" {
725735
var nestedSlotProp string
726736
var nestedSlotEntry *NestedSlotChild
727737
if a.Type == QuotedAttribute {
728738
nestedSlotProp = fmt.Sprintf(`"%s"`, escapeDoubleQuote(a.Val))
729-
hasAnyDynamicSlots = true
730739
} else if a.Type == ExpressionAttribute {
731740
nestedSlotProp = fmt.Sprintf(`[%s]`, a.Val)
732-
hasAnyDynamicSlots = true
741+
hasAnyNestedDynamicSlot = true
733742
} else if a.Type == TemplateLiteralAttribute {
734-
hasAnyDynamicSlots = true
743+
hasAnyNestedDynamicSlot = true
735744
nestedSlotProp = fmt.Sprintf(`[%s%s%s]`, BACKTICK, a.Val, BACKTICK)
736745
} else {
737746
panic(`unknown slot attribute type`)
738747
}
739748
foundNamedSlot = true
740-
isFirstInGroup := c1 == c.FirstChild
741749
nestedSlotEntry = &NestedSlotChild{nestedSlotProp, []*Node{c1}, isFirstInGroup}
742750
nestedSlotChildren = append(nestedSlotChildren, nestedSlotEntry)
743751
continue child_loop
744752
}
745753
}
746-
isFirstInGroup := c1 == c.FirstChild
747754
if !foundNamedSlot && c1.Type == ElementNode {
748-
pseudoSlotEntry := &NestedSlotChild{`"default"`, []*Node{c1}, isFirstInGroup}
755+
pseudoSlotEntry := &NestedSlotChild{DEFAULT_SLOT_PROP, []*Node{c1}, isFirstInGroup}
749756
nestedSlotChildren = append(nestedSlotChildren, pseudoSlotEntry)
750757
} else {
751758
nestedSlotEntry := &NestedSlotChild{`"@@NON_ELEMENT_ENTRY"`, []*Node{c1}, isFirstInGroup}
752759
nestedSlotChildren = append(nestedSlotChildren, nestedSlotEntry)
753760
}
754-
numberOfNestedSlots++
755761
}
756762
continue
757763
}
758764
}
759765

760-
// Only slot ElementNodes (except expressions containing only comments) or non-empty TextNodes!
761-
// CommentNode, JSX comments and others should not be slotted
762-
if expressionOnlyHasComment(c) {
763-
continue
764-
}
765766
if c.Type == ElementNode || c.Type == TextNode && !emptyTextNodeWithoutSiblings(c) {
766767
slottedChildren[slotProp] = append(slottedChildren[slotProp], c)
767768
}
@@ -772,7 +773,12 @@ func handleSlots(p *printer, n *Node, opts RenderOptions, depth int) {
772773
slottedKeys = append(slottedKeys, k)
773774
}
774775
sort.Strings(slottedKeys)
775-
if numberOfNestedSlots > 0 || hasAnyDynamicSlots {
776+
777+
// if any slotted expression contains more than one nested slot (e.g. <Component>{true ? <div slot="bar">Bar</div> : <div slot="foo">Foo</div> }</Component>)
778+
// OR if any expression contains a dynamic slot (e.g. <Component>{items.map((item)=> (<div slot={item.id}>{item.name}</div>)}</Component>)
779+
// we need to use $$mergeSlots
780+
shouldPrintMergeSlots := maxNestedSlotsCount > 1 || hasAnyNestedDynamicSlot
781+
if shouldPrintMergeSlots {
776782
p.print(`$$mergeSlots(`)
777783
}
778784
p.print(`({`)
@@ -783,7 +789,7 @@ func handleSlots(p *printer, n *Node, opts RenderOptions, depth int) {
783789
children := slottedChildren[slotProp]
784790

785791
// If there are named slots, the default slot cannot be only whitespace
786-
if numberOfSlots > 1 && slotProp == "\"default\"" {
792+
if numberOfSlots > 1 && slotProp == DEFAULT_SLOT_PROP {
787793
// Loop over the children and verify that at least one non-whitespace node exists.
788794
foundNonWhitespace := false
789795
for _, child := range children {
@@ -820,10 +826,9 @@ func handleSlots(p *printer, n *Node, opts RenderOptions, depth int) {
820826
}
821827
p.print(`})`)
822828
// print nested slots
823-
if numberOfNestedSlots > 0 || hasAnyDynamicSlots {
829+
if len(nestedSlotChildren) > 0 {
824830
endSlotIndexes := generateEndSlotIndexes(nestedSlotChildren)
825-
mergeDefaultSlotsAndUpdateIndexes(&nestedSlotChildren, endSlotIndexes)
826-
831+
mergeDefaultSlotsAndUpdateIndexes(&nestedSlotChildren, &endSlotIndexes)
827832
hasFoundFirstElementNode := false
828833
for j, nestedSlot := range nestedSlotChildren {
829834
if nestedSlot.FirstInGroup {
@@ -843,6 +848,9 @@ func handleSlots(p *printer, n *Node, opts RenderOptions, depth int) {
843848
hasFoundFirstElementNode = false
844849
}
845850
}
851+
}
852+
if shouldPrintMergeSlots {
853+
// close $$mergeSlots call
846854
p.print(`)`)
847855
}
848856
}
@@ -907,25 +915,42 @@ func generateEndSlotIndexes(nestedSlotChildren []*NestedSlotChild) map[int]bool
907915
return endSlotIndexes
908916
}
909917

910-
func mergeDefaultSlotsAndUpdateIndexes(nestedSlotChildren *[]*NestedSlotChild, endSlotIndexes map[int]bool) {
911-
defaultSlot := &NestedSlotChild{SlotProp: `"default"`, Children: []*Node{}}
912-
mergedSlotChildren := make([]*NestedSlotChild, 0)
913-
numberOfMergedSlotsInSlotChain := 0
918+
func mergeDefaultSlotsAndUpdateIndexes(nestedSlotChildren *[]*NestedSlotChild, endSlotIndexes *map[int]bool) {
919+
bufferedDefaultSlot := &NestedSlotChild{SlotProp: DEFAULT_SLOT_PROP, Children: []*Node{}}
920+
updatedNestedSlotChildren := make([]*NestedSlotChild, 0)
921+
updatedEndSlotIndexes := make(map[int]bool)
914922

915923
for i, nestedSlot := range *nestedSlotChildren {
916924
if isDefaultSlot(nestedSlot) {
917-
defaultSlot.Children = append(defaultSlot.Children, nestedSlot.Children...)
918-
numberOfMergedSlotsInSlotChain++
925+
bufferedDefaultSlot.Children = append(bufferedDefaultSlot.Children, nestedSlot.Children...)
919926
} else {
920-
mergedSlotChildren = append(mergedSlotChildren, nestedSlot)
927+
updatedNestedSlotChildren = append(updatedNestedSlotChildren, nestedSlot)
921928
}
922-
if shouldMergeDefaultSlot(endSlotIndexes, i, defaultSlot) {
923-
resetEndSlotIndexes(endSlotIndexes, i, &numberOfMergedSlotsInSlotChain)
924-
mergedSlotChildren = append(mergedSlotChildren, defaultSlot)
925-
defaultSlot = &NestedSlotChild{SlotProp: `"default"`, Children: []*Node{}}
929+
930+
// we reached the end of a slot chain
931+
if (*endSlotIndexes)[i] {
932+
// free up memory, this information is now outdated
933+
// the updated information is stored in updatedEndSlotIndexes
934+
delete(*endSlotIndexes, i)
935+
936+
if len(bufferedDefaultSlot.Children) > 0 {
937+
// if the buffered default slot contains any children
938+
// add it to the updated nested slot children
939+
updatedNestedSlotChildren = append(updatedNestedSlotChildren, bufferedDefaultSlot)
940+
941+
// reset the buffered default slot
942+
bufferedDefaultSlot = &NestedSlotChild{SlotProp: DEFAULT_SLOT_PROP, Children: []*Node{}}
943+
}
944+
// record the index of the last slot in the chain
945+
updatedEndSlotIndexes[len(updatedNestedSlotChildren)-1] = true
926946
}
947+
948+
// free up memory, the actual information of nested slot
949+
// is now stored in updatedNestedSlotChildren
950+
(*nestedSlotChildren)[i] = nil
927951
}
928-
*nestedSlotChildren = mergedSlotChildren
952+
*nestedSlotChildren = updatedNestedSlotChildren
953+
*endSlotIndexes = updatedEndSlotIndexes
929954
}
930955

931956
func getSlotRenderFunction(isNewSlotObject bool) string {
@@ -943,15 +968,5 @@ func isNonWhitespaceTextNode(n *Node) bool {
943968
}
944969

945970
func isDefaultSlot(slot *NestedSlotChild) bool {
946-
return slot.SlotProp == `"default"`
947-
}
948-
949-
func shouldMergeDefaultSlot(endSlotIndexes map[int]bool, i int, defaultSlot *NestedSlotChild) bool {
950-
return endSlotIndexes[i] && len(defaultSlot.Children) > 0
951-
}
952-
953-
func resetEndSlotIndexes(endSlotIndexes map[int]bool, i int, numberOfMergedSlotsInSlotChain *int) {
954-
endSlotIndexes[i] = false
955-
endSlotIndexes[i-(*numberOfMergedSlotsInSlotChain)+1] = true
956-
(*numberOfMergedSlotsInSlotChain) = 0
971+
return slot.SlotProp == DEFAULT_SLOT_PROP
957972
}

‎internal/printer/printer_test.go

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,35 @@ func TestPrinter(t *testing.T) {
240240
code: "${$$renderComponent($$result,'Component',Component,{},$$mergeSlots(({}),Math.random() > 0.5 ? ({\"a\": () => $$render`${$$maybeRenderHead($$result)}<div>A</div>`}) : ({\"b\": () => $$render`<div>B</div>`})))}",
241241
},
242242
},
243+
{
244+
name: "ternary slot II",
245+
source: `<Layout>
246+
{
247+
Astro.request.method === 'GET' ? (
248+
<h2>Contact Form</h2>
249+
<form action="/contact" method="get">
250+
<input type="hidden" name="name" value="Testing">
251+
<button id="submit" type="submit" formmethod="post" formaction="/form-three">Submit</button>
252+
</form>
253+
) : (
254+
<div id="three-result">Got: {formData?.get('name')}</div>
255+
)
256+
}
257+
</Layout>`,
258+
want: want{
259+
code: `${$$renderComponent($$result,'Layout',Layout,{},$$mergeSlots(({}),
260+
Astro.request.method === 'GET' ? (
261+
262+
({"default": () => $$render` + BACKTICK + `${$$maybeRenderHead($$result)}<h2>Contact Form</h2><form action="/contact" method="get">
263+
<input type="hidden" name="name" value="Testing">
264+
<button id="submit" type="submit" formmethod="post" formaction="/form-three">Submit</button>
265+
</form>` + BACKTICK + `})
266+
) : (
267+
({"default": () => $$render` + BACKTICK + `<div id="three-result">Got: ${formData?.get('name')}</div>` + BACKTICK + `})
268+
)
269+
))}`,
270+
},
271+
},
243272
{
244273
name: "ternary slot with one implicit default",
245274
source: `<Main>
@@ -278,7 +307,7 @@ func TestPrinter(t *testing.T) {
278307
{true && <span>Default</span>}
279308
</Slotted>`,
280309
want: want{
281-
code: "${$$renderComponent($$result,'Slotted',Slotted,{},$$mergeSlots(({\"a\": () => $$render`${true && $$render`${$$maybeRenderHead($$result)}<span>A</span>`}`,}),true ? ({\"b\": () => $$render`<span>B</span>`}) : null,() => ({\"c\": () => $$render`<span>C</span>`}),true && ({\"default\": () => $$render`<span>Default</span>`})))}",
310+
code: "${$$renderComponent($$result,'Slotted',Slotted,{},({\"a\": () => $$render`${true && $$render`${$$maybeRenderHead($$result)}<span>A</span>`}`,\"b\": () => $$render`${true ? $$render`<span>B</span>` : null}`,\"c\": () => $$render`${() => $$render`<span>C</span>`}`,\"default\": () => $$render`${true && $$render`<span>Default</span>`}`,}))}",
282311
},
283312
},
284313
{
@@ -300,7 +329,7 @@ func TestPrinter(t *testing.T) {
300329
}}
301330
</Slotted>`,
302331
want: want{
303-
code: `${$$renderComponent($$result,'Slotted',Slotted,{},$$mergeSlots(({}),true && ({["a"]: () => $$render` + BACKTICK + `${$$maybeRenderHead($$result)}<span>A</span>` + BACKTICK + `}),true ? ({"b": () => $$render` + BACKTICK + `<span>B</span>` + BACKTICK + `}) : null,() => ({"c": () => $$render` + BACKTICK + `<span>C</span>` + BACKTICK + `}),() => {
332+
code: `${$$renderComponent($$result,'Slotted',Slotted,{},$$mergeSlots(({"b": () => $$render` + BACKTICK + `${true ? $$render` + BACKTICK + `${$$maybeRenderHead($$result)}<span>B</span>` + BACKTICK + ` : null}` + BACKTICK + `,"c": () => $$render` + BACKTICK + `${() => $$render` + BACKTICK + `<span>C</span>` + BACKTICK + `}` + BACKTICK + `,}),true && ({["a"]: () => $$render` + BACKTICK + `<span>A</span>` + BACKTICK + `}),() => {
304333
const value = 0.33;
305334
if (value > 0.25) {
306335
return ({"hey": () => $$render` + BACKTICK + `<span>Another</span>` + BACKTICK + `, "default": () => $$render` + BACKTICK + `<span>Default</span>` + BACKTICK + `})

0 commit comments

Comments
 (0)