Skip to content

Commit

Permalink
fix: allow conditional rendering when RenderScript is enabled (#1057)
Browse files Browse the repository at this point in the history
* test: add test cases

* fix: allow processing scripts in expressions when `RenderScript` is enabled

* test: add test case without src attribute

* fix: add `nil` parent guard

* test: add new snapshots

* chore: changeset

* test: remove duplicate snapshot

* apply sarah's changeset suggestion

Co-authored-by: Sarah Rainsberger <[email protected]>

---------

Co-authored-by: Sarah Rainsberger <[email protected]>
  • Loading branch information
MoustaphaDev and sarah11918 authored Jan 30, 2025
1 parent 71fb3ef commit 8cae811
Show file tree
Hide file tree
Showing 8 changed files with 208 additions and 5 deletions.
7 changes: 7 additions & 0 deletions .changeset/grumpy-ads-know.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@astrojs/compiler": patch
---

Fixes an issue with the conditional rendering of scripts.

**This change updates a v5.0 breaking change when `experimental.directRenderScript` became the default script handling behavior.** If you have already successfully upgraded to Astro v5, you may need to review your script tags again and make sure they still behave as desired after this release. [See the v5 Upgrade Guide for more details.](https://docs.astro.build/en/guides/upgrade-to/v5/#script-tags-are-rendered-directly-as-declared)
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@

[TestPrinter/script_external_in_expression_(renderScript:_false) - 1]
## Input

```
<main>{<script src="./hello.js"></script>}
```

## Output

```js
import {
Fragment,
render as $$render,
createAstro as $$createAstro,
createComponent as $$createComponent,
renderComponent as $$renderComponent,
renderHead as $$renderHead,
maybeRenderHead as $$maybeRenderHead,
unescapeHTML as $$unescapeHTML,
renderSlot as $$renderSlot,
mergeSlots as $$mergeSlots,
addAttribute as $$addAttribute,
spreadAttributes as $$spreadAttributes,
defineStyleVars as $$defineStyleVars,
defineScriptVars as $$defineScriptVars,
renderTransition as $$renderTransition,
createTransitionScope as $$createTransitionScope,
renderScript as $$renderScript,
createMetadata as $$createMetadata
} from "http://localhost:3000/";

export const $$metadata = $$createMetadata("/src/pages/index.astro", { modules: [], hydratedComponents: [], clientOnlyComponents: [], hydrationDirectives: new Set([]), hoisted: [] });

const $$Index = $$createComponent(($$result, $$props, $$slots) => {

return $$render`${$$maybeRenderHead($$result)}<main>${$$render`<script src="./hello.js"></script>`}</main>`;
}, '/src/pages/index.astro', undefined);
export default $$Index;
```
---
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@

[TestPrinter/script_external_in_expression_(renderScript:_true) - 1]
## Input

```
<main>{<script src="./hello.js"></script>}
```

## Output

```js
import {
Fragment,
render as $$render,
createAstro as $$createAstro,
createComponent as $$createComponent,
renderComponent as $$renderComponent,
renderHead as $$renderHead,
maybeRenderHead as $$maybeRenderHead,
unescapeHTML as $$unescapeHTML,
renderSlot as $$renderSlot,
mergeSlots as $$mergeSlots,
addAttribute as $$addAttribute,
spreadAttributes as $$spreadAttributes,
defineStyleVars as $$defineStyleVars,
defineScriptVars as $$defineScriptVars,
renderTransition as $$renderTransition,
createTransitionScope as $$createTransitionScope,
renderScript as $$renderScript,
createMetadata as $$createMetadata
} from "http://localhost:3000/";

export const $$metadata = $$createMetadata("/src/pages/index.astro", { modules: [], hydratedComponents: [], clientOnlyComponents: [], hydrationDirectives: new Set([]), hoisted: [{ type: 'external', src: './hello.js' }] });

const $$Index = $$createComponent(($$result, $$props, $$slots) => {

return $$render`${$$maybeRenderHead($$result)}<main>${$$render`${$$renderScript($$result,"/src/pages/index.astro?astro&type=script&index=0&lang.ts")}`}</main>`;
}, '/src/pages/index.astro', undefined);
export default $$Index;
```
---
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@

[TestPrinter/script_in_expression_(renderScript:_false) - 1]
## Input

```
<main>{false && <script>console.log("hello")</script>}
```

## Output

```js
import {
Fragment,
render as $$render,
createAstro as $$createAstro,
createComponent as $$createComponent,
renderComponent as $$renderComponent,
renderHead as $$renderHead,
maybeRenderHead as $$maybeRenderHead,
unescapeHTML as $$unescapeHTML,
renderSlot as $$renderSlot,
mergeSlots as $$mergeSlots,
addAttribute as $$addAttribute,
spreadAttributes as $$spreadAttributes,
defineStyleVars as $$defineStyleVars,
defineScriptVars as $$defineScriptVars,
renderTransition as $$renderTransition,
createTransitionScope as $$createTransitionScope,
renderScript as $$renderScript,
createMetadata as $$createMetadata
} from "http://localhost:3000/";

export const $$metadata = $$createMetadata("/src/pages/index.astro", { modules: [], hydratedComponents: [], clientOnlyComponents: [], hydrationDirectives: new Set([]), hoisted: [] });

const $$Index = $$createComponent(($$result, $$props, $$slots) => {

return $$render`${$$maybeRenderHead($$result)}<main>${false && $$render`<script>console.log("hello")</script>`}</main>`;
}, '/src/pages/index.astro', undefined);
export default $$Index;
```
---
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@

[TestPrinter/script_in_expression_(renderScript:_true) - 1]
## Input

```
<main>{true && <script>console.log("hello")</script>}
```

## Output

```js
import {
Fragment,
render as $$render,
createAstro as $$createAstro,
createComponent as $$createComponent,
renderComponent as $$renderComponent,
renderHead as $$renderHead,
maybeRenderHead as $$maybeRenderHead,
unescapeHTML as $$unescapeHTML,
renderSlot as $$renderSlot,
mergeSlots as $$mergeSlots,
addAttribute as $$addAttribute,
spreadAttributes as $$spreadAttributes,
defineStyleVars as $$defineStyleVars,
defineScriptVars as $$defineScriptVars,
renderTransition as $$renderTransition,
createTransitionScope as $$createTransitionScope,
renderScript as $$renderScript,
createMetadata as $$createMetadata
} from "http://localhost:3000/";

export const $$metadata = $$createMetadata("/src/pages/index.astro", { modules: [], hydratedComponents: [], clientOnlyComponents: [], hydrationDirectives: new Set([]), hoisted: [{ type: 'inline', value: `console.log("hello")` }] });

const $$Index = $$createComponent(($$result, $$props, $$slots) => {

return $$render`${$$maybeRenderHead($$result)}<main>${true && $$render`${$$renderScript($$result,"/src/pages/index.astro?astro&type=script&index=0&lang.ts")}`}</main>`;
}, '/src/pages/index.astro', undefined);
export default $$Index;
```
---
30 changes: 28 additions & 2 deletions internal/printer/printer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -894,6 +894,34 @@ import Widget2 from '../components/Widget2.astro';
},
filename: "/src/pages/index.astro",
},
{
name: "script external in expression (renderScript: true)",
source: `<main>{<script src="./hello.js"></script>}`,
transformOptions: transform.TransformOptions{
RenderScript: true,
},
filename: "/src/pages/index.astro",
},
{
// maintain the original behavior, though it may be
// unneeded as renderScript is now on by default
name: "script external in expression (renderScript: false)",
source: `<main>{<script src="./hello.js"></script>}`,
filename: "/src/pages/index.astro",
},
{
name: "script in expression (renderScript: true)",
source: `<main>{true && <script>console.log("hello")</script>}`,
transformOptions: transform.TransformOptions{
RenderScript: true,
},
filename: "/src/pages/index.astro",
},
{
name: "script in expression (renderScript: false)",
source: `<main>{false && <script>console.log("hello")</script>}`,
filename: "/src/pages/index.astro",
},
{
name: "script inline (renderScript: true)",
source: `<main><script is:inline type="module">console.log("Hello");</script>`,
Expand Down Expand Up @@ -2092,7 +2120,6 @@ const meta = { title: 'My App' };
Kind: test_utils.JsOutput,
FolderName: "__printer_js__",
})

})
}
}
Expand Down Expand Up @@ -2215,7 +2242,6 @@ const c = '\''
code := test_utils.Dedent(tt.source)

doc, err := astro.ParseWithOptions(strings.NewReader(code), astro.ParseOptionEnableLiteral(true), astro.ParseOptionWithHandler(&handler.Handler{}))

if err != nil {
t.Error(err)
}
Expand Down
5 changes: 3 additions & 2 deletions internal/transform/transform.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func ExtractStyles(doc *astro.Node) {
return
}
// Ignore styles in svg/noscript/etc
if !IsHoistable(n) {
if !IsHoistable(n, false) {
return
}
// prepend node to maintain authored order
Expand Down Expand Up @@ -403,7 +403,8 @@ func ExtractScript(doc *astro.Node, n *astro.Node, opts *TransformOptions, h *ha
return
}
// Ignore scripts in svg/noscript/etc
if !IsHoistable(n) {
// In expressions ignore scripts, unless `RenderScript` is true
if !IsHoistable(n, opts.RenderScript) {
return
}

Expand Down
7 changes: 6 additions & 1 deletion internal/transform/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,15 @@ func GetAttr(n *astro.Node, key string) *astro.Attribute {
return nil
}

func IsHoistable(n *astro.Node) bool {
func IsHoistable(n *astro.Node, renderScriptEnabled bool) bool {
parent := n.Closest(func(p *astro.Node) bool {
return p.DataAtom == atom.Svg || p.DataAtom == atom.Noscript || p.DataAtom == atom.Template
})

if renderScriptEnabled && parent != nil && parent.Expression {
return true
}

return parent == nil
}

Expand Down

0 comments on commit 8cae811

Please sign in to comment.