feat(index): allow filename to be a {Function} (options.filename)#225
feat(index): allow filename to be a {Function} (options.filename)#225owlyowl wants to merge 8 commits into
filename to be a {Function} (options.filename)#225Conversation
Store filename in options, this way function is called once
945943f to
378c955
Compare
alexander-akait
left a comment
There was a problem hiding this comment.
Please describe use case, also please improve docs
378c955 to
3149f26
Compare
…, Adding try catch err
3149f26 to
5b45d7e
Compare
|
This PR contains a: Motivation / Use-Case closes #143 and implements improvements as per @shellscape feedback here: https://github.com/webpack-contrib/mini-css-extract-plugin/pull/145 With the deprecation of ExtractTextPlugin for Webpack 4 we need to implement this feature to be at parity with what we had in ExtractTextPlugin with regards to filename options. This is handy for multiple entry point implementation. |
{Function}
|
Any idea when this will be released. Really needing this feature. |
|
Status? |
|
Need rebase |
|
Hi, I've been waiting for this feature . it would be nice if it were released. I am using Extract Text Plugin in beta for webpack 4 and I would be happy to dismiss it as soon as possible. Many many thanks |
| shortChunkHashMap[chunkId] = chunkMaps.hash[ | ||
| let chunkFilename; | ||
| try { | ||
| chunkFilename = JSON.stringify( |
There was a problem hiding this comment.
That's incorrect. chunkFilename can't be a function. The chunkFilename generation logic need to be embedded into the runtime code.
chunk is the runtime chunk here and the referenced chunk is not known.
| plugins: [ | ||
| new Self({ | ||
| filename: (chunk) => chunk.name == 'app' ? 'this.is.app.css' : `[name].css`, | ||
| chunkFilename: (chunk) => '[name].css', |
There was a problem hiding this comment.
chunkFilename can't be a function
| compilation.runtimeTemplate.requestShortener | ||
| ), | ||
| filenameTemplate: this.options.filename, | ||
| filenameTemplate: this.getFilename(chunk, this.options.filename), |
There was a problem hiding this comment.
I believe you can pass a function to filenameTemplate too. webpack will call it with an information object.
| compilation.runtimeTemplate.requestShortener | ||
| ), | ||
| filenameTemplate: this.options.chunkFilename, | ||
| filenameTemplate: this.getFilename( |
| }); | ||
| } | ||
|
|
||
| getFilename(chunk, filename) { |
|
Dear @sokra, |
|
Dear @owlyowl |
my team is also waiting for this release. |
|
Feel free to create new PR looks this abadoned |
|
@evilebottnawi I just saw this PR after opening #321. I'll use the comments here as a guide to improve #321. |
|
Can't wait until this is merged!!! |
|
Need rebase |
|
Is there any update on when this is expected to get merged? Until then, I found a workaround that seemed to work well for me. I originally had a function for the So I jumped into the code to get more context as to why it was failing and found the line that was causing the error was wrapped in a conditional check for the Apparently I do though until this PR gets merged. But for those in the same boat I was in, utilizing the |
|
@jrjacobs24 PR was abandoned, can you send it again? |
|
@evilebottnawi What did you need me to send again? I didn't have anything to do with this PR originally, but I'm happy to help see it through if needed 👍 |
|
👍 Need resend this PR |
|
@evilebottnawi Does #381 do the same thing as what's being requested? |
|
@shanecav84 no |
|
These look to have overlapping functionality:
|
|
@shanecav84 one option for non async chunk other option for async chunk, sometimes developers need this, but i agree we should simplify this, PR welcome |
|
I could help. I'm still not understanding though.
Which option is which? Why not just have this.options = Object.assign(
{
filename: options.filename || DEFAULT_FILENAME,
moduleFilename: () => options.filename,
},
options
)
...
filenameTemplate: typeof this.options.filename === 'function'
? this.options.filename(chunk)
: filename |
|
@shanecav84 filename !== moduleFilename in some cases, but we can use |
|
I'm working on rebasing this branch on master. The conflict is here, where both |
|
Sorry for the delay in sorting this PR out. Thanks @shanecav84 and @evilebottnawi I've tried using the @jrjacobs24 workaround without success as I'm generating some theme based style sheets in production. What it results in is making a bunch of css files with hashes in the names that are webpackbootstrap JS so any subsequent css processing fails as the css files actually contain javascript.. see here: I'd love to know how to fix this it is driving me insane. Having this feature PR'd in would sort it out I suspect but master has moved out from under me sorry. @evilebottnawi any idea how you'd like the merge conflict corrected? |
Codecov Report
@@ Coverage Diff @@
## master #225 +/- ##
======================================
Coverage 0.00% 0.00%
======================================
Files 3 3
Lines 241 246 +5
Branches 49 50 +1
======================================
- Misses 198 202 +4
- Partials 43 44 +1
Continue to review full report at Codecov.
|
|
Supported |
Clean up as per feedback by @shellscape
Issues