Skip to content

babel-loader cache is not invalidated when browserslist's config changes #514

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

Closed
wujekbogdan opened this issue Feb 9, 2019 · 8 comments
Closed
Labels

Comments

@wujekbogdan
Copy link

I found that since webpack 4 upgrade Babel should respect .browserslistrc config and browserslist settings from package.json. Neither of these seem to work.

The output JS file is always the same, no matter what browserslist settings I use.

If I edit the @symfony/webpack-encore/lib/loaders/babel.js file and change the targets entry for @babel/preset-env then the JS output looks as expected.

I know that I could use Encore.configureBabel(), but I would prefer to use more standard configuration techniques.

@Lyrkan
Copy link
Collaborator

Lyrkan commented Feb 10, 2019

Hi @wujekbogdan,

I think that it does work, but you don't see any change because Encore sets the cacheDirectory option of babel-loader to true by default.

Could you try disabling that cache to see if that's the case?

Encore.configureBabel(options => {
  options.cacheDirectory = false;
});

@wujekbogdan
Copy link
Author

wujekbogdan commented Feb 10, 2019

I think that it does work, but you don't see any change because Encore sets the cacheDirectory option of babel-loader to true by default.

Thanks, it works!

But I don't fully understand it - if the problem was caused by caching, then why changing the setting in the babel config directly (@symfony/webpack-encore/lib/loaders/babel.js) has an immediate effect? I guess that the cache is invalidated after the settings are changed. But if it works this way then the cache should be also invalidated in case of editorconfig changes.


UPDATE
One more thing: IMO the cache should be disabled if Encore runs in production mode.

@Lyrkan
Copy link
Collaborator

Lyrkan commented Feb 10, 2019

But I don't fully understand it - if the problem was caused by caching, then why changing the setting in the babel config directly (@symfony/webpack-encore/lib/loaders/babel.js) has an immediate effect? I guess that the cache is invalidated after the settings are changed. But if it works this way then the cache should be also invalidated in case of editorconfig changes.

It probably only checks if the direct settings of the loader have been changed, not the external ones parsed by browserslist.

IMO the cache should be disabled if Encore runs in production mode.

Sounds like a reasonable thing to do :)

@Lyrkan Lyrkan added the good first issue Good for newcomers label Feb 10, 2019
@wujekbogdan
Copy link
Author

It probably only checks if the direct settings of the loader have been changed, not the external ones parsed by browserslist.

So it's an issue to resolve on the Babel side, not on the Encore side. Am I right?

@Lyrkan
Copy link
Collaborator

Lyrkan commented Feb 10, 2019

Actually the way it works is documented in the loader's README:

cacheIdentifier: Default is a string composed by the babel-core's version, the babel-loader's version, the contents of .babelrc file if it exists, and the value of the environment variable BABEL_ENV with a fallback to the NODE_ENV environment variable. This can be set to a custom value to force cache busting if the identifier changes.

It's be a bit outdated since the code that generates this cacheIdentifier can be found here and doesn't seem to exactly match that description, but the idea is kinda the same.

So, the thing is... the loader doesn't use browserslist at all, this is done by @babel/preset-env based on the options passed by the loader (for instance if targets is empty).

This means that from the loader's point of view, Babel's config doesn't change if you modify a .browserslistrc file or your package.json.

There is an issue opened on babel-loader's repository about that: babel/babel-loader#690

In the meantime your suggestion of disabling that cache for production in Encore seems a good trade-off to me.

@Lyrkan Lyrkan changed the title browserslist config does't work babel-loader cache is not invalidated when browserslist's config changes Feb 10, 2019
@Lyrkan Lyrkan added HasPR and removed good first issue Good for newcomers labels Feb 10, 2019
@wujekbogdan
Copy link
Author

@Lyrkan

Thanks for the comprehensive answer. I think we can close this issue then.

weaverryan added a commit that referenced this issue Mar 25, 2019
This PR was merged into the master branch.

Discussion
----------

Don't use babel-loader's cacheDirectory for production

Currently `cacheDirectory` is always set to `true` in `babel-loader`'s options.

This can be an issue because the cache identifier is only based on direct Babel options, but not on external configs such as `.browserslistrc` files or if a `browserslist` key was added to the `package.json` (see babel/babel-loader#690).

Disabling that cache entirely in Encore while waiting for a proper solution in `babel` or `babel-loader` would probably not be a good idea, but we could mitigate the problem by disabling it only for the prod environment.

Closes #514

Commits
-------

ae74298 Don't use babel-loader's cacheDirectory for production
@seyfer
Copy link

seyfer commented Aug 7, 2019

@Lyrkan to do the same as

Encore.configureBabel(options => {
  options.cacheDirectory = false;
});

but with .bablerc
I have found only this way

config.module.rules[0].use[0].options = {
        ...config.module.rules[0].use[0].options,
        cacheDirectory: true,
        cacheCompression: false,
    };

@Lyrkan
Copy link
Collaborator

Lyrkan commented Aug 7, 2019

@seyfer AFAIK you can't, cacheDirectory is a loader-specific option whereas the .babelrc file is intepreted by Babel (which is called by the loader).

We should probably add an option that allows users to say "I know what I'm doing, apply the callback even if there is a .babelrc file"... but in the meantime your workaround is probably the easiest way to do it (maybe you could improve it a bit using Encore.configureLoaderRule(...))

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

No branches or pull requests

3 participants