Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(SSR): 修复多级路由嵌套时，需要合并多级路由serverLoader的数据 & feature(SSR): 支持ssr降级，客户端兜底加载serverLoader数据 #11894

Merged
merged 5 commits into from
Nov 27, 2023
Merged

fix(SSR): 修复多级路由嵌套时，需要合并多级路由serverLoader的数据 & feature(SSR): 支持ssr降级，客户端兜底加载serverLoader数据 #11894

merged 5 commits into from
Nov 27, 2023

Conversation

gwuhaolin
Copy link
Member

No description provided.

fix(SSR): 修复多级路由嵌套时，需要合并多级路由serverLoader的数据
259ab3a 
feature(SSR): 支持ssr降级，客户端兜底加载serverLoader数据
@vercel Vercel
Copy link

vercel bot commented Nov 23, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
umi ⬜️ Ignored (Inspect) Visit Preview Nov 23, 2023 8:13pm

fix(SSR): 修复多级路由嵌套时，需要合并多级路由serverLoader的数据
dfdad62 
feature(SSR): 支持ssr降级，客户端兜底加载serverLoader数据
@codecov Codecov
Copy link

codecov bot commented Nov 23, 2023 •
edited
Loading

Codecov Report

Attention: 36 lines in your changes are missing coverage. Please review.

Comparison is base (2fc2a8c) 28.89% compared to head (69ca1dc) 28.83%.
Report is 4 commits behind head on master.

Files Patch % Lines
packages/renderer-react/src/appContext.ts 3.84% 22 Missing and 3 partials ⚠️
packages/renderer-react/src/dataFetcher.ts 12.50% 7 Missing ⚠️
packages/renderer-react/src/browser.tsx 0.00% 3 Missing ⚠️
packages/renderer-react/src/types.ts 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master   #11894      +/-   ##
==========================================
- Coverage   28.89%   28.83%   -0.07%     
==========================================
  Files         488      489       +1     
  Lines       14892    14930      +38     
  Branches     3538     3548      +10     
==========================================
+ Hits         4303     4305       +2     
- Misses       9829     9861      +32     
- Partials      760      764       +4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

fix(SSR): 修复多级路由嵌套时，需要合并多级路由serverLoader的数据
fbbcab3 
feature(SSR): 支持ssr降级，客户端兜底加载serverLoader数据
@fz6m fz6m self-requested a review November 23, 2023 14:50
@fz6m
Copy link
Contributor

fz6m commented Nov 23, 2023

  1. 我修正了一下类型和补了一个测试 case 。

  2. 代码里把 fetchServerLoader 提取出来又在 appContext.ts 里导入使用，这是一个很明显的相邻文件循环引用，综合之前楼主的 PR 中编写的代码来看，我发现你十分容易编写循环引用，虽然短时间可能发现不了问题，但难免在以后的维护或者某一个人参与改动后就造成很难排查的 undefined 问题，所以为了不留坑，建议在今后的编写中，务必应该深刻注意导入导出的循环引用问题，着重容易发生在 组件、函数逻辑多 的文件以及相邻文件之间，希望多加注意。

降级时机问题

我在测试过程中，发现这个失败降级貌似是个伪命题，如果没有 window.__UMI_LOADER_DATA__ ，无非就是运行崩了，此时为啥要降级，降级到以前正常的 cache 页面去吗，那此时 cache 的页面也是有 window.__UMI_LOADER_DATA__ 的。

何时会发生没有 window.__UMI_LOADER_DATA__ 的 HTML 传输给用户？我们假设就算有一个没有 window.__UMI_LOADER_DATA__ 的页面传给了用户，但 ssr 会把内容渲染好再给用户，也就是逻辑没问题的话，页面上已经有 ssr 后的正常渲染数据了，如果此时再执行一个降级逻辑，会造成在客户端又重复获取一次数据。你可以使用 examples/ssr-demo 这个项目并访问 http://localhost:8000/users/user2/info?fallback=1 来看到问题，页面又重复刷新了一次。

所以我认为降级是个伪命题，实际上不会发生没有 window.__UMI_LOADER_DATA__ 的情况，此部分代码应该去掉。

@gwuhaolin
Copy link
Member Author

gwuhaolin commented Nov 27, 2023 •
edited
Loading

  1. 我修正了一下类型和补了一个测试 case 。
  2. 代码里把 fetchServerLoader 提取出来又在 appContext.ts 里导入使用，这是一个很明显的相邻文件循环引用，综合之前楼主的 PR 中编写的代码来看，我发现你十分容易编写循环引用，虽然短时间可能发现不了问题，但难免在以后的维护或者某一个人参与改动后就造成很难排查的 undefined 问题，所以为了不留坑，建议在今后的编写中，务必应该深刻注意导入导出的循环引用问题，着重容易发生在 组件、函数逻辑多 的文件以及相邻文件之间，希望多加注意。

降级时机问题

我在测试过程中，发现这个失败降级貌似是个伪命题，如果没有 window.__UMI_LOADER_DATA__ ，无非就是运行崩了，此时为啥要降级，降级到以前正常的 cache 页面去吗，那此时 cache 的页面也是有 window.__UMI_LOADER_DATA__ 的。

何时会发生没有 window.__UMI_LOADER_DATA__ 的 HTML 传输给用户？我们假设就算有一个没有 window.__UMI_LOADER_DATA__ 的页面传给了用户，但 ssr 会把内容渲染好再给用户，也就是逻辑没问题的话，页面上已经有 ssr 后的正常渲染数据了，如果此时再执行一个降级逻辑，会造成在客户端又重复获取一次数据。你可以使用 examples/ssr-demo 这个项目并访问 http://localhost:8000/users/user2/info?fallback=1 来看到问题，页面又重复刷新了一次。

所以我认为降级是个伪命题，实际上不会发生没有 window.__UMI_LOADER_DATA__ 的情况，此部分代码应该去掉。

降级的思路是这样的：
如果SSR运行失败，就会直接返回构建出的静态dist/index.html，这个静态的HTML文件里是SPA有JS、CSS没有__UMI_LOADER_DATA__，在浏览器中会当作SPA去运行，由于没有__UMI_LOADER_DATA__数据，SPA运行时useServerLoaderData()拿不到数据，所以检测到没有__UMI_LOADER_DATA__时会去请求__serverLoader获取数据。

比如SSR运行失败时，直接返回静态dist/index.html内容如下：

<!DOCTYPE html><html><head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, user-scalable=no, initial-scale=1.0, maximum-scale=1.0, minimum-scale=1.0">
<meta http-equiv="X-UA-Compatible" content="ie=edge">
<link rel="shortcut icon" href="https://i.alipayobjects.com/common/favicon/favicon.ico">
<link rel="stylesheet" href="/umi.6aa72519.css">
</head>
<body>
<div id="root"></div>
<script src="/umi.59c66b4f.js" crossorigin="anonymous"></script>
</body></html>

在浏览器中会以SPA去运行React App。

当然最完美的降级不是去在浏览器中请求__serverLoader获取useServerLoaderData的数据，而是通过在浏览器中运行 exporrt 的 serverLoader() 去请求，但这样对umi的改动太大，同时要求serverLoader能运行在浏览器中，先搞一个简单版本的降级方案。

@fz6m fz6m merged commit 6d45e02 into umijs:master Nov 27, 2023
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fz6m fz6m fz6m approved these changes

@sorrycc sorrycc Awaiting requested review from sorrycc

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gwuhaolin @fz6m