IE10 insertBefore fix

[codecov-io]

Codecov Report

Merging #140 into master will not change coverage. The diff coverage is n/a.

@@          Coverage Diff          @@
##           master   #140   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           1      1           
  Lines         183    183           
  Branches       52     52           
=====================================
  Hits          183    183

Impacted Files	Coverage Δ
src/index.js	100% <ø> (ø)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 680ab57...b53c594. Read the comment docs.

[shirotech]

@jorgebucaran can this be merged?

[jorgebucaran]

@van-nguyen I don't think this is going to affect performance too much, but I'd like to run it through js-framework-benchmarks again to see if it makes any difference. If it doesn't we can merge it, otherwise, stop supporting IE10.

[cjke]

Thanks @van-nguyen for raising this PR - we too need to support IE10. We're temporarily using your branch.

@jorgebucaran Does it look like this will end up getting merged?

[frenzzy]

Have you considered to import a polyfill before any other code?

// insertBeforePolyfill.js
const insertBefore = HTMLElement.prototype.insertBefore
HTMLElement.prototype.insertBefore = function (newElement, referenceElement) {
  return insertBefore.call(this, newElement, referenceElement || null)
}

// yourEntryPoint.js
import './insertBeforePolyfill'
import { app } from 'hyperapp'

// other code here

[jorgebucaran]

@cjke @van-nguyen I admit I didn't think of @frenzzy's solution, but I think it's an acceptable workaround for now! 🎉

[cjke]

Agreed @jorgebucaran . Nice thinking @frenzzy

[shirotech]

nice one @frenzzy, very clever 👍

[shirotech]

An update for anyone else interested, an improved polyfill version I did was wrap it in a try catch block, the purpose was to not compromise any performance when not needed.

try {
  const div = document.createElement('div');
  const parent = document.body;
  parent.insertBefore(div, undefined);
  parent.removeChild(div);
} catch (e) {
  const insertBefore = HTMLElement.prototype.insertBefore;
  HTMLElement.prototype.insertBefore = function(newElement, referenceElement) {
    return insertBefore.call(this, newElement, referenceElement || null);
  };
}

[jorgebucaran]

Thanks for sharing @van-nguyen! 👍

[frenzzy]

Anyway merging this PR seems much more simpler solution to me, do not see anything bad with it.

[jorgebucaran]

But why are you still dealing with IE10?

[cjke]

Unfortunately that is the reality of business and client engagement.

I'd love to personally say I'm not going to support IE at all, but if a client has a particular demographic that does indeed use IE10, then well, we need to support that (which happens to be the case where I am)

Sure, if you are doing a personal project, or a project where it is your call, drop it - but again, that simply isn't reality for a lot of devs out there.

[jorgebucaran]

@cjke Yeah, I've heard that like twice before. 😄