fix: add SSR support for DOMPurify and fix streaming initial output #1530
Conversation
- Add getDOMPurify utility function to support both server-side and client-side environments - Fix streaming initial output by computing output synchronously on first render - Refactor useStreaming hook to improve incomplete markdown placeholder handling - Update Renderer to use getDOMPurify utility instead of direct import
|
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthrough将对 DOMPurify 的直接导入替换为跨环境工厂函数 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 分钟
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @JUNERDD, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively resolves two distinct issues: SSR compatibility for DOMPurify and a rendering glitch with initial streaming output. The introduction of the getDOMPurify utility is a clean and appropriate solution for handling environment differences. The refactoring within the useStreaming hook to use useState's lazy initializer is a solid fix for the initial empty content problem and improves the code's structure. I have one suggestion to improve performance in the new getDOMPurify utility.
| import DOMPurify from 'dompurify'; | ||
| import { JSDOM } from 'jsdom'; | ||
|
|
||
| /** | ||
| * Returns the DOMPurify instance, compatible with both server-side (Node.js) and client-side (browser) environments. | ||
| * | ||
| * On the server, it creates a DOMPurify instance with a jsdom window; on the client, it returns the browser's DOMPurify. | ||
| * | ||
| * @see https://github.com/cure53/DOMPurify?tab=readme-ov-file#running-dompurify-on-the-server | ||
| */ | ||
| export function getDOMPurify() { | ||
| if (typeof window === 'undefined') { | ||
| const jsWindow = new JSDOM('').window; | ||
| return DOMPurify(jsWindow); | ||
| } | ||
|
|
||
| return DOMPurify; | ||
| } |
There was a problem hiding this comment.
Creating a new JSDOM instance on every call to getDOMPurify on the server can be inefficient. It's better to create the server-side DOMPurify instance once and cache it for subsequent calls. This will improve performance in an SSR environment by avoiding the overhead of creating a new JSDOM window repeatedly.
import DOMPurify from 'dompurify';
import { JSDOM } from 'jsdom';
let serverDOMPurify: ReturnType<typeof DOMPurify>;
/**
* Returns the DOMPurify instance, compatible with both server-side (Node.js) and client-side (browser) environments.
*
* On the server, it creates a DOMPurify instance with a jsdom window; on the client, it returns the browser's DOMPurify.
*
* @see https://github.com/cure53/DOMPurify?tab=readme-ov-file#running-dompurify-on-the-server
*/
export function getDOMPurify() {
if (typeof window === 'undefined') {
if (!serverDOMPurify) {
const jsWindow = new JSDOM('').window;
serverDOMPurify = DOMPurify(jsWindow as any);
}
return serverDOMPurify;
}
return DOMPurify;
}There was a problem hiding this comment.
Will this cause a memory leak for the serverDOMPurify variable?
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/x-markdown/src/XMarkdown/utils/getDOMPurify.ts (1)
11-18: 考虑缓存服务端的 DOMPurify 实例以提升性能。当前实现在每次调用时都会创建新的 JSDOM 实例（在服务端），这可能导致性能开销。建议缓存服务端实例：
+let cachedServerDOMPurify: any; + export function getDOMPurify() { if (typeof window === 'undefined') { + if (cachedServerDOMPurify) { + return cachedServerDOMPurify; + } const jsWindow = new JSDOM('').window; - return DOMPurify(jsWindow); + cachedServerDOMPurify = DOMPurify(jsWindow); + return cachedServerDOMPurify; } return DOMPurify; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/x-markdown/src/XMarkdown/__test__/Renderer.test.ts(3 hunks)packages/x-markdown/src/XMarkdown/core/Renderer.ts(2 hunks)packages/x-markdown/src/XMarkdown/hooks/useStreaming.ts(3 hunks)packages/x-markdown/src/XMarkdown/utils/getDOMPurify.ts(1 hunks)packages/x-markdown/src/XMarkdown/utils/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
packages/x-markdown/src/XMarkdown/utils/getDOMPurify.ts (1)
packages/x-markdown/src/XMarkdown/utils/index.ts (1)
getDOMPurify(1-1)
packages/x-markdown/src/XMarkdown/__test__/Renderer.test.ts (1)
packages/x-markdown/src/XMarkdown/utils/getDOMPurify.ts (1)
getDOMPurify(11-18)
packages/x-markdown/src/XMarkdown/core/Renderer.ts (1)
packages/x-markdown/src/XMarkdown/utils/getDOMPurify.ts (1)
getDOMPurify(11-18)
🔇 Additional comments (13)
packages/x-markdown/src/XMarkdown/__test__/Renderer.test.ts (3)
3-3: LGTM！测试文件正确引入了新的工具函数。测试文件现在通过
getDOMPurify工具函数获取 DOMPurify 实例，这与生产代码的变更保持一致。
779-780: 测试模式正确实现。测试现在通过调用
getDOMPurify()获取 DOMPurify 实例后再进行 spy，这与新的工具函数模式相匹配，确保了测试的准确性。
811-812: 测试模式正确实现。与第 779 行类似，这里正确地通过
getDOMPurify()获取实例后再创建 spy，保证了测试覆盖的有效性。packages/x-markdown/src/XMarkdown/utils/index.ts (1)
1-1: LGTM！标准的模块导出。通过 utils/index.ts 重新导出
getDOMPurify，使其可以从 utils 模块直接访问，这是标准的桶式导出模式。packages/x-markdown/src/XMarkdown/core/Renderer.ts (2)
7-7: LGTM！正确引入跨环境工具函数。Renderer 现在通过工具函数获取 DOMPurify 实例，这使得代码能够在 SSR 环境中正常工作。
146-148: SSR 支持正确实现。通过在运行时调用
getDOMPurify()获取实例而不是在模块加载时直接导入，成功解决了 SSR 环境中 window 对象不存在的问题。packages/x-markdown/src/XMarkdown/utils/getDOMPurify.ts (1)
4-10: LGTM！文档清晰且引用了官方资源。JSDoc 注释清楚地说明了函数的用途和跨环境行为，并引用了 DOMPurify 官方文档作为参考，这是良好的实践。
packages/x-markdown/src/XMarkdown/hooks/useStreaming.ts (6)
2-2: 导入添加正确。新增的
StreamingOption导入是后续新函数所必需的，符合预期。
200-231: 纯函数提取合理。
getIncompleteMarkdownPlaceholder函数成功将不完整 Markdown 占位符逻辑提取为纯函数，提高了代码的可测试性和可维护性。特殊情况处理（Image 的单字符!和 Table 的多行判断）逻辑正确。
233-287: 流式处理逻辑提取合理。
computeStreamingOutput函数成功将核心流式处理逻辑集中化，提高了代码的可测试性。缓存重置机制（lines 243-247）可以正确处理输入流重置的情况，字符逐个处理的逻辑保持了原有行为。
300-316: 懒初始化修复了初始渲染为空的问题。使用
useState的懒初始化函数在首次渲染前同步计算初始输出，成功解决了流式模式下初始内容为空的问题。这是 PR 的核心修复。需要注意的是，对于大型输入，首次挂载时的同步计算可能会有性能开销，但这是为了修复空白初始渲染而做出的必要权衡。
326-333: 重构提升了代码质量。
processStreaming函数重构为使用提取的纯函数computeStreamingOutput，代码更简洁且易于测试，同时保持了原有的处理逻辑。
335-335: 依赖数组更新正确，但需注意特定使用场景下的性能考虑。在依赖数组中包含
components是正确的，因为processStreaming回调在第 330 行的computeStreamingOutput调用中使用了该参数。需要说明的是：如果父组件在每次渲染时都传递新的
components对象引用（例如components={{...}}），会导致processStreaming的依赖变化，进而触发第 346 行的useEffect重新执行。若components是稳定的对象引用，则不会产生额外性能开销。建议在父组件中使用useMemo缓存components对象，特别是当该对象包含复杂的组件映射时。
|
No dependency changes detected. Learn more about Socket for GitHub. 👍 No dependency changes detected in pull request |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/x-markdown/package.json(1 hunks)packages/x-markdown/src/XMarkdown/utils/getDOMPurify.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/x-markdown/src/XMarkdown/utils/getDOMPurify.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test
- GitHub Check: size
- GitHub Check: build preview
🔇 Additional comments (1)
packages/x-markdown/package.json (1)
61-61: 类型定义依赖位置正确。@types/jsdom 正确放置在 devDependencies 中。
Bundle ReportChanges will increase total bundle size by 1.19MB (35.27%) ⬆️
Affected Assets, Files, and Routes:view changes for bundle: antdx-array-pushAssets Changed:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/x-markdown/src/XMarkdown/utils/getDOMPurify.ts (1)
3-4: 考虑添加注释说明 jsdomInstance 的用途。
jsdomInstance在第 15 行被赋值后再未被读取。如果保留此引用是为了防止 jsdom window 对象被垃圾回收（确保serverDOMPurify依赖的 window 对象在模块生命周期内有效），建议添加注释说明；如果不需要保留引用，可以移除该变量。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/x-markdown/src/XMarkdown/utils/getDOMPurify.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-23T15:51:02.475Z
Learnt from: YumoImer
Repo: ant-design/x PR: 223
File: package.json:169-169
Timestamp: 2024-11-23T15:51:02.475Z
Learning: 在使用 pnpm 严格模式时，即使存在间接依赖，也需要在 package.json 中直接声明依赖包，否则项目无法正常启动。
Applied to files:
packages/x-markdown/src/XMarkdown/utils/getDOMPurify.ts
🧬 Code graph analysis (1)
packages/x-markdown/src/XMarkdown/utils/getDOMPurify.ts (1)
packages/x-markdown/src/XMarkdown/utils/index.ts (1)
getDOMPurify(1-1)
🔇 Additional comments (1)
packages/x-markdown/src/XMarkdown/utils/getDOMPurify.ts (1)
6-21: 实现方案合理。动态加载 jsdom、处理多种导出模式以及错误处理逻辑都很稳健，能够适配不同的模块系统和运行环境。
| export function getDOMPurify(): ReturnType<typeof DOMPurify> { | ||
| if (typeof window === 'undefined') { | ||
| if (!serverDOMPurify) { | ||
| initializeServerDOMPurify(); | ||
| } | ||
| return serverDOMPurify || DOMPurify; | ||
| } | ||
|
|
||
| return DOMPurify; | ||
| } |
There was a problem hiding this comment.
服务端初始化失败时的回退逻辑存在问题。
第 39 行的回退逻辑 return serverDOMPurify || DOMPurify 在服务端环境下可能导致运行时错误：
- 如果
initializeServerDOMPurify()失败（例如 jsdom 加载失败），serverDOMPurify为null - 此时直接返回
DOMPurify在服务端无法工作，因为 DOMPurify 需要一个 window 对象 - 用户会收到关于缺少 window 的隐晦错误信息
建议改进回退策略：
export function getDOMPurify(): ReturnType<typeof DOMPurify> {
if (typeof window === 'undefined') {
if (!serverDOMPurify) {
initializeServerDOMPurify();
}
- return serverDOMPurify || DOMPurify;
+ if (!serverDOMPurify) {
+ throw new Error(
+ 'Failed to initialize DOMPurify for SSR. Please ensure jsdom is installed.'
+ );
+ }
+ return serverDOMPurify;
}
return DOMPurify;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function getDOMPurify(): ReturnType<typeof DOMPurify> { | |
| if (typeof window === 'undefined') { | |
| if (!serverDOMPurify) { | |
| initializeServerDOMPurify(); | |
| } | |
| return serverDOMPurify || DOMPurify; | |
| } | |
| return DOMPurify; | |
| } | |
| export function getDOMPurify(): ReturnType<typeof DOMPurify> { | |
| if (typeof window === 'undefined') { | |
| if (!serverDOMPurify) { | |
| initializeServerDOMPurify(); | |
| } | |
| if (!serverDOMPurify) { | |
| throw new Error( | |
| 'Failed to initialize DOMPurify for SSR. Please ensure jsdom is installed.' | |
| ); | |
| } | |
| return serverDOMPurify; | |
| } | |
| return DOMPurify; | |
| } |
| "clsx": "^2.1.1", | ||
| "dompurify": "^3.2.6", | ||
| "html-react-parser": "^5.2.5", | ||
| "jsdom": "^26.1.0", |
2 participants
中文版模板 / Chinese template
🤔 This is a ...
🔗 Related Issues
💡 Background and Solution
Problems Addressed
SSR Compatibility Issue: DOMPurify was directly imported, which caused errors in server-side rendering (SSR) environments like Next.js because DOMPurify requires a browser
windowobject. This prevented XMarkdown from working properly in SSR contexts.Streaming Initial Output Issue (related to useStreaming 增加 output 的初始值 #1522): When using streaming mode with
hasNextChunk: true, the initial render would show empty content because the output was computed asynchronously inuseEffect, causing a flash of empty content before the first update. This caused layout shift and content flickering in SSR scenarios or when other components rendered synchronously.Solution
Added
getDOMPurifyutility function: Created a utility that detects the environment and returns the appropriate DOMPurify instance:Fixed streaming initial output: Modified
useStreaminghook to compute the initial output synchronously usinguseStatelazy initializer, ensuring content is rendered immediately on first mount without waiting foruseEffect.Refactored Renderer: Updated Renderer to use the new
getDOMPurifyutility instead of direct import, ensuring consistent behavior across environments.API Changes
No breaking changes. The API remains the same, but now works correctly in SSR environments.
📝 Change Log
Summary by CodeRabbit
发布说明
重构（Refactor）
依赖项（Chores）
✏️ Tip: You can customize this high-level summary in your review settings.