-
-
Notifications
You must be signed in to change notification settings - Fork 199
Add Encore.addCacheGroup() method and depreciate Encore.createSharedEntry() #680
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor stuff. GREAT idea - it's time for the shared entry to go away :).
options['node_modules'] | ||
.map(regexpEscaper) | ||
.join('|') | ||
})[\\\\/]`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow :)
58dd191
to
269198c
Compare
lib/config/validator.js
Outdated
|
||
_validateCacheGroupNames() { | ||
for (const groupName of Object.keys(this.webpackConfig.cacheGroups)) { | ||
if (['defaultVendors', 'default'].includes(groupName)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we also include vendors
, which is the name in webpack 4 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we also warn if you pass this.webpackConfig.sharedCommonsEntryName
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we also include vendors, which is the name in webpack 4 ?
Oh, so that's why we previously used vendors
in _validateSharedEntryName()
!
I couldn't find a reference to it in the doc anymore since the following page already got updated with defaultVendors
: https://webpack.js.org/plugins/split-chunks-plugin/#optimizationsplitchunks
I think I'll add it and we'll remove it when we upgrade to Webpack 5.
should we also warn if you pass
this.webpackConfig.sharedCommonsEntryName
?
We could but I'm not sure it'll be really useful... people will probably either:
- start a new project and directly use
addCacheGroup()
sincecreateSharedEntry()
throws a deprecation warning - see the deprecation warning and try to replace their
createSharedEntry()
calls byaddCacheGroup()
Doesn't cost much to add a warning though.
… than in createSharedEntry()
Thank you @Lyrkan! |
@Lyrkan @weaverryan Can you guys give an example how I should migrate this:
To The documentation still refers to |
@XWB If you can, use This requires reading the generated
If you can't use
Note that |
Thank you @Lyrkan :) |
As discussed in #645 (comment) our
Encore.createSharedEntry()
hack won't work anymore with Webpack 5.Since it was mostly something that was added to make the transition from Webpack 3 to Webpack 4 less painful it's probably time to depreciate it and encourage people to use cache groups properly instead.
This PR adds a new
Encore.addCacheGroup()
method that, as its name implies, simply adds a new cache group to the config:To make it a bit easier in case people want to directly reference things from the
node_modules
directory I also added anode_modules
option which is basically a shorthand that sets thetest
option: