-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix(router-ssr): ensure charset meta is placed before dehydration script #5038
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
import { expect } from '@playwright/test' | ||
import { test } from './fixture' | ||
|
||
test.describe('Charset Encoding', () => { | ||
test('asserts charset meta tag appears before dehydration script', async ({ | ||
page, | ||
}) => { | ||
// Navigate to a server-rendered page to get the HTML with dehydration scripts | ||
const response = await page.goto('/') | ||
const html = await response?.text() | ||
|
||
expect(html).toBeDefined() | ||
if (!html) return | ||
|
||
// Case-insensitive search for charset meta and TSR script | ||
const htmlLower = html.toLowerCase() | ||
const charsetIndex = htmlLower.search(/<meta\s+charset=(["'])utf-8\1/) | ||
const tsrScriptIndex = htmlLower.search(/<script\s+class=(["'])\$tsr\1/) | ||
|
||
// Both should exist in server-rendered HTML with dehydration | ||
expect(charsetIndex).toBeGreaterThan(-1) | ||
expect(tsrScriptIndex).toBeGreaterThan(-1) | ||
|
||
// With the fix, the charset meta tag should now appear BEFORE the TSR dehydration script. | ||
// This ensures correct character encoding and compliance with the HTML5 spec. | ||
expect(charsetIndex).toBeLessThan(tsrScriptIndex) | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -24,6 +24,8 @@ const patternBodyStart = /(<body)/ | |||||||||||||
const patternBodyEnd = /(<\/body>)/ | ||||||||||||||
const patternHtmlEnd = /(<\/html>)/ | ||||||||||||||
const patternHeadStart = /(<head.*?>)/ | ||||||||||||||
const patternHeadEnd = /(<\/head>)/ | ||||||||||||||
const patternCharset = /(<meta\s+charset=(["']?)[^"'>\s]+\2.*?>)/i | ||||||||||||||
// regex pattern for matching closing tags | ||||||||||||||
Comment on lines
+27
to
29
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Broaden charset detection to handle spacing and additional attributes. Current pattern requires -const patternHeadEnd = /(<\/head>)/
-const patternCharset = /(<meta\s+charset=(["']?)[^"'>\s]+\2.*?>)/i
+const patternHeadEnd = /(<\/head>)/
+const patternCharset = /(<meta[^>]*\bcharset\s*=\s*(["'])?[^"'>\s]+\2[^>]*>)/i 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||
const patternClosingTag = /(<\/[a-zA-Z][\w:.-]*?>)/g | ||||||||||||||
|
||||||||||||||
|
@@ -98,6 +100,7 @@ export function transformStreamWithRouter( | |||||||||||||
let pendingClosingTags = '' | ||||||||||||||
let bodyStarted = false as boolean | ||||||||||||||
let headStarted = false as boolean | ||||||||||||||
let headScriptInjected = false as boolean | ||||||||||||||
let leftover = '' | ||||||||||||||
let leftoverHtml = '' | ||||||||||||||
|
||||||||||||||
|
@@ -181,18 +184,35 @@ export function transformStreamWithRouter( | |||||||||||||
} | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
if (!headStarted) { | ||||||||||||||
const headStartMatch = chunkString.match(patternHeadStart) | ||||||||||||||
if (headStartMatch) { | ||||||||||||||
headStarted = true | ||||||||||||||
const index = headStartMatch.index! | ||||||||||||||
const headTag = headStartMatch[0] | ||||||||||||||
const remaining = chunkString.slice(index + headTag.length) | ||||||||||||||
finalPassThrough.write( | ||||||||||||||
chunkString.slice(0, index) + headTag + getBufferedRouterStream(), | ||||||||||||||
) | ||||||||||||||
// make sure to only write `remaining` until the next closing tag | ||||||||||||||
chunkString = remaining | ||||||||||||||
if (!headScriptInjected && !bodyStarted) { | ||||||||||||||
if (!headStarted) { | ||||||||||||||
const headStartMatch = chunkString.match(patternHeadStart) | ||||||||||||||
if (headStartMatch) { | ||||||||||||||
headStarted = true | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
if (headStarted) { | ||||||||||||||
const charsetMatch = chunkString.match(patternCharset) | ||||||||||||||
|
||||||||||||||
if (charsetMatch) { | ||||||||||||||
headScriptInjected = true | ||||||||||||||
const index = charsetMatch.index! + charsetMatch[0]!.length | ||||||||||||||
finalPassThrough.write( | ||||||||||||||
chunkString.slice(0, index) + getBufferedRouterStream(), | ||||||||||||||
) | ||||||||||||||
chunkString = chunkString.slice(index) | ||||||||||||||
} else { | ||||||||||||||
const headEndMatch = chunkString.match(patternHeadEnd) | ||||||||||||||
if (headEndMatch) { | ||||||||||||||
headScriptInjected = true | ||||||||||||||
const index = headEndMatch.index! | ||||||||||||||
finalPassThrough.write( | ||||||||||||||
chunkString.slice(0, index) + getBufferedRouterStream(), | ||||||||||||||
) | ||||||||||||||
chunkString = chunkString.slice(index) | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||
} | ||||||||||||||
|
||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Harden selectors: support optional quotes/spacing and avoid lowercasing.
Match
<meta charset>
with optional quotes/whitespace and allow other attributes; match the$TSR
script class among other attributes. This removes the need to lowercase the HTML.📝 Committable suggestion
🤖 Prompt for AI Agents