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

[Webpack 5] Adding new enableBuildCache() method for Webpack 5 persistent caching #884

Merged
merged 3 commits into from
Jan 23, 2021

Conversation

weaverryan
Copy link
Member

@weaverryan weaverryan commented Jan 11, 2021

Hi!

This is a very simple feature, but it can also easily be improperly used. It is part of #880. See some comments about it from @Lyrkan from #645:

Things can easily go wrong with persistent caching... even only enabling it for the config file could lead to some issues if not done properly (for instance if the config use values coming from environment variables, which is far from an edge case imo: https://github.com/webpack/changelog-v5/blob/master/guides/persistent-caching.md#version).

The trick, for us, is to:

A) Make sure we are doing everything as correctly as we can. For example, I assume that WE don't need to include .babelrc or postcss.config.js, but I actually don't know that for sure. Should we also potentially be adding Encore itself to the build dependencies?

B) Good communication in the documentation above the method and in the (eventual) recipe where we include this in the user's webpack.config.js file

TODO

  • 1) Test in a real project to see if we're missing anything (also play with what the valid keys are under buildDependencies - other than config, this is not documented anywhere).
  • 2) Check into Stimulus: I'm curious if controllers.json will need to be added to the build dependencies. I'm also curious if the UX vendor libraries will need to be in the build dependencies, as these do not follow the normal versioning (i.e. their directories in node_modules/ can change without a change to yarn.lock or package.json) should be solved by linked changes in updating stimulus-bridge plugin to work with proposed new loader #888

Cheers!

@weaverryan weaverryan mentioned this pull request Jan 11, 2021
10 tasks
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.

1 participant