refactor: handle rapid mode for tnpm and cnpm detection #11710
Merged
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
|
|
Size Change: +42 B (0%) Total Size: 10.2 MB
ℹ️ View Unchanged
|
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #11710 +/- ##
==========================================
- Coverage 28.93% 28.92% -0.01%
==========================================
Files 485 485
Lines 14773 14776 +3
Branches 3498 3500 +2
==========================================
Hits 4274 4274
- Misses 9739 9742 +3
Partials 760 760
☔ View full report in Codecov by Sentry. |
sorrycc
reviewed
Oct 8, 2023
| // 此处通过软链的目标文件夹名称来判断 node_modules 是否为 tnpm 的 npminstall 模式 | ||
| // 因为 rapid 模式下 package.json 中没有 __npminstall_done 的标记 | ||
| // ex. node_modules/_@umijs_utils@4.0.83@@umijs/utils/package.json | ||
| if (require.resolve('@umijs/utils/package').includes('_@umijs_utils@')) { |
There was a problem hiding this comment.
有两点需要确认的。
1、会不会有其他 package manager 也是这种格式？
2、tnpm 所有模式下都是这种吗？
There was a problem hiding this comment.
- 不确定，但常用的几个我看了都不是，pnpm、yarn、npm
- tnpm 仅 npminstall 模式下是这种，yarn 和 npm 模式都是普通目录结构，但也不会存在软链导致的序列化 OOM 问题
fz6m
reviewed
Oct 8, 2023
| // 此处通过软链的目标文件夹名称来判断 node_modules 是否为 tnpm 的 npminstall 模式 | ||
| // 因为 rapid 模式下 package.json 中没有 __npminstall_done 的标记 | ||
| // ex. node_modules/_@umijs_utils@4.0.83@@umijs/utils/package.json | ||
| if (require.resolve('@umijs/utils/package').includes('_@umijs_utils@')) { |
There was a problem hiding this comment.
cnpm 好像也有包前面会带 _ 开头的例子，在历史上存在过这种兼容，比如 这个正则 。
这个特征感觉不够唯一，感觉应该找一个更明确的特征，看看有没有 tnpm 的执行命令或者可执行文件，安装目录之类的会不会更好。
另外 __npminstall_done 可以和新特征一起用吧（ __npminstall_done || ... ）
There was a problem hiding this comment.
cnpm 也是软链结构，这个逻辑对它也是有效的，路径的确不是好的方式但目前没找到更合适的特征
__npminstall_done 之所以去掉是因为只要有这个标记路径一定是 node_modules/_xxx 这种结构的
There was a problem hiding this comment.
还有一处 __npminstall_done 在 packages/utils/src/npmClient.ts
PeachScript
changed the title
sorrycc
approved these changes
Oct 11, 2023
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.
检测 tnpm 和 cnpm 的相关逻辑支持 rapid 模式，主要有两处：
__npminstall_done标记改为软链的目标文件夹名称，以解决在 tnpm npminstall mode + rapid mode 时 snapshot 配置未设置导致缓存序列化 OOM 的问题__npminstall_done+_resolved的判定组合改为_resolved+.package-lock.json的组合，以解决在 rapid 模式下没有__npminstall_done字段导致 npmClient 识别错误的问题问题原因：和 tnpm 同学确认在 rapid 模式下使用 npminstall 模式安装，package.json 里不会有
__npminstall_done和_resoved标记，而雨燕云构建环境默认是 rapid 模式穷举情况：
注1：为什么 Bigfish 4 的持久缓存可以正常工作，因为 Bigfish merge 了 tnpm 提供的 cache config，它也设置了一份 snapshot 正好补齐