-
-
Notifications
You must be signed in to change notification settings - Fork 8.8k
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
refactor(es6): refactor PrefetchPlugin to ES6 class #3665
Conversation
}); | ||
compiler.plugin("make", (compilation, callback) => { | ||
compilation.prefetch(this.context || compiler.context, new PrefetchDependency(this.request), callback); | ||
}.bind(this)); |
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 think bind(this)
is unnecessary.
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.
Thanks. Glad you caught that.
forgot to take out the params at the top of the class
class PrefetchPlugin { | ||
|
||
constructor(context, request) { | ||
if(!this.request) { |
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.
This is always true
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.
@Kovensky What was your procedure to check?
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.
Nothing, really; I just noticed while reading the code.
Looking at the original also shows it was request
instead of this.request
:)
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 made my first commit at 4AM :-) May be why! Thank you
Thanks. Congrats on first PR. :-D 😍 🤗 |
What kind of change does this PR introduce?
Refactor (ES5 => ES6)
Did you add tests for your changes?
No functionality was edited. Tested with webpack@2.2.0-rc.3!
If relevant, link to documentation update:
N/A
Summary
This is my first OSS contribution ever. Saw that this dependency needed some ES6 care... so I gave it some.
Does this PR introduce a breaking change?
Nope.