From 7857889d7282f8d25f24cab254cdd3779e807a95 Mon Sep 17 00:00:00 2001 From: Moustapha HappyDev Date: Tue, 7 Jan 2025 20:25:34 +0000 Subject: [PATCH 1/8] test: add test cases --- internal/printer/printer_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/internal/printer/printer_test.go b/internal/printer/printer_test.go index 0089ea34..12609d96 100644 --- a/internal/printer/printer_test.go +++ b/internal/printer/printer_test.go @@ -894,6 +894,22 @@ import Widget2 from '../components/Widget2.astro'; }, filename: "/src/pages/index.astro", }, + { + name: "script external in expression (renderScript: true)", + source: `
{}`, + 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: `
{}`, + filename: "/src/pages/index.astro", + }, + { name: "script inline (renderScript: true)", source: `
`, From 65e344415b727611c0dc11e0c159b39c5aae9cbf Mon Sep 17 00:00:00 2001 From: Moustapha HappyDev Date: Tue, 7 Jan 2025 21:09:16 +0000 Subject: [PATCH 2/8] fix: allow processing scripts in expressions when `RenderScript` is enabled --- internal/printer/printer_test.go | 2 -- internal/transform/transform.go | 4 ++-- internal/transform/utils.go | 6 +++++- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/internal/printer/printer_test.go b/internal/printer/printer_test.go index 12609d96..6e1d4136 100644 --- a/internal/printer/printer_test.go +++ b/internal/printer/printer_test.go @@ -2108,7 +2108,6 @@ const meta = { title: 'My App' }; Kind: test_utils.JsOutput, FolderName: "__printer_js__", }) - }) } } @@ -2231,7 +2230,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) } diff --git a/internal/transform/transform.go b/internal/transform/transform.go index c411361b..efb51931 100644 --- a/internal/transform/transform.go +++ b/internal/transform/transform.go @@ -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 @@ -403,7 +403,7 @@ func ExtractScript(doc *astro.Node, n *astro.Node, opts *TransformOptions, h *ha return } // Ignore scripts in svg/noscript/etc - if !IsHoistable(n) { + if !IsHoistable(n, opts.RenderScript) { return } diff --git a/internal/transform/utils.go b/internal/transform/utils.go index 1a5e082b..24a5de02 100644 --- a/internal/transform/utils.go +++ b/internal/transform/utils.go @@ -47,10 +47,14 @@ func GetAttr(n *astro.Node, key string) *astro.Attribute { return nil } -func IsHoistable(n *astro.Node) bool { +func IsHoistable(n *astro.Node, hoistInExpression bool) bool { parent := n.Closest(func(p *astro.Node) bool { return p.DataAtom == atom.Svg || p.DataAtom == atom.Noscript || p.DataAtom == atom.Template }) + + if hoistInExpression && parent.Expression { + return true + } return parent == nil } From fd8cb93533f46475098c1f178e49c926c280d0d9 Mon Sep 17 00:00:00 2001 From: Moustapha HappyDev Date: Tue, 7 Jan 2025 21:15:35 +0000 Subject: [PATCH 3/8] test: add test case without src attribute --- internal/printer/printer_test.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/internal/printer/printer_test.go b/internal/printer/printer_test.go index 6e1d4136..12d1ae9e 100644 --- a/internal/printer/printer_test.go +++ b/internal/printer/printer_test.go @@ -909,7 +909,19 @@ import Widget2 from '../components/Widget2.astro'; source: `
{}`, filename: "/src/pages/index.astro", }, - + { + name: "script in expression (renderScript: true)", + source: `
{true && }`, + transformOptions: transform.TransformOptions{ + RenderScript: true, + }, + filename: "/src/pages/index.astro", + }, + { + name: "script in expression (renderScript: false)", + source: `
{false && }`, + filename: "/src/pages/index.astro", + }, { name: "script inline (renderScript: true)", source: `
`, From 7e51d534daedd0b295461709942686120889d95f Mon Sep 17 00:00:00 2001 From: Moustapha HappyDev Date: Tue, 7 Jan 2025 21:16:31 +0000 Subject: [PATCH 4/8] fix: add `nil` parent guard --- internal/transform/transform.go | 1 + internal/transform/utils.go | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/transform/transform.go b/internal/transform/transform.go index efb51931..effa2148 100644 --- a/internal/transform/transform.go +++ b/internal/transform/transform.go @@ -403,6 +403,7 @@ func ExtractScript(doc *astro.Node, n *astro.Node, opts *TransformOptions, h *ha return } // Ignore scripts in svg/noscript/etc + // In expressions ignore scripts, unless `RenderScript` is true if !IsHoistable(n, opts.RenderScript) { return } diff --git a/internal/transform/utils.go b/internal/transform/utils.go index 24a5de02..1a6bbe5e 100644 --- a/internal/transform/utils.go +++ b/internal/transform/utils.go @@ -47,14 +47,15 @@ func GetAttr(n *astro.Node, key string) *astro.Attribute { return nil } -func IsHoistable(n *astro.Node, hoistInExpression bool) 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 hoistInExpression && parent.Expression { + if renderScriptEnabled && parent != nil && parent.Expression { return true } + return parent == nil } From 339179bacbda8e9e37c1a5fc2db717b4cc617236 Mon Sep 17 00:00:00 2001 From: Moustapha HappyDev Date: Tue, 7 Jan 2025 21:31:57 +0000 Subject: [PATCH 5/8] test: add new snapshots --- ...l_in_expression__renderScript__false_.snap | 82 +++++++++++++++++++ ...al_in_expression__renderScript__true_.snap | 41 ++++++++++ ...t_in_expression__renderScript__false_.snap | 41 ++++++++++ ...pt_in_expression__renderScript__true_.snap | 41 ++++++++++ 4 files changed, 205 insertions(+) create mode 100755 internal/printer/__printer_js__/script_external_in_expression__renderScript__false_.snap create mode 100755 internal/printer/__printer_js__/script_external_in_expression__renderScript__true_.snap create mode 100755 internal/printer/__printer_js__/script_in_expression__renderScript__false_.snap create mode 100755 internal/printer/__printer_js__/script_in_expression__renderScript__true_.snap diff --git a/internal/printer/__printer_js__/script_external_in_expression__renderScript__false_.snap b/internal/printer/__printer_js__/script_external_in_expression__renderScript__false_.snap new file mode 100755 index 00000000..65702aed --- /dev/null +++ b/internal/printer/__printer_js__/script_external_in_expression__renderScript__false_.snap @@ -0,0 +1,82 @@ + +[TestPrinter/script_external_in_expression_(renderScript:_false) - 1] +## Input + +``` +
{} +``` + +## 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)}
${$$render``}
`; +}, '/src/pages/index.astro', undefined); +export default $$Index; +``` +--- + +[TestPrinter/script_external_in_expression_(renderScript:_false)#01 - 1] +## Input + +``` +
{false && } +``` + +## 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)}
${false && $$render``}
`; +}, '/src/pages/index.astro', undefined); +export default $$Index; +``` +--- diff --git a/internal/printer/__printer_js__/script_external_in_expression__renderScript__true_.snap b/internal/printer/__printer_js__/script_external_in_expression__renderScript__true_.snap new file mode 100755 index 00000000..179e5709 --- /dev/null +++ b/internal/printer/__printer_js__/script_external_in_expression__renderScript__true_.snap @@ -0,0 +1,41 @@ + +[TestPrinter/script_external_in_expression_(renderScript:_true) - 1] +## Input + +``` +
{} +``` + +## 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)}
${$$render`${$$renderScript($$result,"/src/pages/index.astro?astro&type=script&index=0&lang.ts")}`}
`; +}, '/src/pages/index.astro', undefined); +export default $$Index; +``` +--- diff --git a/internal/printer/__printer_js__/script_in_expression__renderScript__false_.snap b/internal/printer/__printer_js__/script_in_expression__renderScript__false_.snap new file mode 100755 index 00000000..79ac613b --- /dev/null +++ b/internal/printer/__printer_js__/script_in_expression__renderScript__false_.snap @@ -0,0 +1,41 @@ + +[TestPrinter/script_in_expression_(renderScript:_false) - 1] +## Input + +``` +
{false && } +``` + +## 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)}
${false && $$render``}
`; +}, '/src/pages/index.astro', undefined); +export default $$Index; +``` +--- diff --git a/internal/printer/__printer_js__/script_in_expression__renderScript__true_.snap b/internal/printer/__printer_js__/script_in_expression__renderScript__true_.snap new file mode 100755 index 00000000..6a96b964 --- /dev/null +++ b/internal/printer/__printer_js__/script_in_expression__renderScript__true_.snap @@ -0,0 +1,41 @@ + +[TestPrinter/script_in_expression_(renderScript:_true) - 1] +## Input + +``` +
{true && } +``` + +## 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)}
${true && $$render`${$$renderScript($$result,"/src/pages/index.astro?astro&type=script&index=0&lang.ts")}`}
`; +}, '/src/pages/index.astro', undefined); +export default $$Index; +``` +--- From 00c14abe1ee677389ced573deec9c181fa488285 Mon Sep 17 00:00:00 2001 From: Moustapha HappyDev Date: Tue, 7 Jan 2025 21:38:57 +0000 Subject: [PATCH 6/8] chore: changeset --- .changeset/grumpy-ads-know.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/grumpy-ads-know.md diff --git a/.changeset/grumpy-ads-know.md b/.changeset/grumpy-ads-know.md new file mode 100644 index 00000000..04a6f387 --- /dev/null +++ b/.changeset/grumpy-ads-know.md @@ -0,0 +1,5 @@ +--- +"@astrojs/compiler": patch +--- + +Allows conditional rendering of scripts when `RenderScript` is enabled From 33cff5187cf4040728cbb00a1cfee2fb978631c4 Mon Sep 17 00:00:00 2001 From: Moustapha HappyDev Date: Wed, 8 Jan 2025 15:33:07 +0000 Subject: [PATCH 7/8] test: remove duplicate snapshot --- ...l_in_expression__renderScript__false_.snap | 41 ------------------- 1 file changed, 41 deletions(-) diff --git a/internal/printer/__printer_js__/script_external_in_expression__renderScript__false_.snap b/internal/printer/__printer_js__/script_external_in_expression__renderScript__false_.snap index 65702aed..685add51 100755 --- a/internal/printer/__printer_js__/script_external_in_expression__renderScript__false_.snap +++ b/internal/printer/__printer_js__/script_external_in_expression__renderScript__false_.snap @@ -39,44 +39,3 @@ return $$render`${$$maybeRenderHead($$result)}
${$$render``}
`; -}, '/src/pages/index.astro', undefined); -export default $$Index; -``` ---- From 3f1f082de0d7951514e769166eb1732b7a2b3df9 Mon Sep 17 00:00:00 2001 From: Happydev <81974850+MoustaphaDev@users.noreply.github.com> Date: Fri, 10 Jan 2025 15:23:37 +0000 Subject: [PATCH 8/8] apply sarah's changeset suggestion Co-authored-by: Sarah Rainsberger <5098874+sarah11918@users.noreply.github.com> --- .changeset/grumpy-ads-know.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.changeset/grumpy-ads-know.md b/.changeset/grumpy-ads-know.md index 04a6f387..b0a1473d 100644 --- a/.changeset/grumpy-ads-know.md +++ b/.changeset/grumpy-ads-know.md @@ -2,4 +2,6 @@ "@astrojs/compiler": patch --- -Allows conditional rendering of scripts when `RenderScript` is enabled +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)