-
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: router base not working for ssr and export static #12140
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #12140 +/- ##
==========================================
- Coverage 27.96% 27.96% -0.01%
==========================================
Files 499 499
Lines 15773 15775 +2
Branches 3789 3793 +4
==========================================
Hits 4411 4411
- Misses 10557 10559 +2
Partials 805 805 ☔ View full report in Codecov by Sentry. |
目前项目是布署在 github.io 的二级目录下, 这种场景应该是挺多的. 现在每次发版前都需要对静态产物中的 a 标签 href 进行手动替换, 还挺麻烦的, 希望能尽快处理一下这个 pr. 感谢! @PeachScript |
|
||
// If router has a basename, location should be concatenated with that basename | ||
const finalLocation = `${ | ||
basename.endsWith('/') ? basename.slice(0, -1) : basename |
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.
这里可能有问题,getClientRootComponent
会同时用于 ssr 和 ssg,在 ssr 场景下是作为 express middleware 的形态存在的,也就是说在 ssr 场景下这里接收到的 location 来源于 request.url
是包含 basename 的,得抹平一下两个场景下的差异,目前的想法:
getClientRootComponent
接收的 location 是带 basename 的 location- ssg
exportStatic
调用预渲染时带上 basename,ref - history 初始化的时候也得处理下 basename,但这个和 href 没啥关系,可以看下要不要在这个 PR 里一起处理掉
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.
还有其他可能的影响吗,调整后存量有 base 的 ssr 或 ssg 项目会被影响吧。
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.
存量项目只要没配 base 都是不受影响的,因为目前就是把 basename 当做 /
,如果配了 base,那么:
- ssr 存量项目配了 base 的话,产出的 middleware 应该也是用不了的,因为带 base 的 url 匹配不到路由,ref:
umi/packages/server/src/ssr.ts
Line 102 in 60896f5
const matches = matchRoutesForSSR(url, routes); - ssg 存量项目配了 base 目前是存在 HTML 里 href 不正确的问题(虽然 Link 跳转可以正常工作但 SEO 的确有问题),这个改动正是为了修复该问题
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.
这里可能有问题,
getClientRootComponent
会同时用于 ssr 和 ssg,在 ssr 场景下是作为 express middleware 的形态存在的,也就是说在 ssr 场景下这里接收到的 location 来源于request.url
是包含 basename 的,得抹平一下两个场景下的差异,目前的想法:
getClientRootComponent
接收的 location 是带 basename 的 location- ssg
exportStatic
调用预渲染时带上 basename,ref- history 初始化的时候也得处理下 basename,但这个和 href 没啥关系,可以看下要不要在这个 PR 里一起处理掉
我重新整理了一遍代码, 增加了 ssr 的案例, 第1点和第2点已经改好了.
对于第3点不是太明白, 目前看 history 是带上 basename 的. @PeachScript
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.
@PeachScript 麻烦再审核一下
可以先把 base 用 define 作为环境变量注入到项目里然后给 href 的地方加上前缀,比如: // .umirc.ts
define: { 'process.env.BASE_URL': '/base/' } <Link to={`${process.env.BASE_URL}/path/to/url`} /> 或者把 |
这个方案不合适吧,SPA 只需要感知 base 内的路由;如果只是临时解的话还不如写个脚本修正产物 👀 |
同意, 还是得考虑抹平一下两个场景下的差异, 我再修改一下. |
如果用脚本去修改产物或者 html ,毕竟是一种外部行为,可能匹配不能精准到每个地方,不够完备。 对于临时解,我感觉还是要从源码出发做调整,如果是只为了 SEO 服务,机器人的请求是已知的(通过 bot 标头判断,主流搜索引擎都有其公开特征,或 CF 的 WAF 规则可以精准知道所有已知的 SEO bot ),可以考虑给 SEO 的请求加一个标识,之后生成不一样的 href 即可。 |
不考虑临时解了吧,上面提临时解是相对于 define 来说的,现在 PR 调整好就是彻底解了 |
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe recent updates focus on enhancing server-side rendering (SSR) and static export functionalities in the Umi framework by properly handling the Changes
Assessment against linked issues
Poem
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
ssr
exportStatic
not handled with base
@Jinbao1001 看看这个修复与近期 ssr 的变更是否有冲突 |
@Jinbao1001 麻烦看一下这个修复 |
sorry, 刚看到! |
@jaykou25 明天发版. |
fix: #11233
修复开启
ssr
后输出产物中的前端路由链接href
没有正确的base
前缀的问题. 该问题会影响 SEO 的跳转.Summary by CodeRabbit