paldepind / union-type

A small JavaScript library for defining and using union types.
MIT License
477 stars 28 forks source link

getting rid of Ramda dependency #2

Closed yelouafi closed 9 years ago

yelouafi commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

@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 https://github.com/ramda/ramda/pull/1156. @jdalton was initially interested in supporting the specification but then rejected it.

jdalton commented 9 years ago

@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 commented 9 years ago

@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 commented 9 years ago

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

paldepind commented 9 years ago

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

raine commented 9 years ago

Should a new version be published?

paldepind commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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?