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: not found module after ask installation #2761

Merged
merged 2 commits into from
Jun 5, 2021

Conversation

alexander-akait
Copy link
Member

What kind of change does this PR introduce?

bugfix

Did you add tests for your changes?

Hard to add, manually tested

If relevant, did you update the documentation?

No need

Summary

fixes #2716

Does this PR introduce a breaking change?

No

Other information

@webpack/cli-team

  • we need backport this solution, in webpack-dev-server and webpack itself, just replace require.resolve on this function
  • also we need small refactor, promptInstallation should not return something, only install package

@alexander-akait alexander-akait requested a review from a team as a code owner June 4, 2021 18:24
@@ -51,7 +51,7 @@ async function promptInstallation(packageName, preMessage) {
process.exit(2);
}

return utils.packageExists(packageName);
return packageName;
Copy link
Member Author

Choose a reason for hiding this comment

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

No need to check package on exists, because if it was failed we will exit with 2 code above

@codecov
Copy link

codecov bot commented Jun 4, 2021

Codecov Report

Merging #2761 (5118b05) into master (8f4077a) will decrease coverage by 0.10%.
The diff coverage is 90.90%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2761      +/-   ##
==========================================
- Coverage   96.02%   95.91%   -0.11%     
==========================================
  Files          31       31              
  Lines        1634     1641       +7     
  Branches      476      478       +2     
==========================================
+ Hits         1569     1574       +5     
- Misses         65       67       +2     
Impacted Files Coverage Δ
packages/webpack-cli/lib/utils/package-exists.js 91.66% <90.00%> (-8.34%) ⬇️
...kages/webpack-cli/lib/utils/prompt-installation.js 75.00% <100.00%> (ø)
packages/generators/src/addon-generator.ts 91.37% <0.00%> (-1.73%) ⬇️

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 8f4077a...5118b05. Read the comment docs.

@anshumanv
Copy link
Member

we need backport this solution, in webpack-dev-server and webpack itself, just replace require.resolve on this function

👍

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.

3 participants