fix: metadata and hydrate root mismatched between csr and ssr #12220
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
added 2 commits
March 13, 2024 19:37
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Size Change: +1.86 kB (0%) Total Size: 9.9 MB
ℹ️ View Unchanged
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature/new-unio-ssr #12220 +/- ##
========================================================
- Coverage 28.41% 28.23% -0.19%
========================================================
Files 492 493 +1
Lines 15168 15266 +98
Branches 3627 3667 +40
========================================================
Hits 4310 4310
- Misses 10086 10179 +93
- Partials 772 777 +5 ☔ View full report in Codecov by Sentry. |
added 2 commits
March 22, 2024 15:17
fz6m
requested changes
Mar 24, 2024
|
|
此 PR 因为评论太多了，可能影响在网页上查看变化的代码，如果太乱的话新开一个 PR 也可以。 因为可能需要改动的地方比较多，后续如有必要，我有时间时可以协助修订。 |
MadCcc
reviewed
Mar 27, 2024
| // TODO: 处理 head 标签，比如 favicon.ico 的一致性 | ||
| // TODO: root 支持配置 | ||
| return ( | ||
| <html lang={metadata?.lang || 'en'}> |
There was a problem hiding this comment.
lang 不能用 metadata 的配置了，客户端没有，水合会报错
There was a problem hiding this comment.
线下讨论：Umi 加 htmlLang: string or lang: string 静态配置，不允许通过 metadataLoader 来改，后续调研下 Next.js 的做法，因为只是 warning 暂时不急
|
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 WalkthroughThe recent updates aim to enhance server-side rendering (SSR) capabilities by introducing new properties like Changes
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
packages/server/package.jsonis excluded by!**/*.jsonpnpm-lock.yamlis excluded by!**/*.yaml
Files selected for processing (11)
- examples/ssr-demo/.umirc.ts (1 hunks)
- packages/preset-umi/src/features/ssr/ssr.ts (1 hunks)
- packages/preset-umi/src/features/tmpFiles/tmpFiles.ts (2 hunks)
- packages/preset-umi/templates/server.tpl (1 hunks)
- packages/renderer-react/src/browser.tsx (3 hunks)
- packages/renderer-react/src/html.tsx (1 hunks)
- packages/renderer-react/src/server.tsx (2 hunks)
- packages/renderer-react/src/types.ts (2 hunks)
- packages/server/src/server.ts (1 hunks)
- packages/server/src/ssr.ts (6 hunks)
- packages/server/src/types.ts (2 hunks)
Files skipped from review due to trivial changes (1)
- packages/server/src/server.ts
Additional comments (16)
examples/ssr-demo/.umirc.ts (2)
- 7-27: Consider using meaningful values for the example configuration options. For instance, instead of
name: 'test', use a more descriptive name that aligns with common front-end concepts, likename: 'description'. Additionally, ensure that URLs used in the configuration (e.g.,https://a.com/b.js,https://a.com/b.css,https://www.baidu.com) are valid and won't cause errors when loading the demo. It's important for examples to be functional and error-free to serve as a reliable reference for users.- 10-11: The
hydrateFromHtmloption is set totrue, which aligns with the PR's objective to enhance SSR hydration. Ensure that this setting is consistently applied across all examples and documentation to avoid confusion.packages/renderer-react/src/server.tsx (1)
- 53-57: The conditional rendering logic based on the
hydrateFromHtmloption is correctly implemented. This allows for flexibility in how the application is hydrated, catering to different SSR scenarios. Ensure that this behavior is well-documented, so users understand when and how to use thehydrateFromHtmloption effectively.packages/renderer-react/src/types.ts (1)
- 46-74: The addition of
IRootComponentOptions,IHtmlProps, andIScriptinterfaces/types supports the new SSR features well. These changes enhance the framework's ability to handle diverse SSR scenarios. Ensure that these interfaces are used consistently across the codebase and that their properties are adequately documented for developers.packages/server/src/types.ts (2)
- 1-23: The introduction of the
IOptsinterface adds a structured way to define server-side rendering options. However, ensure that types such asstylesandfaviconsare correctly defined to match their actual usage in the server logic. It's important that these types accurately reflect the data they're intended to represent, including the possibility of being either a string or an object with specific properties.- 65-69: The update to the
IMetadatainterface to include properties fromIOptsis a good approach to ensure consistency across the server and client sides. Make sure that the documentation clearly explains how these properties are used and how they affect the rendering process, especially for new users of the framework.packages/preset-umi/templates/server.tpl (1)
- 54-56: The inclusion of
metadata,scripts, andhydrateFromHtmlin thecreateOptsobject aligns with the PR's objectives to enhance SSR functionality. Ensure that these options are properly documented and examples are provided to demonstrate their usage. This will help developers understand how to leverage these new capabilities in their applications.packages/renderer-react/src/html.tsx (1)
- 39-115: The
Htmlcomponent is well-structured and plays a crucial role in enabling dynamic HTML document customization based on user-provided props. Ensure that the handling of metadata, scripts, styles, and other elements is consistent and efficient. Also, consider the impact of usingdangerouslySetInnerHTMLand ensure it's used safely to prevent XSS vulnerabilities.packages/preset-umi/src/features/ssr/ssr.ts (1)
- 30-30: The addition of the
hydrateFromHtmlproperty to the SSR configuration schema is a significant enhancement, allowing developers to control the hydration process more granely. Ensure that this property is well-documented, including examples of when to set it totrueorfalse, to help developers understand its impact on their applications.packages/renderer-react/src/browser.tsx (2)
- 99-103: The addition of the
hydrateFromHtmloption inRenderClientOptsprovides flexibility in controlling the hydration process. It's important to ensure that this option is clearly documented, explaining its default behavior and how it affects the rendering process. This will help developers make informed decisions when configuring their SSR applications.- 343-348: The update to use
ReactDOM.hydrateRootwith theHtmlcomponent and thehydrateFromHtmloption is correctly implemented. This change aligns with the enhancements introduced in the PR for optimizing the hydration mechanism. Ensure that the implications of this change are thoroughly tested, especially in scenarios wherehydrateFromHtmlis set tofalse.packages/server/src/ssr.ts (4)
- 1-1: The addition of
mergeWithfrom 'lodash.mergewith' is appropriate for merging objects with custom logic. Ensure thatlodash.mergewithis included in the project's dependencies to avoid runtime errors.- 41-43: The
metadata,scripts, andhydrateFromHtmlproperties have been added toCreateRequestHandlerOptions. It's important to ensure that these properties are documented and their types are correctly defined inIOptsto maintain type safety and clarity for developers.- 129-145: The use of
mergeWithto mergemetadataLoaderDatawithopts.metadatais a good approach for combining metadata from different sources. However, ensure that the custom merge function correctly handles all expected data types, not just arrays, to avoid unexpected behavior.- 164-164: The
hydrateFromHtmlproperty is correctly passed to the context object. This is crucial for controlling the hydration behavior based on the configuration. Ensure that this property is properly handled in the client-side rendering logic to respect the server-side configuration.packages/preset-umi/src/features/tmpFiles/tmpFiles.ts (1)
- 496-511: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [499-536]
The extraction and writing of
headScripts,scripts,styles,title,favicons,links,metas, andhydrateFromHtmlproperties to a temporary file is a significant change. Ensure that all these properties are correctly extracted fromapi.configand that the JSON structure in the temporary file matches the expected format for further processing. Additionally, consider validating the presence and types of these properties to avoid runtime errors.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
packages/server/package.jsonis excluded by!**/*.jsonpnpm-lock.yamlis excluded by!**/*.yaml
Files selected for processing (2)
- packages/preset-umi/src/commands/dev/getBabelOpts.ts (1 hunks)
- packages/preset-umi/src/features/routePreloadOnLoad/routePreloadOnLoad.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- packages/preset-umi/src/features/routePreloadOnLoad/routePreloadOnLoad.ts
Additional comments (1)
packages/preset-umi/src/commands/dev/getBabelOpts.ts (1)
- 6-9: The adjustment in formatting for the
shouldUseAutomaticRuntimecomparison operation enhances readability without altering the logic. This is a good practice for maintaining code clarity, especially in complex conditional checks.
There was a problem hiding this comment.
Actionable comments posted: 1
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (10)
- examples/ssr-demo/.umirc.ts (1 hunks)
- packages/preset-umi/src/features/ssr/ssr.ts (1 hunks)
- packages/preset-umi/src/features/tmpFiles/tmpFiles.ts (3 hunks)
- packages/preset-umi/templates/server.tpl (1 hunks)
- packages/renderer-react/src/browser.tsx (3 hunks)
- packages/renderer-react/src/html.tsx (1 hunks)
- packages/renderer-react/src/server.tsx (2 hunks)
- packages/renderer-react/src/types.ts (2 hunks)
- packages/server/src/ssr.ts (9 hunks)
- packages/server/src/types.ts (2 hunks)
Files skipped from review as they are similar to previous changes (9)
- examples/ssr-demo/.umirc.ts
- packages/preset-umi/src/features/ssr/ssr.ts
- packages/preset-umi/src/features/tmpFiles/tmpFiles.ts
- packages/preset-umi/templates/server.tpl
- packages/renderer-react/src/browser.tsx
- packages/renderer-react/src/html.tsx
- packages/renderer-react/src/server.tsx
- packages/renderer-react/src/types.ts
- packages/server/src/types.ts
Additional Context Used
Additional comments not posted (7)
packages/server/src/ssr.ts (7)
8-8: The import formergeWithfrom 'lodash.mergewith' is mentioned in the summary but not visible in the provided code. Ensure it's used appropriately if it's indeed part of the changes, or remove it if not needed.
129-141: The logic for mergingmetadataLoaderDataintoopts.metadatauses a direct assignment for non-array values and concatenation for arrays. This approach might not handle deep merging of objects correctly. Consider using a deep merge utility if nested object merging is required.Consider using a deep merge utility like
lodash.mergefor complex object structures to ensure all nested properties are correctly merged.
159-160: The assignment ofmetadataandhydrateFromRoot(should behydrateFromHtmlas per previous comment) directly fromoptstocontextis straightforward. Ensure thathydrateFromHtmlis correctly utilized downstream to control hydration behavior as intended.Ensure the
hydrateFromHtmlproperty is correctly utilized in the hydration logic and that its true/false states are handled as expected.
250-254: The introduction ofIExpressRequestHandlerArgsandIWorkerRequestHandlerArgstypes is a good practice for clarifying the function signatures and improving code readability. Ensure these types are correctly applied in all relevant places.
260-383: ThenormalizeHandlerArgsfunction is a significant addition that abstracts the handling of different request handler arguments for Express and worker modes. This abstraction improves code modularity and readability. However, ensure thorough testing, especially around the branching logic for different modes and the correct assignment of request properties.
427-438: The deprecation warning increateUmiHandleris a good practice for informing users about outdated methods. Ensure that all documentation and examples are updated to reflect this change and guide users towards the new preferred method.Update documentation and examples to reflect the deprecation of
renderRootand promote the use ofrequestHandlerinstead.
461-469: Similar to the previous comment, the deprecation warning increateUmiServerLoaderis important for guiding users away from deprecated methods. Again, ensure documentation and examples are updated accordingly.Ensure all references to
serverLoaderin documentation and examples are updated to encourage the use ofrequestHandler.
There was a problem hiding this comment.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (1)
- packages/renderer-react/src/browser.tsx (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/renderer-react/src/browser.tsx
Additional Context Used
PeachScript
changed the title
PeachScript
reviewed
Apr 1, 2024
| <meta key={em.name} name={em.name} content={em.content} /> | ||
| ))} | ||
|
|
||
| {metadata?.links?.map((link: Record<string, string>, key: number) => { |
There was a problem hiding this comment.
后续限制一下，不允许通过 metadataLoader 设置非 tdk 的配置
PeachScript
reviewed
Apr 1, 2024
| metadata, | ||
| renderFromRoot, | ||
| }: React.PropsWithChildren<IHtmlProps>) { | ||
| // TODO: 处理 head 标签，比如 favicon.ico 的一致性 |
|
感谢 PR！如果有兴趣一起参与维护 Umi，可加入我们的 Umi Contributor 群。加入方式是先用钉钉扫下方二维码加我钉钉，记得注明 github id，然后我会拉你到群里。 |
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.
Summary by CodeRabbit
hydrateFromHtmlproperty for improved HTML hydration support in SSR.HtmlReact component to dynamically generate HTML structure with metadata, links, styles, scripts, and children content.Htmlcomponent.Htmlcomponent based onhydrateFromHtml.getClientRootComponentfunction to accept new options for improved flexibility.