-
-
Notifications
You must be signed in to change notification settings - Fork 384
fix: onerror mem leak #640
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #640 +/- ##
=======================================
Coverage 89.55% 89.55%
=======================================
Files 11 11
Lines 651 651
Branches 181 181
=======================================
Hits 583 583
Misses 65 65
Partials 3 3
Continue to review full report at Codecov.
|
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.
@evilebottnawi Do you mean I should add this line "prev(event)" ? |
We should move onerror handler to separate function and handle this (look at link), we should generate runtime as little as possible |
Is the generated code ok like this? var onStyleComplete = function (event) {
// avoid mem leaks
linkTag.onerror = linkTag.onload = null;
if (event.type === 'load') {
resolve();
} else {
var request = (event && event.target && event.target.href) || fullhref;
var err = new Error(
'Loading CSS chunk ' + chunkId + ' failed.\n(' + request + ')'
);
err.code = 'CSS_CHUNK_LOAD_FAILED';
err.request = request;
linkTag.parentNode.removeChild(linkTag);
reject(err);
}
};
linkTag.onerror = linkTag.onload = onStyleComplete; |
Yes 👍 |
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.
Good job, thanks
This PR contains a:
Motivation / Use-Case
Fixes #639
Breaking Changes
No
Additional Info
No