-
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
feat: align compile time and runtime plugin api between csr and ssr #12279
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
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 You can disable this status message by setting 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 (
|
Size Change: +539 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 #12279 +/- ##
========================================================
- Coverage 28.41% 28.06% -0.35%
========================================================
Files 492 493 +1
Lines 15168 15355 +187
Branches 3627 3695 +68
========================================================
Hits 4310 4310
- Misses 10086 10261 +175
- Partials 772 784 +12 ☔ View full report in Codecov by Sentry. |
const serverLoaderRequest = new Request(req.query.url, { | ||
headers: req.headers, | ||
}); | ||
const { serverLoaderArgs } = normalizeRequest(...args); |
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.
这里后续也改成从 normalizeHandlerArgs
返回吧
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.
+1
@@ -52,7 +52,7 @@ export default function InitialStateProvider(props: any) { | |||
appLoaded.current = true; | |||
} | |||
}, [loading]); | |||
if (loading && !appLoaded.current) { | |||
if (loading && !appLoaded.current && typeof window !== 'undefined') { |
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.
这个条件第一个判断呗,放到最后一个判断反而浪费时间去判断前面的了。
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.
ok
@@ -19,6 +19,11 @@ const bundlerWebpack: typeof import('@umijs/bundler-webpack') = | |||
const bundlerVite: typeof import('@umijs/bundler-vite') = | |||
lazyImportFromCurrentPkg('@umijs/bundler-vite'); | |||
|
|||
enum MetadataLoaderOmitKeys { | |||
Title = 'title', |
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.
枚举最好用 E
开头,接口最好用 I
开头。
枚举里要不然就全大写,要不就小写开头的驼峰,大写开头很奇怪。
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.
ok, next pr fixied
MetadataLoaderOmitKeys.Meta, | ||
]) as Omit< | ||
typeof args, | ||
MetadataLoaderOmitKeys.Title & MetadataLoaderOmitKeys.Meta |
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.
这里的类型有问题吧,Omit
等高级类型的第二个参数在选取列表时,不应该用 |
吗
@@ -153,7 +153,7 @@ export type { | |||
}); | |||
|
|||
api.onBeforeCompiler(async ({ opts }) => { | |||
const { builder = 'esbuild' } = api.config.ssr; | |||
const { builder = 'webpack' } = api.config.ssr; |
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.
为了防止突然把默认构建器给切换了,导致以前没显示的配置 builder
的人的 ssr 应用突然挂掉,最好给一个打印提示,如果用户没显示的配置 buidler
,就告诉用户现在开始使用 webpack 默认构建了,并附带一个指向 umi 文档 ssr 部分的链接,在 ssr 文档那块补一下为什么要切换到 webpack 以及不再使用 esbuild ssr 的说明。
从而防止如果有社区的用户在使用 ssr 但没配置 builder
,这次升级 umi 后构建挂了,导致的各种黑盒和非预期问题,减少一些争吵和怒气。
@@ -1,11 +1,12 @@ | |||
{{{ importsAhead }}} |
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.
这个新加的不会有 breaking change 么,假如有的人插入了各种脚本和样式,在 umi.ts
有这里为啥还需要再重复一次。
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.
内部 ssr 业务流程需要, 有 breaking change 的场景吗?
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.
直接来看这部分导入其实在 umi.ts
是有一份的,但是这里也有一份,相当于导入了两次吧,一个脚本在两处被导入就是不正常比较奇怪的,不应该这么做。
我理解这个文件只是一个导出 handler 给外部直接导入使用的导出型文件,这里如果加了这些(可能包含 css 导入等),相当于给一个专门用来导出的文件新增了副作用,这是很奇怪的,明明只是导入一个东西却被副作用干扰了,这在 tree shake 上也是奇怪的模块副作用 case 。
@@ -351,9 +351,15 @@ export function renderClient(opts: RenderClientOpts) { | |||
// @ts-ignore | |||
const metadata = window.__UMI_METADATA_LOADER_DATA__ || {}; | |||
|
|||
const hydtateHtmloptions = { |
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.
新声明 object 格式的变量时,在 TypeScript 里一定要加类型,因为它没法自动推断类型。
比如:
const obj: Record<string, any> = {}
const obj: ISomeType = {}
const obj = {} satisfies ISomeType
const obj = {} as const
const obj = {} as const satisfies ISomeType
const obj = {} as ISomeType
如果是 native 值(比如字符串、数字等),就没必要加类型了。
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.
ok, next pr fixed
* 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 <xiaoxiao.lh@antgroup.com> Co-authored-by: Jinbao1001 <nodewebli@163.com> * 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 <xiaoxiao.lh@antgroup.com> Co-authored-by: Jinbao1001 <nodewebli@163.com> * 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 <xiaoxiao.lh@antgroup.com> Co-authored-by: Jinbao1001 <nodewebli@163.com> * 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 <scdzwyxst@gmail.com> Co-authored-by: MadCcc <1075746765@qq.com> Co-authored-by: xiaoxiao <xiaoxiao.lh@antgroup.com> Co-authored-by: Jinbao1001 <nodewebli@163.com> Co-authored-by: Bravepg <gaopeng19960108@gmail.com>
...