azproduction / autopolyfiller

Autopolyfiller — Precise polyfills. This is like Autoprefixer, but for JavaScript polyfills.
http://azproduction.github.io/autopolyfiller
MIT License
550 stars 15 forks source link

Does not polyfill certain DOM functions #11

Open baileyparker opened 10 years ago

baileyparker commented 10 years ago

jonathantneal/polyfill has polyfills for several DOM methods that autopolyfiller doest not recognize. When running the following:

autopolyfiller('IE 6').add(
    "var abc = document.getElementsByClassName('abc');" +
    "abc.classList.add('abc');" +
    "abc.querySelector('span.def');" +
    "document.querySelectorAll('span.def');"
).polyfills

I expect the output to be:

[
    'Element.prototype.getElementsByClassName',
    'Element.prototype.classList',
    'Element.prototype.querySelector',
    'Element.prototype.querySelectorAll'
]

But instead it is:

[]

I can submit a PR to fix getElementsByClassName and querySelector/querySelectorAll, but there doesn't seem to be an expression matcher for properties (and I don't understand the API enough to write one for properties like classList).

azproduction commented 10 years ago

I know, but I am trying to play safe. Also these polyfills for IE6 are a bit outdated. And many modules and libraries including code of real application are not expecting these functions in IE6.

Pros:

Cons:

Actually it easy to forge a "plugin" for Autopolyfiller. What do you think?

require('autopolyfiller')
.use(require('autopolyfiller-ie6')) // <<<
('IE 6')
.add(
    "var abc = document.getElementsByClassName('abc');" +
    "abc.classList.add('abc');" +
    "abc.querySelector('span.def');" +
    "document.querySelectorAll('span.def');"
)
.toString();

don't understand the API enough to write one for properties like classList

Are you speaking about autopolyfiller.use() or?

baileyparker commented 10 years ago

Shouldn't the modules and libraries who don't expect these features add exclude: 'Element.prototype.addEventListener' to the options passed to autopolyfiller (assuming this was implemented)? I do agree that IE6 polyfills are outdated and for most devs, it (& IE 7) have become browsers which don't need to be targeted. However, unfortunately many still need support for IE 8 (which doesn't support addEventListener & classList) and IE 9 (which does not support classList).

I think a "plugin" would be a nice solution for those that don't need IE 6, but for features not supported by IE 8 & 9, I think this is something that should be included in vanilla autopolyfiller.

Are you speaking about autopolyfiller.use() or?

I am referring to the code that autopolyfiller uses to determine whether a polyfill is needed. I was saying that I could add addEventListener/removeEventListener to lib/polyfill-scan/matchers/method.js, but classList is a property and there is no expression matcher in lib/polyfill-expression-matcher/index.js for matching properties. However, since classList is typically used with a method like ele.classList.add('foo') or ele.classList.remove('bar'), perhaps a the methods could be added to the methods object in lib/polyfill-scan/matchers/method.js like so:

'classList.add': 'Element.prototype.classList',
'classList.remove': 'Element.prototype.classList',
'classList.contains': 'Element.prototype.classList',
'classList.toggle': 'Element.prototype.classList',
'classList.item': 'Element.prototype.classList',

But I'm unsure whether this would work because the . in the keys may not be escaped for the regex.

azproduction commented 10 years ago

Sorry for the late response.

no expression matcher

Well, you can simply looking at smth.classList say that it is using classList. Just add it to https://github.com/azproduction/autopolyfiller/blob/master/lib/polyfill-scan/matchers/method.js it should work.

baileyparker commented 10 years ago

No worries. I'm working on a PR to add support for this, the DOM functions I mentioned above, and a few other DOM functions.

azproduction commented 10 years ago

Thanks, Bailey.

baileyparker commented 10 years ago

It turns out that Element.prototype.getElementsByClassName is a lost cause since IE <= 8 will not allow Element.prototype to be modified. However, I've run into another matcher issue. I was looking into adding the polyfills for the DOMContentLoaded and hashchange events. Which matcher should I use to match them (since they are contained within a string)? I tried putting them in global.js, method.js, but neither worked:

'DOMContentLoaded': 'Window.prototype.Event.ie8.DOMContentLoaded',
'hashchange': 'hashchange'
anselmbradford commented 9 years ago

What's the status of this? addEventListener and classList are both in polyfill-service but don't seem to be available via autopolyfiller-loader?

anselmbradford commented 8 years ago

Ping

anselmbradford commented 8 years ago

@baileyparker what did you end up doing about this?