You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
The text was updated successfully, but these errors were encountered:
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.
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 havingrequire('galaxy/core')
available and documented would be enough.The text was updated successfully, but these errors were encountered: