fix(index): add this.rootContext
#234
Conversation
joshwiens
changed the title
michael-ciniawsky
requested review from
alexander-akait,
michael-ciniawsky and
joshwiens
michael-ciniawsky
changed the title
this.context to this.rootContext
src/index.js
Outdated
| @@ -10,7 +10,7 @@ export default function loader(content) { | |||
|
|
|||
| validateOptions(schema, options, 'File Loader'); | |||
|
|
|||
| const context = options.context || this.options.context; | |||
| const context = this.rootContext; | |||
There was a problem hiding this comment.
Was the option for a setting a custom context be removed on purpose?
Doesn't this work anymore in general now ?
There was a problem hiding this comment.
Yes, this is breaking for anything that needs is much like we had to do for etwp 3.1
For any loaders & plugins that require this change, make sure the next branch has the peerDependency set to 4.x only
There was a problem hiding this comment.
So a options.context can't technically exists anymore with the changes made to the underlying logic in webpack (rootContext) ?
There was a problem hiding this comment.
const context = options.context || this.rootContext || this.options && this.options.context
this.options doesn't exist anymore. It was already deprecated in webpack 3. webpack 4 adds this.rootContext as replacement for the needed context information. We could backport this if it helps.
There was a problem hiding this comment.
Is there particular reason to name it rootContext instead of context ? :) I assumed the meaning/logic also changed somehow due to the rename and the correlation between config.context => this.context is cleaner. There is also no e.g childContext etc, or is there anything new ?
👍 for a backport of rootContext to webpack v3.0.0 so loaders/plugins relying on it don't need to add legacy code checks
@TheLarkInn Could you update the PR with @sokra suggestions so we can release a patch instead of a major for this/for now, so folks can continue trying out more advanced stuff (changes) in the meantime ? 😛 (Fairly depressing to fail at the file-loader 🙃...)
|
@TheLarkInn Can I take this over or could you apply these changes - const context = this.rootContext
+ const context = options.context || this.rootContext || this.options && this.options.contextand rebase please ? |
michael-ciniawsky
added
pr_action: Rebase
semver: Patch
and removed
pr_action: Review
semver: Major
labels
michael-ciniawsky
changed the title
this.context to this.rootContextthis.rootContext
michael-ciniawsky
added
priority: 1 (critical)
type: Bug
and removed
priority: 3 (required)
labels
|
Closing this because someone needs to take a git class ..... |
|
The fix should go into master please, so webpack v3.0.0 && 4.0.0 is supported by the |
michael-ciniawsky
added
status: Declined
and removed
priority: 1 (critical)
pr_action: Rebase
severity: 2 (regression)
status: Pending
labels
Notable Changesthis.rootContext(webpack >= v4.0.0) for compatibility withwebpackv4.0.0Issuesthis.context=>this.rootContext(Breaking Change) #233