galaxy icon indicating copy to clipboard operation
galaxy copied to clipboard

Warn about Array.prototype modification and/or allow opt-out

Open hybrist opened this issue 12 years ago • 2 comments

I understand that it's convenient but I think quite some people would consider it a bad practice to change the prototype of built-in types. It's modifying global state which would prevent me from using galaxy in any library: I wouldn't want to mess with global scope of the app using my library. I think the work-around is require('galaxy/lib/galaxy') but I'm not sure if anything in there relies on the Array prototype being patched. Just having require('galaxy/core') available and documented would be enough.

hybrist avatar Dec 19 '13 17:12 hybrist

+1

jdavisclark avatar Dec 19 '13 17:12 jdavisclark

I had forgotten to watch the repo so I did not see this issue. Sorry for the delay in reponding.

require('galaxy/lib/galaxy') does the trick. The array module won't be loaded.

I think that this issue of modifying Array.prototype is a bit blown out of proportion. It doesn't prevent you from mixing with other libraries at all. The extra array methods are non enumerable so they won't interfere with code that introspects arrays or their prototypes. The only potential conflict that I see is if some other library tries to add functions with the same names. Pretty unlikely.

Adding a galaxy/core that only loads galaxy is not a big deal. But isn't require('galaxy/lib/galaxy') sufficient? I don't want to add bells and whistles unless there is a real issue.

bjouhier avatar Feb 15 '14 14:02 bjouhier