fix(index): Add check for this.options
#189
Conversation
`this.options` doesn't seem to be defined always. If I don't have the LoaderOptionsPlugin it crashes :/
|
|
src/index.js
Outdated
| @@ -10,7 +10,7 @@ export default function fileLoader(content) { | |||
|
|
|||
| const query = loaderUtils.getOptions(this) || {}; | |||
| const configKey = query.config || 'fileLoader'; | |||
| const options = this.options[configKey] || {}; | |||
| const options = this.options && this.options[configKey] || {}; | |||
There was a problem hiding this comment.
Why? please provide more information, also need tests, thanks!
There was a problem hiding this comment.
I'm not sure what I would write a test for, without setting up and running through webpack. The loader assumes this.options is provided by webpack unconditionally, its not. It only exists in v2+ if the user provides the LoaderOptionsPlugin, a plugin that exists only to help ease migration from v1. This is just simple mistake on the webpack api I believe. Either this or add to the Readme saying the LoaderOptionsPlugin is required
There was a problem hiding this comment.
@jquense i am use file-loader without LoaderOptionsPlugin, can your provide repo where it is not works?
Codecov Report
@@ Coverage Diff @@
## master #189 +/- ##
=========================================
+ Coverage 97.43% 97.5% +0.06%
=========================================
Files 2 2
Lines 39 40 +1
Branches 18 19 +1
=========================================
+ Hits 38 39 +1
Misses 1 1
Continue to review full report at Codecov.
|
|
Sorry about that, the problem is that if you do provide a LoadersOptionsPlugin but leave options undefined. On the face of it it seems like a tiny edge case. but we generally use functions to build our configs so this is actually easy to do e.g. getLoaderOptions = (options) => new webpack.LoaderOptionsPlugins({
minimize: true,
options
})
plugins: [getLoaderOptions()] |
| *.sublime-workspace |
There was a problem hiding this comment.
Please exclude this from commit, we use webpack-defaults for all repos, need do PR there https://github.com/webpack-contrib/webpack-defaults
There was a problem hiding this comment.
Sorry I'm not sure what you want excluded here, the line was already there my editor just inserted a newline character. I'm not even using sublime
|
@jquense In case you are using webpack >= v2.0.0 please setup options in module.rules instead, since |
michael-ciniawsky
added
semver: Patch
severity: 4 (inconvenient)
status: Pending
type: Bug
and removed
flag: Needs more info
labels
michael-ciniawsky
changed the title
this.options
|
@michael-ciniawsky can we close this PR? |
michael-ciniawsky
added
status: Declined
and removed
pr_action: Review
severity: 4 (inconvenient)
status: Pending
labels
this.optionsdoesn't seem to be defined always. If I don't have the LoaderOptionsPlugin it crashes :/