-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Method prefix #5
Comments
It may be nice to make the prefix customizable. Not sure where that would happen though. |
It could be done like this:
But of course that would mean that it could no longer happen automatically, i.e. even if you don't want to customize the prefix you'd have to do this in order to initialize it:
An alternative could be:
...but I don't like that as much. I'm not sure that a customizable prefix is actually needed though; the module currently only supports lodash and underscore. Were you planning to add another library to the mix? If so, then you might want to use both lodash/underscore and that other library, in which case you'd probably want the prefix to be '_' for the lodash/underscore methods but something else for the other library. In conclusion, it seems simpler just to have the prefix hard-coded as a variable within the module. |
I have a few ideas, but feel free to submit a pull request for your changes. I'll worry about it later on, if at all. |
I can do that...and I'll simplify it to just add the property in ES3 style; thinking about it more I don't think making it non-enumerable is necessary or even preferable. |
Hi @milesj, this (package) is a great and natural idea. Somehow my feeling that the comments at lodash/lodash#409 are about 50% real concerns and 50% not wanting to loose all those cutle little _ characters in zillions of lines of code. Lodash functions are natural extenstions of native types. Will you update this package? |
I added a method prefix, to make it clearer which methods are non-native and to avoid clashing with any future native methods:
mbrowne@1a8d6bf
If you would be interested in incorporating this change, let me know and I'll update the readme and submit a pull request.
BTW I also made the non-native methods non-enumerable...maybe that's not needed, but I was trying to address the concern mentioned here ("This could cause unexpected issues with code iterating over native prototypes")...of course code that iterates over native prototypes should probably account for the possibility that they were extended anyway, so maybe it would actually be better with them being enumerable. Also I assumed ES5 is available; not sure if you preferred to keep it ES3-compatible.
The text was updated successfully, but these errors were encountered: