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.

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: install local folder use npm pack #330

Merged
merged 4 commits into from
Jul 7, 2020
Merged

fix: install local folder use npm pack #330

merged 4 commits into from
Jul 7, 2020

Conversation

dead-horse
Copy link
Member

@dead-horse dead-horse commented Jul 7, 2020

之前的目录中可能有 node_modules 等不需要的文件,通过 npm pack 来打包,如果没有 npm,fallback 到直接 copy

return await utils.copyInstall(filepath, options);
try {
// use npm pack to ensure npmignore/gitignore/package.files work fine
const res = await cp.exec('npm pack', { cwd: filepath });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

万一 npm 命令不存在呢?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不存在会 fallback 到老的逻辑

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

老的逻辑安装是没问题的,不过在有些场景下会比较慢。

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我们自己去实现 pack 的过滤逻辑,要读 files/.npmignore/.gitignore,还有各种默认打包进去的规则,不太靠谱,而且绝大部分的运行环境肯定有 npm 的,只要有 node 就有 npm。

@dead-horse
Copy link
Member Author

CI 过了, @fengmk2

@codecov
Copy link

codecov bot commented Jul 7, 2020

Codecov Report

Merging #330 into master will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #330      +/-   ##
==========================================
+ Coverage   93.04%   93.08%   +0.03%     
==========================================
  Files          30       30              
  Lines        1928     1938      +10     
==========================================
+ Hits         1794     1804      +10     
  Misses        134      134              
Impacted Files Coverage Δ
lib/download/local.js 95.00% <100.00%> (+1.66%) ⬆️
lib/utils.js 92.44% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a92ab3b...c62dfc7. Read the comment docs.

@fengmk2 fengmk2 merged commit 1320f30 into master Jul 7, 2020
@fengmk2 fengmk2 deleted the local-with-pack branch July 7, 2020 14:12
@fengmk2
Copy link
Member

fengmk2 commented Jul 7, 2020

4.9.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants