search

qodesmith / datepicker

Get a date with JavaScript! A datepicker with no dependencies.
344 stars 101 forks source link

refactor: avoid .toString.call() #95

Closed KillerCodeMonkey closed 4 years ago

KillerCodeMonkey commented 4 years ago

It is strange to use ({}).toString.call(thing) instead just use instanceof and for the generic object check, use .toString() in this case

qodesmith commented 4 years ago

I appreciate the effort here, but instanceof won't work in the case of ShadowRoot for older versions of IE since IE doesn't support anything shadow-DOM related.

KillerCodeMonkey commented 4 years ago

maybe just check if it is IE or if there is ShadowRoot?

I just added a special case with toString.

One question: You have var hasShadowDomSupport = 'getRootNode' in window.Node.prototype and additionally in } else if (currentParent.toString() === '[object ShadowRoot]') { a check about hasShadowDomSupport. Isn't it obsolete when checking for '[object ShadowRoot]'. Why should a browser not supporting shadowDom have a dom node of ShadowRoot?

qodesmith commented 4 years ago

@KillerCodeMonkey

Isn't it obsolete when checking for '[object ShadowRoot]'. Why should a browser not supporting shadowDom have a dom node of ShadowRoot?

You're right about this. I'm kinda flying blind here since I don't really know a reliable way to test in IE. I'll have to take a 2nd look at all this, but at least it won't break anything.