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

Use standard cache dir as default cacheDirectory #301

Merged
merged 1 commit into from
Oct 26, 2016

Conversation

fson
Copy link
Contributor

@fson fson commented Sep 18, 2016

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce?

  • Bugfix

What is the current behavior?

Currently, when cacheDirectory is set to true, babel-loader tries to use the operating system's temporary directory as a cache directory.

However, when running npm scripts as the root user, npm overrides the TMPDIR environment variable. This causes the cache files to be created in the project folder itself, for example when using Docker: facebook/create-react-app#483.

What is the new behavior?

Use the common cache directory, ./node_modules/.cache/babel-loader, as the default cache directory (when the cacheDirectory setting is enabled).

Does this PR introduce a breaking change?

  • No

Set the default cache directory as the
[common cache directory](https://github.com/avajs/find-cache-dir),
`./node_modules/.cache/babel-loader`.

Previously, when `cacheDirectory` was set to `true`, babel-loader
tried to use the operating system's temporary directory as a cache
directory.

However, when running npm scripts as the root user,
[npm overrides the TMPDIR environment variable](npm/npm#4531).
This caused the cache files to be created in the project folder itself,
for example when using Docker: facebook/create-react-app#483.
@codecov-io
Copy link

codecov-io commented Sep 18, 2016

Current coverage is 94.66% (diff: 100%)

Merging #301 into master will increase coverage by 0.03%

@@             master       #301   diff @@
==========================================
  Files             5          5          
  Lines           149        150     +1   
  Methods          21         21          
  Messages          0          0          
  Branches         30         30          
==========================================
+ Hits            141        142     +1   
  Misses            8          8          
  Partials          0          0          

Powered by Codecov. Last update 32de4b9...0145474

@danez
Copy link
Member

danez commented Sep 22, 2016

Thanks that looks perfect.

I will have a look at babel-register too, as it uses a similar cache strategy and maybe we can put that into a babel-cache package.

@MoOx
Copy link
Contributor

MoOx commented Sep 30, 2016

Any ETA for this? :)

@MoOx
Copy link
Contributor

MoOx commented Oct 25, 2016

This PR looks great. Anything missing to get this merged?

@hzoo
Copy link
Member

hzoo commented Oct 26, 2016

Thanks for the help, sorry for the long wait

@hzoo hzoo merged commit 92fe52e into babel:master Oct 26, 2016
Ognian pushed a commit to Ognian/babel-loader that referenced this pull request Feb 27, 2017
Set the default cache directory as the
[common cache directory](https://github.com/avajs/find-cache-dir),
`./node_modules/.cache/babel-loader`.

Previously, when `cacheDirectory` was set to `true`, babel-loader
tried to use the operating system's temporary directory as a cache
directory.

However, when running npm scripts as the root user,
[npm overrides the TMPDIR environment variable](npm/npm#4531).
This caused the cache files to be created in the project folder itself,
for example when using Docker: facebook/create-react-app#483.
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.

5 participants