justmoon / node-extend

Simple function to extend objects
MIT License
341 stars 68 forks source link

use `isarray` for old browsers #27

Closed vvo closed 9 years ago

vvo commented 9 years ago

IE<=9 has no Array.isArray.

It's the only change needed to make this module work in IE8.

vvo commented 9 years ago

ping @ljharb

ljharb commented 9 years ago

Alternatively you could use the es5-shim, which would provide it. I can't imagine working in IE 8 without that :-)

ljharb commented 9 years ago

While I think this is great, I think that a dependency would break component usage. Can you make an internal isArray function instead?

vvo commented 9 years ago

@ljharb Yeah I know es5-shim is a great solution! It's just that when you find yourself authoring modules that need to be compatible with old browsers, you cannot just include es5-shim in your module.

Do you think I can add the dep to component.json also?

Otherwise Ill just make it as an internal func. Let me know.

ljharb commented 9 years ago

Unfortunately I don't use component - only npm and browserify - so I'm not sure what will break it, but the decision to support it was made before my arrival.

If you can verify that adding it as a component dep works, then yay! Otherwise an internal function would be preferred.

Thanks!

vvo commented 9 years ago

ok @ljharb I inlined isarray, could not make component install work.

ljharb commented 9 years ago

One problem is that this will break code coverage - you'd need to have a test that deletes Array.isArray, makes sure it extend still works, and then restores it.

vvo commented 9 years ago

One problem is that this will break code coverage - you'd need to have a test that deletes Array.isArray, makes sure it extend still works, and then restores it.

will do tonight

vvo commented 9 years ago

@ljharb fixed

ljharb commented 9 years ago

Thanks, this looks great!