Skip to content

Commit 11c65dc

Browse files
authored
fix: XML Node with title tag nested in any other XML Node breaks document (#4361)
## Description closes #3715 ## Steps for reproduction paste in chrome `view-source:https://webstudio-fixture-project-a-0su3o.wstd.work/sitemap.xml` See title tag exists <img width="412" alt="image" src="https://github.com/user-attachments/assets/66df98b3-8d41-4890-8957-6f0bf6a758bb"> ## Code Review - [ ] hi @kof, I need you to do - conceptual review (architecture, feature-correctness) - detailed review (read every line) - test it on preview ## Before requesting a review - [ ] made a self-review - [ ] added inline comments where things may be not obvious (the "why", not "what") ## Before merging - [ ] tested locally and on preview environment (preview dev login: 5de6) - [ ] updated [test cases](https://github.com/webstudio-is/webstudio/blob/main/apps/builder/docs/test-cases.md) document - [ ] added tests - [ ] if any new env variables are added, added them to `.env` file
1 parent b762a92 commit 11c65dc

File tree

9 files changed

+63
-19
lines changed

9 files changed

+63
-19
lines changed

fixtures/webstudio-custom-template/app/__generated__/[sitemap.xml]._index.tsx

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

fixtures/webstudio-custom-template/app/routes/[sitemap.xml]._index.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ export const loader = async (arg: LoaderFunctionArgs) => {
6060
// typecheck
6161
arg.context.EXCLUDE_FROM_SEARCH satisfies boolean;
6262

63-
const text = renderToString(
63+
let text = renderToString(
6464
<ReactSdkContext.Provider
6565
value={{
6666
imageLoader,
@@ -73,6 +73,10 @@ export const loader = async (arg: LoaderFunctionArgs) => {
7373
</ReactSdkContext.Provider>
7474
);
7575

76+
// Xml is wrapped with <svg> to prevent React from hoisting elements like <title>, <meta>, and <link> out of their intended scope during rendering.
77+
// More details: https://github.com/facebook/react/blob/7c8e5e7ab8bb63de911637892392c5efd8ce1d0f/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js#L3083
78+
text = text.replace(/^<svg>/g, "").replace(/<\/svg>$/g, "");
79+
7680
return new Response(`<?xml version="1.0" encoding="UTF-8"?>\n${text}`, {
7781
headers: { "Content-Type": "application/xml" },
7882
});

fixtures/webstudio-remix-vercel/.webstudio/data.json

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
{
22
"build": {
3-
"id": "1710e6a3-6f3b-44c9-8e4f-4176be77673e",
3+
"id": "5646b5e6-a161-4fbc-8312-6e2852e1d4b3",
44
"projectId": "cddc1d44-af37-4cb6-a430-d300cf6f932d",
5-
"version": 341,
6-
"createdAt": "2024-10-28T17:19:04.754+00:00",
7-
"updatedAt": "2024-10-28T17:19:04.754+00:00",
5+
"version": 345,
6+
"createdAt": "2024-11-04T04:15:27.144+00:00",
7+
"updatedAt": "2024-11-04T04:15:27.144+00:00",
88
"pages": {
99
"meta": {
1010
"siteName": "KittyGuardedZone",
@@ -2601,6 +2601,16 @@
26012601
"type": "string",
26022602
"value": "http://www.w3.org/1999/xhtml"
26032603
}
2604+
],
2605+
[
2606+
"kQb_yRWDNAsug8p2lhSxN",
2607+
{
2608+
"id": "kQb_yRWDNAsug8p2lhSxN",
2609+
"instanceId": "wLYkMGH-OJP4SnFB4F-bs",
2610+
"name": "tag",
2611+
"type": "string",
2612+
"value": "title"
2613+
}
26042614
]
26052615
],
26062616
"dataSources": [
@@ -4075,6 +4085,10 @@
40754085
{
40764086
"type": "id",
40774087
"value": "bBAE2vpcCLzlcin29wQYA"
4088+
},
4089+
{
4090+
"type": "id",
4091+
"value": "wLYkMGH-OJP4SnFB4F-bs"
40784092
}
40794093
]
40804094
}
@@ -4124,6 +4138,20 @@
41244138
"component": "XmlNode",
41254139
"children": []
41264140
}
4141+
],
4142+
[
4143+
"wLYkMGH-OJP4SnFB4F-bs",
4144+
{
4145+
"type": "instance",
4146+
"id": "wLYkMGH-OJP4SnFB4F-bs",
4147+
"component": "XmlNode",
4148+
"children": [
4149+
{
4150+
"type": "text",
4151+
"value": "Hello"
4152+
}
4153+
]
4154+
}
41274155
]
41284156
],
41294157
"deployment": {

fixtures/webstudio-remix-vercel/app/__generated__/$resources.sitemap.xml.ts

Lines changed: 6 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

fixtures/webstudio-remix-vercel/app/__generated__/[sitemap.xml]._index.tsx

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

fixtures/webstudio-remix-vercel/app/routes/[sitemap.xml]._index.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ export const loader = async (arg: LoaderFunctionArgs) => {
6060
// typecheck
6161
arg.context.EXCLUDE_FROM_SEARCH satisfies boolean;
6262

63-
const text = renderToString(
63+
let text = renderToString(
6464
<ReactSdkContext.Provider
6565
value={{
6666
imageLoader,
@@ -73,6 +73,10 @@ export const loader = async (arg: LoaderFunctionArgs) => {
7373
</ReactSdkContext.Provider>
7474
);
7575

76+
// Xml is wrapped with <svg> to prevent React from hoisting elements like <title>, <meta>, and <link> out of their intended scope during rendering.
77+
// More details: https://github.com/facebook/react/blob/7c8e5e7ab8bb63de911637892392c5efd8ce1d0f/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js#L3083
78+
text = text.replace(/^<svg>/g, "").replace(/<\/svg>$/g, "");
79+
7680
return new Response(`<?xml version="1.0" encoding="UTF-8"?>\n${text}`, {
7781
headers: { "Content-Type": "application/xml" },
7882
});

fixtures/webstudio-remix-vercel/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
"typecheck": "tsc",
77
"cli": "NODE_OPTIONS='--conditions=webstudio --import=tsx' webstudio",
88
"fixtures:link": "pnpm cli link --link https://p-cddc1d44-af37-4cb6-a430-d300cf6f932d-dot-${BUILDER_HOST:-main.development.webstudio.is}'?authToken=1cdc6026-dd5b-4624-b89b-9bd45e9bcc3d'",
9-
"fixtures:sync": "pnpm cli sync --buildId 1710e6a3-6f3b-44c9-8e4f-4176be77673e && pnpm prettier --write ./.webstudio/",
9+
"fixtures:sync": "pnpm cli sync --buildId 5646b5e6-a161-4fbc-8312-6e2852e1d4b3 && pnpm prettier --write ./.webstudio/",
1010
"fixtures:build": "pnpm cli build --template vercel --template internal --preview && pnpm prettier --write ./app/ ./package.json ./tsconfig.json"
1111
},
1212
"private": true,

packages/cli/src/prebuild.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -550,16 +550,19 @@ export const prebuild = async (options: {
550550
// In case of xml it's the only component we are supporting
551551
componentImports = `import { XmlNode } from "@webstudio-is/sdk-components-react";\n`;
552552

553-
// Passthrough (render children) for Body, do not render all other components
554553
xmlPresentationComponents += Array.from(componentsSet)
555554
.map(([shortName, component]) =>
556555
scope.getName(component, shortName)
557556
)
558557
.filter((scopedName) => scopedName !== "XmlNode")
559558
.map((scopedName) =>
560559
scopedName === "Body"
561-
? `const ${scopedName} = (props: any) => props.children;`
562-
: `const ${scopedName} = (props: any) => null;`
560+
? // Using <svg> prevents React from hoisting elements like <title>, <meta>, and <link>
561+
// out of their intended scope during rendering.
562+
// More details: https://github.com/facebook/react/blob/7c8e5e7ab8bb63de911637892392c5efd8ce1d0f/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js#L3083
563+
`const ${scopedName} = (props: any) => <svg>{props.children}</svg>;`
564+
: // Do not render all other components
565+
`const ${scopedName} = (props: any) => null;`
563566
)
564567
.join("\n");
565568
}

packages/cli/templates/defaults/app/route-templates/xml.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ export const loader = async (arg: LoaderFunctionArgs) => {
5656
// typecheck
5757
arg.context.EXCLUDE_FROM_SEARCH satisfies boolean;
5858

59-
const text = renderToString(
59+
let text = renderToString(
6060
<ReactSdkContext.Provider
6161
value={{
6262
imageLoader,
@@ -69,6 +69,10 @@ export const loader = async (arg: LoaderFunctionArgs) => {
6969
</ReactSdkContext.Provider>
7070
);
7171

72+
// Xml is wrapped with <svg> to prevent React from hoisting elements like <title>, <meta>, and <link> out of their intended scope during rendering.
73+
// More details: https://github.com/facebook/react/blob/7c8e5e7ab8bb63de911637892392c5efd8ce1d0f/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js#L3083
74+
text = text.replace(/^<svg>/g, "").replace(/<\/svg>$/g, "");
75+
7276
return new Response(`<?xml version="1.0" encoding="UTF-8"?>\n${text}`, {
7377
headers: { "Content-Type": "application/xml" },
7478
});

0 commit comments

Comments
 (0)