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

feat: support --no-optional on install process #282

Merged
merged 3 commits into from
Nov 22, 2018
Merged

Conversation

fengmk2
Copy link
Member

@fengmk2 fengmk2 commented Nov 21, 2018

Don’t install optional dependencies.

like yarn does: https://yarnpkg.com/lang/en/docs/cli/install/#toc-yarn-install-ignore-optional

@codecov
Copy link

codecov bot commented Nov 21, 2018

Codecov Report

Merging #282 into master will decrease coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #282      +/-   ##
=========================================
- Coverage   92.83%   92.8%   -0.04%     
=========================================
  Files          27      27              
  Lines        1912    1917       +5     
  Branches      323     325       +2     
=========================================
+ Hits         1775    1779       +4     
- Misses        137     138       +1
Impacted Files Coverage Δ
lib/local_install.js 98.09% <100%> (ø) ⬆️
lib/install.js 97.91% <100%> (ø) ⬆️
bin/install.js 90.11% <100%> (+0.05%) ⬆️
lib/dependencies.js 100% <100%> (ø) ⬆️
lib/utils.js 90.63% <0%> (-0.34%) ⬇️

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 4166ccf...67ae0f4. Read the comment docs.

// link every packages' latest version to target directory
yield linkAllLatestVersion(rootPkgsMap, options);
// don't link latest versions on `npminstall pkgName` way
if (options.pkgs.length === 0) {
Copy link
Member Author

Choose a reason for hiding this comment

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

之前的实现有 bug

all[name] = optionalDependencies[name];
prod[name] = optionalDependencies[name];
if (options.ignoreOptionalDependencies) {
delete all[name];
Copy link
Member

Choose a reason for hiding this comment

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

如果在 dependencies 和 optionalDependencies 中都有,也会被删掉么?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.

npm 叫 --no-optional

@fengmk2
Copy link
Member Author

fengmk2 commented Nov 22, 2018

yarn 叫 ignore-optional
npm 是 --no-optional ...

@fengmk2 fengmk2 changed the title feat: support --ignore-optional feat: support --no-optional on install process Nov 22, 2018
@dead-horse
Copy link
Member

node 4 挂了

@fengmk2 fengmk2 merged commit 1c74e7d into master Nov 22, 2018
@fengmk2 fengmk2 deleted the ignore-optional branch November 22, 2018 06:16
@fengmk2
Copy link
Member Author

fengmk2 commented Nov 22, 2018

3.16.0

@fengmk2 fengmk2 mentioned this pull request Jan 2, 2023
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.

2 participants