-
-
Notifications
You must be signed in to change notification settings - Fork 384
feat(index): allow filename
to be a {Function}
(options.filename
)
#225
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
feat(index): allow filename
to be a {Function}
(options.filename
)
#225
Conversation
Store filename in options, this way function is called once
945943f
to
378c955
Compare
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
chunkFilename can't be a function
@@ -170,7 +170,7 @@ class MiniCssExtractPlugin { | |||
renderedModules, | |||
compilation.runtimeTemplate.requestShortener | |||
), | |||
filenameTemplate: this.options.filename, | |||
filenameTemplate: this.getFilename(chunk, this.options.filename), |
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.
I believe you can pass a function to filenameTemplate
too. webpack will call it with an information object.
@@ -194,7 +194,10 @@ class MiniCssExtractPlugin { | |||
renderedModules, | |||
compilation.runtimeTemplate.requestShortener | |||
), | |||
filenameTemplate: this.options.chunkFilename, | |||
filenameTemplate: this.getFilename( |
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.
Same
@@ -366,6 +378,10 @@ class MiniCssExtractPlugin { | |||
}); | |||
} | |||
|
|||
getFilename(chunk, filename) { |
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.
Not needed
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