-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
fix(ssr): render same root in server #12002
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #12002 +/- ##
==========================================
- Coverage 28.77% 28.76% -0.01%
==========================================
Files 489 489
Lines 14962 14965 +3
Branches 3554 3556 +2
==========================================
Hits 4305 4305
- Misses 9891 9894 +3
Partials 766 766 ☔ View full report in Codecov by Sentry. |
packages/server/src/ssr.ts
Outdated
@@ -314,7 +328,10 @@ export function createUmiHandler(opts: CreateRequestHandlerOptions) { | |||
throw new Error('no page resource'); | |||
} | |||
|
|||
return ReactDomServer.renderToNodeStream(jsx.element); | |||
// TODO: render 结果由 html 变为仅 app element,会影响到存量业务吗? |
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.
@gwuhaolin 兄弟,辛苦看下这里是否会出现问题,现在要解的问题是只让 react dom render id 为 root 的 app element 而不是整个 html element (详细背景见 PR 正文)。
但我理解 createUmiHandler
这个方法是给 serverless 用的?如果更改了会导致返回结果变为 <div id='root'>...</div>
(以前是 <html>...</html>
),这可能会造成存量业务影响,辛苦帮评估下。
如果不改这里,虽然不会对存量业务造成影响,但会导致 PR 正文描述的 react 18 方法服务端和客户端不一致的问题。
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.
基于 renderToNodeStream 的返回值套一个 Stream 把 beforeHtml 和 afterHtml 拼上能解吗
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.
renderToNodeStream
已经废弃了,这里能直接复用 createRequestHandler
吗
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.
还有更好的实现想法吗,其实把 before 和 after 的字符串直接发给用户其实是这部分没有 stream 流效果了,但头部其实没多少字符串,我不太熟悉流,这个地方有没有更好的解。
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.
renderToNodeStream
已经废弃了,这里能直接复用createRequestHandler
吗
这是另一个问题,这个 PR 先解 Breaking Change 的问题吧,后面 @gwuhaolin 看能不能改成 renderToPipeableStream
还有更好的实现想法吗
@MadCcc 有其他思路么,如果大家暂时没有更好的解法,可以先这么解?
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.
没用过不太了解 renderToNodeStream
,感觉是可以和 renderToPipeableStream
一样处理,before 和 after 都当成流的一部分写入
原来的代码有一些冗余导入,我个人不太喜欢,给整合到一起了,我认为滥用 |
@@ -60,7 +61,21 @@ export async function getClientRootComponent(opts: IHtmlProps) { | |||
{rootContainer} | |||
</AppContext.Provider> | |||
); | |||
return <Html {...opts}>{app}</Html>; | |||
const innerPlaceholderForGetHtml = '__UMI_SERVER_HTML_PLACEHOLDER__'; | |||
const htmlStringWithoutApp = ReactDOM.renderToString( |
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.
此处导出的 appBeforeHtml、appAfterHtml、appElement 三个值改成按需计算,不需要消费这三个变量的情况不需要执行这个耗时的操作。你这波操作是不是可以移到 createMarkupGenerator 函数里面去?
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.
不只是 createMarkupGenerator, createRequestHandler 也会用到,理论上 createUmiHandler 也应该这么做,才能保证没有问题。
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.
这个地方只序列化一次 head 的节点而已,没有几个 element ,和生成字符串拼接差不多,和真正耗时的 app 内部内容相比只是很小的性能损耗。
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.
在我上个 comment 中提到的问题需要确认下后续的计划,现在为了修复水合问题,需要让 createMarkupGenerator
/ createRequestHandler
/ createUmiHandler
都只 render app 节点,修改 createMarkupGenerator
/ createRequestHandler
的都是内部逻辑,产物上没有区别,主要是 createUmiHandler
的修改,会导致产物得到的内容有区别。
)}`, | ||
}} | ||
/> | ||
{loaderData && ( |
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.
感觉这里有问题吧:
-
不应该在
browser.tsx
里导入server.tsx
的东西,会造成 browser 运行时和 node 运行时混在一起,如果要复用应该单独抽取文件出来。 -
最重要的是这里服务端有
loaderData
的 html 发送给用户,而用户的 js 代码里 hydrate 的是一个没有loaderData
的 html 树,这不一致吧。
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.
这里虽然看上去是不一致的但是不会导致注水失败。这段脚本如果只是修改 window 参数的话可以考虑放到 head 里
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.
注水的具体机制我还没有深挖过,目前做的尝试发现不是所有情况下的 html 不一致都会打断注水,比如 head 里的标签数量等,会保留服务端的 html
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.
无论是否会注水正确,我认为都不应该从 html 开始 hydrate ,因为不管是社区共识还是 react 官方文档,都统一从 react root 节点开始 hydrate 保持一致,应该保持这个基本原则。
any progress ? |
Background
ant-design/ant-design#46517
在 Umi SSR 下使用 React 18 的
useId
时会出现 client 和 server 不一致的情况,但是根据useId
的描述只要 fiber 树一致结果就应该是相同的。检查发现 Server 渲染的 root 是从 html 开始的 (ref),而
hydrateRoot
是从document.getElementById('root')
开始注水的 (ref),所以导致两边 fiber 树不一致,useId
得到的结果也不同。Solution
renderToPipeable
接受的 jsx 改为从<div id="root">
开始的节点,与 client 一致,外层的 html 由字符串拼接。根据文档,使用
renderToPipeableStream
需要传入整个 html 作为渲染组件,同时hydrateRoot
也同样需要接受 html 作为注水组件,并且 dom 节点需要指定为document
。所以在注水阶段给
<Browser />
套了一层 server 同样的<Html>
来保证 ReactDOM 深度一致。但是由于客户端缺少信息,注水时在 head 中会有一些 html 不一致的警告:但是并不影响注水,生产下该警告会消失。这个问题暂时可以忽略,因为之前也不一致,只不过注水没有经过 html 元素。