Skip to content
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

Open
mbrowne opened this issue Jul 22, 2015 · 5 comments · May be fixed by #6
Open

Method prefix #5

mbrowne opened this issue Jul 22, 2015 · 5 comments · May be fixed by #6

Comments

@mbrowne
Copy link

mbrowne commented Jul 22, 2015

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.

@milesj
Copy link
Member

milesj commented Jul 22, 2015

It may be nice to make the prefix customizable. Not sure where that would happen though.

@mbrowne
Copy link
Author

mbrowne commented Jul 23, 2015

It could be done like this:

require('titon-probe')({prefix: '_'});
//or
require('titon-probe').init({prefix: '_'});

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:

require('titon-probe')();

An alternative could be:

global.titon_probe_options = {prefix: '_'};
var probe = require('titon-probe');

...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.

@milesj
Copy link
Member

milesj commented Jul 23, 2015

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.

@mbrowne
Copy link
Author

mbrowne commented Jul 23, 2015

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.

@andrisi
Copy link

andrisi commented Oct 2, 2020

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?

@mbrowne mbrowne linked a pull request Oct 3, 2020 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants