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

bind factory: create partial function applications with injected arguments #167

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

Flygsand
Copy link

@Flygsand Flygsand commented Dec 6, 2014

This PR implements the bind factory, which takes a module that is a function/constructor and a list of arguments, and creates a component that is a partial application of that function on those arguments.

Use case
I'm using a third-party data format parser (MapParser) that supports binding constructors to annotated entities in the data. Whenever an bound entity is encountered, the parser instantiates an object by passing entity data to the corresponding constructor.

function MapScreen(entityTypes) {
  this.parser = new MapParser();
  this.parser.bindTypes(entityTypes);
}

MapScreen.prototype.preload = function() {
  this.map = this.parser.parse(this.assets.mapFile);
};

Here, entityTypes refers to an object injected by wire:

mapScreen: {
  create: {
    module: 'screens/map_screen',
    args: { $ref: 'entityTypes' }
  }
},

entityTypes: {
  opponent: {
    bind: {
      module: 'entities/opponent',
      args: { $ref: 'opponentFactory' }
    }
  }
}

MapParser expects a unary constructor function(entity), but the constructor requires additional components that must be wired in. Continuing the example above, entities/opponent.js:

function Opponent(opponentFactory, entity) {
  this.model = opponentFactory.create(entity.name);
}

In such scenarios, the bind factory can be useful.

@briancavalier
Copy link
Member

Thanks @protomouse! This looks pretty good overall. I'll take a closer look at the code shorty.

@Flygsand
Copy link
Author

Flygsand commented Dec 8, 2014

Updated PR after feedback from @briancavalier.

@briancavalier
Copy link
Member

lgtm. Any thoughts, @unscriptable?

@unscriptable
Copy link
Member

I think this a really great feature. Just a couple of notes:

  • I prefer the word "partial" over "bind" because it's a more general term and the feature isn't binding a context (this). It's only capturing arguments.
  • fn.bind.apply, iirc, breaks on a few versions of Safari and Safari mobile.
  • fn.bind.apply, as used here, works fine for constructors, but not plain functions since it loses the this.
  • We have a partial function in wire/lib/functional already. It would be nice to share the code.

@unscriptable
Copy link
Member

The nicest feature of fn.bind.apply is that it preserves the correct function arity. It'd be nice to keep that somehow.

@briancavalier
Copy link
Member

I prefer the word "partial" over "bind" because it's a more general term and the feature isn't binding a context (this). It's only capturing arguments

Good point. Should this new factory also provide the ability to bind context? I always feel like these two actions, binding context and partial application, are actually two separate things, but many JS devs will tend to think of them as belonging together since that's how Function.prototype.bind works.

fn.bind.apply, iirc, breaks on a few versions of Safari and Safari mobile

Ah yes, Function.prototype.bind is entirely missing on older versions of iOS, although I don't remember exactly which versions, probably <= iOS 6.

Another concern is that Function.prototype.bind is a performance disaster. It may not be a big deal in application-level code, but wire/functional's partial() will be faster on 2 axes: 1) partialing itself will be faster, and 2) the resulting partially applied function will also be faster.

fn.bind.apply, as used here, works fine for constructors, but not plain functions since it loses the this.

Hmmm, I thought binding undefined meant "don't affect this" in ES6. But maybe ES5 forces undefined in that case? If it does, then that's another reason to use wire/functional's partial, which will preserve this.

The nicest feature of fn.bind.apply is that it preserves the correct function arity. It'd be nice to keep that somehow.

The only 2 ways I know to do this are: 1) use Function.prototype.bind, or 2) Generate new functions from source using new Function or eval. I'd be ok with losing the arity in favor of using wire/functional.partial, at least initially.

@unscriptable
Copy link
Member

Hmmm, I thought binding undefined meant "don't affect this" in ES6. But maybe ES5 forces undefined in that case?

I just ran a very simple test in Chrome to make sure. In my test, fn.bind(undefined) explicitly binds the function to the global scope, except in strict mode wherein it binds the function to undefined.

@briancavalier
Copy link
Member

I tried the same in Node 0.11.14 and got the same behaviors as you did for Chrome in both strict and non-strict.

@Flygsand
Copy link
Author

Good point. Should this new factory also provide the ability to bind context?

I thought about this myself, but couldn't come up with concrete uses for this functionality. The factory consumes "free" functions, not object/prototype members. To my knowledge, there isn't (yet) a way to reference the latter in a wire spec, i.e.

myObj: {
  create: 'myModule'
},

myOtherObj: {
  create: 'myOtherModule'
},

myFn: {
  bind: {
    fn: { $ref: 'myObj.myMember' },
    thisArg: { $ref: 'myOtherObj' }
  }
}

In such a scenario, having control over thisArg would be useful. Otherwise, I'd just wire everything in as parameters.

The only 2 ways I know to do this are: 1) use Function.prototype.bind, or 2) Generate new functions from source using new Function or eval. I'd be ok with losing the arity in favor of using wire/functional.partial, at least initially.

new Function() has the unwanted effect of losing context - you'd have to bind(). I decided to throw together a benchmark of:

  • wire/functional.partial()
  • wire/functional.partial() rewritten using eval() for arity preservation
  • bind()

http://jsperf.com/partial-vs-partial-w-arity-vs-bind-construct (creating the functions)
http://jsperf.com/partial-vs-partial-w-arity-vs-bind-call (calling the functions)

The eval() solution is slower than bind() in Chrome, and uglier to boot. I can't say I've ever written (or, for that matter, seen) a line of JS implementation that performs introspection on function arity.

@unscriptable
Copy link
Member

I can't say I've ever written (or, for that matter, seen) a line of JS implementation that performs introspection on function arity.

Heh. Here's an awesome one:
https://github.com/cujojs/most/blob/606712be5768978d96f3f6e5cc568f354d1fb5b6/lib/dispatch.js

It's ugly, but helps performance a ton in critical parts of the code. :)

@briancavalier
Copy link
Member

That one is just checking args.length. I agree it's pretty rare to write code that relies on function.length. The only time I can remember doing it is when implementing currying, like the various implementations here, here, and here

@briancavalier
Copy link
Member

@protomouse Thanks for doing some research.

Perhaps the answer here is just to name this factory partial, like @unscriptable suggested, and go with it?

(Just a note: I'm not sure your "call" jsperf is valid since the result of the calling the functions is never used. In those cases, engines may actually optimize the function call out of existence, esp. in this case since the fn created in the test is a noop function that uses none of its parameters, causes no side effects, and doesn't return anything.)

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 this pull request may close these issues.

3 participants