Skip to content
This repository has been archived by the owner on Apr 16, 2019. It is now read-only.

memory leak fix, only reload css files #22

Merged
merged 2 commits into from
Oct 7, 2017
Merged

memory leak fix, only reload css files #22

merged 2 commits into from
Oct 7, 2017

Conversation

nickav
Copy link
Contributor

@nickav nickav commented Oct 6, 2017

There are two issues I noticed while using this loader:

  1. The reference to el is not bound correctly when calling updateCss

For example I think it suffers from this kind of issue:

function something(i) { console.log(i); }
for (var i = 0; i < 10; i++) {
  setTimeout(function() { something(i); }, 0)
}

Will print 10 10 times. If you replace var with let this works the way you would expect. That means that this method will leak orphaned link elements on the page and not remove them correctly.

  1. Sometimes el will not receive the load event, again orphaning link elements.

I added an additional listener for error and made sure that the element is actually a css link. For example, this element would be duplicated on the page:

<link rel=icon type=image/png href=/img/favicon/favicon_32.png sizes=32x32>

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.06%) to 72.093% when pulling a0cef3f on hd-dg:master into 63eb466 on shepherdwind:master.

@shepherdwind shepherdwind merged commit c5d98c7 into shepherdwind:master Oct 7, 2017
shepherdwind added a commit that referenced this pull request Oct 7, 2017
@shepherdwind
Copy link
Owner

1.3.2 published

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants