fix: add react-helmet-async for alias #12511
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
|
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 Walkthrough这些更改在 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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/bundler-vite/src/config/transformer/alias.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- packages/bundler-vite/src/config/transformer/alias.ts
SK-Luffa
changed the title
| @@ -39,6 +39,7 @@ export default (function alias(userConfig) { | |||
| alias: [ | |||
| // to support less-loader ~ for local deps, refer: https://github.com/vitejs/vite/issues/2185 | |||
| { find: /^~/, replacement: '' }, | |||
| { find: /react-helmet-async/, replacement: 'react-helmet-async' }, | |||
There was a problem hiding this comment.
这个包未来依赖的版本会变化吗？如果不会变化或者确定暂时不会变化的话，可以锁定开头和结尾的
There was a problem hiding this comment.
主要是担心如果锁定开头和结尾，如果未来依赖的新的包，而这里没有进行修改的话，会不会再次产生这个问题？
There was a problem hiding this comment.
逻辑放到 packages/preset-umi/src/features/vite/vite.ts 下吧、加一下 window 判断 import { isWindows } from '../../utils/platform' 在增加别名逻辑 理论上直接控制 modifyConfig 就行 可以试试下面的代码
// packages/preset-umi/src/features/vite/vite.ts
import { isWindows } from '../../utils/platform'
api.modifyConfig((memo) => {
if (isWindows) {
memo.alias['react-helmet-async'] = 'react-helmet-async'
}
return memo;
});
|
@xierenyuan review 下？ |
|
另外，ci 挂了。 |
|
需要修理一下单测，参考 ci 结果。 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #12511 +/- ##
==========================================
- Coverage 28.04% 28.03% -0.02%
==========================================
Files 497 498 +1
Lines 15698 15704 +6
Branches 3764 3766 +2
==========================================
Hits 4402 4402
- Misses 10498 10504 +6
Partials 798 798 ☔ View full report in Codecov by Sentry. |
SK-Luffa
changed the title
SK-Luffa
changed the title
SK-Luffa
changed the title
SK-Luffa
changed the title
|
@SK-Luffa 辛苦看下我的评论修改后测试下(我没有 window 电脑) 辛苦了 |
| @@ -19,6 +20,9 @@ export default (api: IApi) => { | |||
|
|
|||
| api.modifyConfig((memo) => { | |||
| // like vite, use to pre-bundling dependencies in vite mode | |||
| if (isWindows) { | |||
| memo.alias['react-helmet-async'] = require.resolve('react-helmet-async'); | |||
There was a problem hiding this comment.
这个依赖在 renderer-react 下，不在 preset-umi 包里面，在 pnpm 严格的依赖结构下是获取不到的吧（此问题无法在此仓库内测试出来，但一般认为 require.resolve 在严格模式下只能取自己包的依赖）。
如果非要找这个包的绝对位置，应该先找 renderer-react ，之后基于 renderer-react 的位置作为起点找 react-helmet-async （参考 plugins/src/react-query 里找 @tanstack/query-core 的位置的代码）。
为了避免找绝对路径比较麻烦，值直接写包名可以吗？
| const rendererReactPath = resolveProjectDep({ | ||
| pkg: api.pkg, | ||
| cwd: api.cwd, | ||
| dep: RENDERER_REACT, |
There was a problem hiding this comment.
这里要先从本级包 preset-umi 开始找他依赖的 renderer-react ，起点是本身（因为 preset-umi 直接依赖了 renderer-react ），可以用 require.resolve ，不指定 paths 时默认从当前文件开始找，往上找也就是本级包开始找。
而 resolveProjectDep 是从用户的项目作为起点开始找，用户并不直接依赖 renderer-react ，所以严格的依赖结构里找不到，这里和 RQ 插件那边还是有一些区别的，因为那边 resolveProjectDep 的本意是，如果用户自己安装了，就使用 用户自己安装的，否则用自己依赖的（从本级找），而这里只是内部寻找，和用户依赖无关，但原理是一样的。
| import { dirname } from 'path'; | ||
| import { resolveProjectDep } from '../../utils/resolveProjectDep'; | ||
|
|
||
| // 解析react-helmet-async 在renderer-react中的路径 |
There was a problem hiding this comment.
另外这里还有一些细节问题，比如没有盘古之白，上面的导入中间有一个空行。
#10212 在windows上 报错的原因是因为这个库的esm版本内的依赖 仍是comjs 但是vite没有识别到，所以需要在vite内resolve 的alias 添加上react-helmet-async 我测试过在umi脚手架生成的项目内添加是完全可以解决的
但是不知道在源码里这样改是否正确
Summary by CodeRabbit
bundler-vite包中新增了别名规则，将react-helmet-async替换为自身。