fix: hydrate logic for ssr #12255
Merged
fix: hydrate logic for ssr #12255
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Important Auto Review SkippedAuto reviews are disabled on base/target branches other than the default branch. Please add the base/target branch pattern to the list of additional branches to be reviewed in the settings. Please check the settings in the CodeRabbit UI or the To trigger a single review, invoke the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
|
Size Change: +432 B (0%) Total Size: 9.9 MB
ℹ️ View Unchanged
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature/new-unio-ssr #12255 +/- ##
========================================================
- Coverage 28.41% 28.16% -0.25%
========================================================
Files 492 493 +1
Lines 15168 15300 +132
Branches 3627 3681 +54
========================================================
Hits 4310 4310
- Misses 10086 10208 +122
- Partials 772 782 +10 ☔ View full report in Codecov by Sentry. |
PeachScript
approved these changes
Apr 2, 2024
| const args = await getMarkupArgs({ api }); | ||
|
|
||
| // renderFromRoot = true, 将 html 中的 title, metas 标签逻辑全部交给 metadataLoader 合并逻辑处理 | ||
| const markupArgs = api.config?.ssr?.renderFromRoot |
|
|
||
| // renderFromRoot = true, 将 html 中的 title, metas 标签逻辑全部交给 metadataLoader 合并逻辑处理 | ||
| const markupArgs = api.config?.ssr?.renderFromRoot | ||
| ? omit(args, ['title', 'metas']) |
There was a problem hiding this comment.
想了下这个得放到 getMarkupArgs 里处理，renderFromRoot 的时候 addHTMLMetas 应当是不出发的，否则会出现执行但不生效的情况
| </> | ||
| ); | ||
| } | ||
| // @ts-ignore |
| typeof window === 'undefined' | ||
| ? manifest | ||
| : // @ts-ignore | ||
| window.__UMI_BUILD_MANIFEST_DATA__; |
There was a problem hiding this comment.
这里最好 as typeof manifest，不然后面类型推导应该有问题
| <script | ||
| suppressHydrationWarning |
There was a problem hiding this comment.
几个 suppressHydrationWarning 加上 FIXME: balabala....，后面接着处理这几个 warning
PeachScript
changed the title
|
对了，后续新 PR 从目标分支重新切分支开发，现在看都是在同一个分支，会带上此前全部的 commit |
|
感谢 PR！如果有兴趣一起参与维护 Umi，可加入我们的 Umi Contributor 群。加入方式是先用钉钉扫下方二维码加我钉钉，记得注明 github id，然后我会拉你到群里。 |
fz6m
reviewed
Apr 3, 2024
|
|
||
| // renderFromRoot = true, 将 html 中的 title, metas 标签逻辑全部交给 metadataLoader 合并逻辑处理 | ||
| const markupArgs = api.config?.ssr?.renderFromRoot | ||
| ? omit(args, ['title', 'metas']) |
There was a problem hiding this comment.
这里如果要用字符串，最好定义成枚举来用，字符串用一堆，感觉像 js 一样，不像 ts 。（下面的字符串列表也是）
| @@ -37,7 +37,6 @@ function createRouteMiddleware(opts: { api: IApi }) { | |||
| stats, | |||
| publicPath: opts.api.config.publicPath!, | |||
| }); | |||
|
|
|||
There was a problem hiding this comment.
为啥每次改动都会动到其他文件，在每次 commit 提交前，都应该在 vscode 的 git 可视化页面（或者其他本地 git 客户端工具）里仔细审查一遍发生变动的文件，有预期之外的文件变动就恢复它，如果发现错误就修正它，而不是写一写就无脑提交了。
| @@ -516,7 +516,7 @@ if (process.env.NODE_ENV === 'development') { | |||
| join(api.paths.absOutputPath, 'build-manifest.json'), | |||
| ), | |||
| env: JSON.stringify(api.env), | |||
| metadata: JSON.stringify({ | |||
| tplOpts: JSON.stringify({ | |||
There was a problem hiding this comment.
这里的命名，一眼看到以为是 template 模板用的参数，实际上他是 html head 里的参数信息，这两者的跳跃太大了，建议改名为 htmlHeadData 等与 html 相关或者 resources 之类的选项名。
另外 opts 这个的意思是一个可以配置的选项，而这里只是一个内部数据传递，我认为和 options 的语意差距很大，称不上是一个 opts 。
| @@ -526,6 +526,7 @@ if (process.env.NODE_ENV === 'development') { | |||
| scripts: scripts || [], | |||
| }), | |||
| renderFromRoot: api.config.ssr?.renderFromRoot ?? false, | |||
| mountElementId: api.config.mountElementId, | |||
There was a problem hiding this comment.
我发现楼主特别喜欢直接改动或者直接添加对象的值，而这个值往往一大串，比如：
renderFromRoot: api.config.ssr?.renderFromRoot ?? false,
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
mountElementId: api.config.mountElementId,
// ^^^^^^^^^^^^^^^^^^^^^^^^^这样会导致这一行很长，同时如果以后有其他地方需要复用这个值，很容易就被再写了一遍 api.config.XXXX 造成二次使用未被抽象（特别是在多人协作的时候）。
我的建议是在上面把这个变量定义出来先，特别是像 api.config.ssr?.renderFromRoot ?? false 这种：
const renderFromRoot = api.config.ssr?.renderFromRoot ?? false
// ...
renderFromRoot,| document, | ||
| <Html {...{ metadata, loaderData }}> | ||
| opts.renderFromRoot ? rootElement : document, | ||
| <Html {...{ metadata, loaderData, mountElementId: opts.mountElementId }}> |
| serverLoaderData, | ||
| ); | ||
| // types IMetadata | ||
| return ['title', 'description', 'keywords', 'lang', 'metas'].reduce( |
There was a problem hiding this comment.
-
这里太丑陋了，如果要用字符串，最好换成枚举，否则就是 JS 不是 TS 了。
-
要考虑性能问题，每次执行都创建这个字符串列表的话，放到更高的级别或全局会更好。
-
这里为啥不用 lodash 的 pick 方法，或者直接解构出来你想要的这几个值。
-
reduce最好少用，因为这个方法在 TS 里的类型支持太差了，很容易就不能自动推断了，导致要加额外的类型声明、类型断言，甚至是你这里都直接as any了，如果使用 lodash ，其实 reduce 是根本不会在代码库里出现的，你想要的方法在 lodash 里都有。
| // types IMetadata | ||
| return ['title', 'description', 'keywords', 'lang', 'metas'].reduce( | ||
| (acc, key) => { | ||
| if (Object.prototype.hasOwnProperty.call(result, key)) |
There was a problem hiding this comment.
没必要用这么底层的方法，看起来像在写 core-js 还是什么特别注重安全的底层库，普通的工具库、业务代码里最好不要出现这种原型方法，换言之，result?.[key] ，再比如 lodash 的 has 方法都更美观。
| styles?: string[]; | ||
| favicons?: string[]; | ||
| scripts?: (Record<string, string> | string)[]; | ||
| [key: string]: any; |
There was a problem hiding this comment.
到目前为止这两个 SSR 的 PR ，里面的类型都是一大坨一大坨的往代码里恩造，而且大部分都是重复的行，写类型要以复用为优先，多用 interface extends 。
我理解目前新增的所有类型或多或少都是重复的，在代码库里一搜就有写了两处的冗余代码，或者是内容都是重复的，没有继承复用的，很乱。
|
希望能精心准备一个时间，精心的逐行去优化代码，这个 PR 和上个 PR 貌似还遗留了不少问题。 累计到现在的代码变动让我一下子感觉是业务代码，感觉不像开源代码。 |
Jinbao1001
added a commit
that referenced
this pull request
Jun 27, 2024
* feat(preset-umi): unify request handler for ssr and always use stream (#12229) * refactor(preset): improve types for ssr request handler * refactor(preset-umi): provide unified request handler for ssr * refactor: add stream response header * refactor: correct ts lib usage * chore: update comment * refactor: warn for deprecated ssr exports * refactor: async-able for worker ssr request handler * refactor: update worker mode condition * refactor: type correct * feat: SSR support useServerInsertedHTML (#12247) * feat: SSR support useServerInsertedHTML * feat: ssr insert html * fix: string template * chore: update lock * fix: metadata and hydrate root mismatched between csr and ssr (#12220) * feat: ssr支持head body 配置 * feat: support ssr * fix: 回退metaloader执行逻辑判断 * fix: ts lint * feat: 优化部分ssr代码 * feat: add client metadata hydrate data * docs: hydtateFromRoot doc 修正 * fix: delete merge.with deps * fix: delete merge.with deps * fix: change hydrateFromRoot root to renderFromRoot * fix: NormalizeMeta component for render root * fix: NormalizeMeta component for render root --------- Co-authored-by: xiaoxiao <[email protected]> Co-authored-by: Jinbao1001 <[email protected]> * fix: hydrate logic for ssr (#12255) * feat: ssr支持head body 配置 * feat: support ssr * fix: 回退metaloader执行逻辑判断 * fix: ts lint * feat: 优化部分ssr代码 * feat: add client metadata hydrate data * docs: hydtateFromRoot doc 修正 * fix: delete merge.with deps * fix: delete merge.with deps * fix: change hydrateFromRoot root to renderFromRoot * fix: NormalizeMeta component for render root * fix: NormalizeMeta component for render root * fix: hydrate 遗留问题处理 * fix: ts-ignore window.__ * fix: 空格 * fix: lint --------- Co-authored-by: xiaoxiao <[email protected]> Co-authored-by: Jinbao1001 <[email protected]> * release: 4.0.0-canary.20240402.1 * fix: wrong react-dom server api for worker ssr mode (#12263) * fix: wrong react-dom server api for worker ssr mode * refactor: rename config * refactor: correct logic * fix: locked stream in ssr * feat: align compile time and runtime plugin api between csr and ssr (#12279) * feat: ssr支持head body 配置 * feat: support ssr * fix: 回退metaloader执行逻辑判断 * fix: ts lint * feat: 优化部分ssr代码 * feat: add client metadata hydrate data * docs: hydtateFromRoot doc 修正 * fix: delete merge.with deps * fix: delete merge.with deps * fix: change hydrateFromRoot root to renderFromRoot * fix: NormalizeMeta component for render root * fix: NormalizeMeta component for render root * fix: hydrate 遗留问题处理 * fix: ts-ignore window.__ * fix: 空格 * fix: lint * feat: addEntryCode to ssr and share the pluginManager * fix: curry and createPluginManager * feat: 提取公共 request 方法 * fix: serverloaderRequest * fix: serverloaderRequest * fix: serverloaderRequest * fix: serverloaderRequest * fix: curry * fix: curry * fix: 补充importsAhead and imports * fix: 条件判断更换 * fix: 代码优化 * fix: tslint * fix: tslint * fix: async function export * fix: add g_umi export and some fixded * fix: string export * fix: await clientroutePatch * feat: patchClientRoutes to async * fix: ssr禁用 inintial state loading * feat: 提供render钩子给主应用执行 * feat: 提供render钩子给主应用执行 * feat: 提供render钩子给主应用执行 * feat: to async * feat: stream render 钩子 * fix: 修改render执行时机 * fix: 移出otherwise逻辑 --------- Co-authored-by: xiaoxiao <[email protected]> Co-authored-by: Jinbao1001 <[email protected]> * feat: qiankun plugin compatible with ssr runtime (#12295) * feat: qiankun 插件支持 ssr * fix: cr * fix: 修改 external 的机制 * fix: 增加 ssr render 后，处理 qiankun 的生命周期 * feat: use prerender html directly in ssg (#12317) * feat: use prerender html directly in ssg * fix: ssg * fix: add bootstrap script * chore: 优先从环境变量读取 manifest 路径 (#12354) * fix: ssr manifest 正确读取环境变量 (#12357) * fix: ssr manifest 正确读取环境变量 * chore: 新增 ssr 黑盒变量 SSR_RESOURCE_DIR * refactor: improve platform checking logic for qiankun slave (#12331) * feat: qiankun 插件支持 ssr * fix: cr * fix: 修改 external 的机制 * fix: 增加 ssr render 后，处理 qiankun 的生命周期 * fix: qiankun slave ssr * fix: change ssr to isServer * chore: use process.env.SSR_RESOURCE_DIR replace SSR_RESOURCE_DIR (#12370) * chore: use process.env.ssr_manifest * chore: fomatcode * feat: provide useLoaderData for fallback serverLoader (#12339) * fix: ssr downgrade init * feat: add deprecated * chore: 代码优化 * fix: woker don't need to inject umi.js * chore: renderFromRoot to __SPECIAL_HTML_DO_NOT_USE_OR_YOU_WILL_BE_FIRED (#12384) * refactor: add renderFromRoot for tern theme (#12385) * feat: mako for ssr (#12409) * fix: ssr mako init * chore: 删除冗余webpack配置代码 * feat: finish mako bundler for ssr * feat: generator manifest * refactor: mako outputpath use bundler-webpack default value * feat: add mako hooks (#12412) * refactor(preset-umi): handle illegal route absPath in route preload (#12363) * refactor(preset-umi): handle unexpected route absPath in route preload * chore: correct logic * fix: renderClient opts miss internal vars (#12419) * release: 4.2.6-alpha.1 * release: 4.2.6-alpha.2 * release: 4.2.6-alpha.3 * release: 4.2.6-alpha.4 * feat: mako build and ssr finished * chore: delete code * chore: update lock * chore: update lock * chore: update lock * chore: update lock * chore: change plugins to makoPlugins * chore(deps): update mako version --------- Co-authored-by: Peach <[email protected]> Co-authored-by: MadCcc <[email protected]> Co-authored-by: xiaoxiao <[email protected]> Co-authored-by: Jinbao1001 <[email protected]> Co-authored-by: Bravepg <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
metadataLoader 的过滤项还是需要计算一下, 不然后续还是需要过滤处理
目前对 html(lang影响), umi.css(位置影响), dangerouslySetInnerHTML script 三个元素, 添加了 suppressHydrationWarning 忽略水合警告