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

getting rid of Ramda dependency #2

Closed
yelouafi opened this issue May 28, 2015 · 12 comments
Closed

getting rid of Ramda dependency #2

yelouafi opened this issue May 28, 2015 · 12 comments

Comments

@yelouafi
Copy link

Great library!. i've noticed that the lib imports the whole ramda lib but actually just uses the curryN function. Maybe a good idea is to self implement the function and get rid of the ramda deps, this will keep the lib small and minimal (which is a good fit for browser uses) while not locking users into a specific lib.

a quick implementation

function curryN(fn, len) {
    len = len !== undefined ? len : fn.length;
    return lambda([]);

    function lambda(cargs) {
        return function() {
            var aargs = cargs.concat( Array.prototype.slice.call(arguments) );
            return aargs.length < len ? lambda(aargs) : fn.apply(this, aargs);
        }
    }
}

var sum = curryN(function(a,b,c) {
    return a+b+c;
})


console.log(sum(1,2,3))
console.log(sum(1)(2,3))
console.log(sum(1)(2)(3))
@paldepind
Copy link
Owner

Thank you! Yes, it uses just curryN from Ramda.

I actually did what you suggest in another library. See here. As you can see a curry with the same power as Ramdas is a bit longer than what you posted.

Given that only curryN is actually pulled in from Ramda what disadvantages do you see to the current approach?

@yelouafi
Copy link
Author

Sorry i didn't noticed that only the curryN is pulled from the sources. I was just agreeably surprised that the whole source is less than 60 sloc. But i think it's also good, for such a small lib, to provide a standalone bundle to simplify deployment in browser apps, i mean no build step, just use the js file.

@megawac
Copy link

megawac commented Jun 21, 2015

Another option is to just use lodash npm modules, glancing through the code I would highly recommend using the following lodash modules:
lodash.curry, lodash.isArray, lodash.isObject, lodash.isString, lodash.isFunction, etc.

@paldepind
Copy link
Owner

@megawac I might have been interested in doing that if lodash.curry had supported the placeholder specification as discussed here and implemented by Ramda in ramda/ramda#1156. @jdalton was initially interested in supporting the specification but then rejected it.

@jdalton
Copy link

jdalton commented Jun 21, 2015

@paldepind

jdalton was initially interested in supporting the specification but then rejected it.

Ah, at the time I thought you all were talking about an ES spec proposal. I was "heck ya"ing that.

Are you using placeholders in this package?

@paldepind
Copy link
Owner

@jdalton

Are you using placeholders in this package?

Not internally. But I'm exposing curried functions and a way to create curried functions. Both of those should support placeholders but users should not care what curry function I'm using internally. That is the reason why I came up with the specification idea in the first place.

@jdalton
Copy link

jdalton commented Jun 21, 2015

Ah, sorry bout that, but I'm cool not supporting it for now.

paldepind added a commit that referenced this issue Jun 22, 2015
@paldepind
Copy link
Owner

There is now a dependency free build in the /dist folder.

@raine
Copy link
Contributor

raine commented Jun 23, 2015

Should a new version be published?

@paldepind
Copy link
Owner

I'm not sure. There is no changes made to the version in npm. It is just the addition of a browser friendly build.

@raine
Copy link
Contributor

raine commented Jun 23, 2015

You did update ramda though, I'm using 0.15 in my app so I think browserify would then be capable of flattening the dependency tree into a single instance of Ramda.

I also tried require('union-type/dist/union-type') which gave me

[13:26:47] Browserify Error: Cannot find module './internal/_curry2' from '/node_modules/union-type/dist'
[13:26:47] Browserify Error: Cannot find module './internal/_curryN' from '/node_modules/union-type/dist'
[13:26:47] Browserify Error: Cannot find module './arity' from '/node_modules/union-type/dist'

Should that happen?

@paldepind
Copy link
Owner

You did update ramda though, I'm using 0.15 in my app so I think browserify would then be capable of flattening the dependency tree into a single instance of Ramda.

Good point. I didn't think of that.

Should that happen?

I've just published a new version to npm. How is that working for you?

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

No branches or pull requests

5 participants