fix: ReferenceError: onScriptComplete is not defined when using HMR o… #7210
Conversation
|
|
|
Met the same problem when using Firefox 40, but I have tested that it did not seem the problem of hoisted, as the following snippet has worked under Firefox 40: (function () {
console.log(test);
function test() {}
})();The real reason is that function if (true) {
/** throw an error: func is not defined */
console.log(func);
function func () {}
} |
|
@chris-czopp Please accept CLA |
|
@aleen42 I rushed too quickly with the conclusions, good to know the root cause of the issue :) |
|
@chris-czopp your should rewrite your email in commints |
|
Hi @chris-czopp. Just a little hint from a friendly bot about the best practice when submitting pull requests:
You don't have to change it for this PR, just make sure to follow this hint the next time you submit a PR. |
|
@sokra the method in the timeout handler function before has the same problem. |
|
For maintainers only:
|
|
Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon. |
|
And any milestones here? |
| @@ -173,7 +173,6 @@ class JsonpMainTemplatePlugin { | |||
| "onScriptComplete({ type: 'timeout', target: script });" | |||
There was a problem hiding this comment.
Here has the same question inside the setTimeout handler function, but not fix at all
There was a problem hiding this comment.
yes, however the timeout var is used inside onScriptComplete. If we moved it below we'd have timeout var hoisted... I think it needs to be further refactored.
|
/cc @chris-czopp friendly ping |
|
Thanks, more changes can be done in a separate PR |
revert changes of webpack#7210, and use a varaible declaration outside a condition block instead, according to https://eslint.org/docs/rules/no-inner-declarations
revert changes of webpack#7210, and use a varaible declaration outside a condition block instead, according to https://eslint.org/docs/rules/no-inner-declarations
…n Firefox 45
What kind of change does this PR introduce?
bugfix for Firefox 45 and code improvement
Did you add tests for your changes?
no, it's an enhancement covered by existing tests
If relevant, link to documentation update:
N/A
Summary
I've been testing my app on out-dated Firefox (45) and realised the template relies on hoisting (
onScriptCompleteassigned before its declaration) which is not only a bad practice but it was crashing the bundling.Does this PR introduce a breaking change?
no, this is just enhancement