feat(preset-umi): unify request handler for ssr and always use stream #12229
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
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
WalkthroughThe update introduces a shift towards a unified request handling mechanism across different modes, specifically focusing on worker and express modes. It deprecates older functions in favor of a new 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 (
|
|
Size Change: +748 B (0%) Total Size: 9.9 MB
ℹ️ View Unchanged
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #12229 +/- ##
==========================================
- Coverage 28.41% 28.33% -0.09%
==========================================
Files 492 492
Lines 15166 15209 +43
Branches 3626 3637 +11
==========================================
Hits 4310 4310
- Misses 10084 10124 +40
- Partials 772 775 +3 ☔ View full report in Codecov by Sentry. |
![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: 1
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
packages/server/tsconfig.jsonis excluded by:!**/*.json
Files selected for processing (2)
- packages/preset-umi/templates/server.tpl (1 hunks)
- packages/server/src/ssr.ts (4 hunks)
Additional comments: 3
packages/preset-umi/templates/server.tpl (1)
- 56-62: The deprecation comments for
renderRootandserverLoaderare clear and guide developers towards using the newrequestHandler. This aligns well with the PR's objectives of simplifying the API and unifying request handling.packages/server/src/ssr.ts (2)
- 238-386: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [246-405]
The modifications to
createRequestHandlerto support both worker and express modes are well-implemented. This change aligns with the PR's objectives of unifying request handling and improving SSR performance.
- 389-415: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [410-440]
The deprecation of
createUmiHandlerandcreateUmiServerLoaderis clearly communicated, guiding developers towards the new, unifiedcreateRequestHandler. This simplifies the API and aligns with the PR's objectives.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- packages/server/src/ssr.ts (4 hunks)
Additional comments: 5
packages/server/src/ssr.ts (5)
- 1-2: The addition of
RequestHandlerfrom@umijs/bundler-utils/compiled/expressaligns with the PR's objective to unify request handling. Ensure that all necessary types are imported for the new unified approach.- 242-246: Introduced types
IExpressRequestHandlerArgsandIWorkerRequestHandlerArgsto accommodate the different argument structures for express and worker modes. This is a good practice for type safety and clarity in function signatures.- 239-394: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [247-413]
The
createRequestHandlerfunction has been significantly refactored to support both worker and express modes, utilizing thenormalizeHandlerArgsfunction to handle different argument structures. This change aligns with the PR's goal of unifying request handling. However, ensure that error handling is robust, especially in theotherwisecases and when modifying responses.
- 418-420: The deprecation of
createUmiHandlerin favor ofcreateRequestHandlerwithmode: 'worker'is noted. Ensure that documentation and migration guides are updated to reflect this change and guide developers on how to transition to the new approach.- 446-448: Similarly, the deprecation of
createUmiServerLoaderis aligned with the PR's objectives. As withcreateUmiHandler, ensure that sufficient guidance is provided for developers to migrate to the new unified request handling method.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- packages/server/src/ssr.ts (4 hunks)
Additional comments: 6
packages/server/src/ssr.ts (6)
- 1-2: The addition of the
RequestHandlerimport from@umijs/bundler-utils/compiled/expressaligns with the PR's objective to unify request handling. Ensure that this import is utilized effectively in the newcreateRequestHandlerfunction.- 242-246: The introduction of
IExpressRequestHandlerArgsandIWorkerRequestHandlerArgstypes is a good practice for handling different argument structures in a unified request handler. This approach supports the PR's goal of streamlining request handling for both worker and express environments.- 252-375: The
normalizeHandlerArgsfunction is a critical part of unifying the request handling logic. It effectively handles the differences between express and worker environments by normalizing the arguments. However, ensure that the error handling is robust, especially in scenarios where the arguments do not match expected patterns.- 239-394: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [377-413]
The
createRequestHandlerfunction is the centerpiece of this PR, unifying the request handling process. It correctly differentiates between server loader requests and page requests, leveraging the new stream response feature for worker mode. Ensure comprehensive testing, particularly for edge cases in request handling and stream responses.
- 418-420: Marking
createUmiHandleras deprecated is in line with the PR's objective to streamline the API. It's important to communicate these deprecations clearly in the documentation to guide developers towards the newcreateRequestHandler.- 446-448: Similarly, deprecating
createUmiServerLoadersupports the goal of simplifying the API. Ensure that the documentation reflects these changes and provides clear migration paths for developers.
fz6m
reviewed
Mar 26, 2024
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (1)
- packages/server/src/ssr.ts (4 hunks)
Additional comments: 2
packages/server/src/ssr.ts (2)
- 242-246: The introduction of
IWorkerRequestHandlerArgswithFetchEventas a parameter is a good approach for handling worker mode requests. However, ensure that theFetchEventtype is correctly recognized in the context where this code runs, as it typically comes from a service worker or cloud worker environment.- 419-430: The deprecation warnings in
createUmiHandlerandcreateUmiServerLoaderare a good practice to inform users about the shift towards usingcreateRequestHandler. However, consider adding more detailed migration instructions or links to documentation in the warning messages to help developers transition smoothly.Also applies to: 453-461
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- packages/server/src/ssr.ts (4 hunks)
Additional comments: 4
packages/server/src/ssr.ts (4)
- 2-2: The import of
RequestHandleraligns with the PR's goal of unifying request handling. Good inclusion.- 242-246: The introduction of
IExpressRequestHandlerArgsandIWorkerRequestHandlerArgstypes is crucial for handling different request environments. Well-defined types.- 239-394: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [247-413]
The
createRequestHandlerfunction has been significantly refactored to support bothworkerandexpressmodes. This unification simplifies the API and aligns with the PR's objectives. However, ensure proper testing across both modes to verify the seamless operation and performance improvements.
- 426-429: The deprecation warnings for
createUmiHandlerandcreateUmiServerLoaderare clear. Consider adding guidance on migrating to the newcreateRequestHandlerfunction to assist developers in transitioning smoothly.Also applies to: 457-460
fz6m
approved these changes
Mar 27, 2024
sorrycc
approved these changes
Mar 28, 2024
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.
主要做了 3 处改动：
createUmiHandler、createUmiServerLoader的逻辑到createRequestHandler，对外统一使用requestHandler进行调用，目前支持响应worker和express两种 requestcreateUmiHandler复制为worker逻辑的同时，改用 Stream 响应请求，降低TTFBrenderRoot、severLoader导出均加上了deprecated，现在都可以用requestHandler替代Summary by CodeRabbit
requestHandlerfor improved performance and flexibility.