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
[Parser] Support rename this's property for IIFE #5076
[Parser] Support rename this's property for IIFE #5076
Conversation
284e98d
to
399471c
Compare
Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon. |
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 add a config test case testing the Provide plugin with IIFE like in the issue.
lib/Parser.js
Outdated
} | ||
const params = functionExpression.params; | ||
const args = options.map(renameArgOrThis, this); | ||
const renameThis = currentThis ? renameArgOrThis.call(this, currentThis) : null; |
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.
Could you switch this line with the previous line? So that arguments are evaluated in the correct order.
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.
sure
Hi, @sokra , ci/circleci failed because some unrelated tests timeout. How could I force reevaluate the tests? thanks. |
4a407a5
to
cc2df2f
Compare
Hi, @sokra , the comments are resolved, ci checks are passed too. Could you help continue the review? Thanks. |
Thanks |
What kind of change does this PR introduce?
bugfix
Did you add tests for your changes?
Yes
If relevant, link to documentation update:
N/A
Summary
This is for fixing #5067.
ProvidePlugin supports replacing IIFE argument's property with resolved modules, but not
this
's. This PR makes it available.For example, for code
config
will replace the
jQuery
with resolved one.Does this PR introduce a breaking change?
No
Other information
This PR only handles
iife.call()
oriife.bind()
style call of IIFE, notiife()
style as it has more complicatedthis
resolving rules (environment, strict mode or not)..apply