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: use find-cache-dir to temporarily store certificates #2729

Merged
merged 1 commit into from
Sep 8, 2020
Merged

fix: use find-cache-dir to temporarily store certificates #2729

merged 1 commit into from
Sep 8, 2020

Conversation

wojtekmaj
Copy link
Contributor

@wojtekmaj wojtekmaj commented Sep 2, 2020

  • This is a bugfix
  • This is a feature
  • This is a code refactor
  • This is a test update
  • This is a docs update
  • This is a metadata update

For Bugs and Features; did you add new tests?

No :( It's a path change, so no new tests would apply here. There also didn't seem to be any tests I could modify.

Motivation / Use-Case

This fixes HTTPS server not working properly in Yarn PnP mode due to the way the packages are stored, unless the user manually unplugs the package. By using find-cache-dir, used widely (e.g. by babel-loader, copy-webpack-plugin), we ensure that Yarn PnP will correctly handle reading and writing these certificates.

See thread under one of Yarn package manager maintainer's tweet:
https://twitter.com/wojtekmaj91/status/1300849402001739776?s=20

Breaking Changes

Theoretically, if someone used node_modules/webpack-dev-server/ssl as a place to store their certificates for webpack-dev-server to consume, they may have a problem. However, there are options to provide certificates to webpack-dev-server in a better way.

@codecov
Copy link

codecov bot commented Sep 2, 2020

Codecov Report

Merging #2729 into v4 will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##               v4    #2729      +/-   ##
==========================================
+ Coverage   92.86%   92.88%   +0.02%     
==========================================
  Files          39       39              
  Lines        1304     1308       +4     
  Branches      349      350       +1     
==========================================
+ Hits         1211     1215       +4     
  Misses         89       89              
  Partials        4        4              
Impacted Files Coverage Δ
lib/utils/getCertificate.js 88.46% <100.00%> (+2.09%) ⬆️

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 c7e53b0...0ee4ddb. Read the comment docs.

@alexander-akait
Copy link
Member

Please send a PR to v4 branch, v3 accept only security fixes, I am with it, for webpack@5 we can use API, but we can improve it in future

@wojtekmaj wojtekmaj changed the base branch from master to v4 September 2, 2020 15:51
@hiroppy hiroppy merged commit 2090a41 into webpack:v4 Sep 8, 2020
@hiroppy
Copy link
Member

hiroppy commented Sep 8, 2020

thanks

@wojtekmaj wojtekmaj deleted the feature/find-cache-dir branch September 8, 2020 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants