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

executeBound on var arg constructors #2214

Open
megawac opened this issue Jun 12, 2015 · 9 comments
Open

executeBound on var arg constructors #2214

megawac opened this issue Jun 12, 2015 · 9 comments

Comments

@megawac
Copy link
Collaborator

megawac commented Jun 12, 2015

As mentioned earlier (#2199), exectueBound doesn't support all of Function.prototype.bind cases on constructors which take variable arguments. As the main implementation of bind no longer uses the native implementation, it's important we fix it.

Lodash currently supports these cases.

var D = _.bind(Date, null, 2015, 05)
new D(10);
@megawac megawac added the bug label Jun 12, 2015
@megawac megawac mentioned this issue Jun 12, 2015
6 tasks
@jdalton
Copy link
Contributor

jdalton commented Jun 12, 2015

Lodash currently supports these cases.

FWIW lodash didn't support this until 3.9.0 (last month). Not sure if that affects the priority but it's something to consider.

@megawac
Copy link
Collaborator Author

megawac commented Jun 12, 2015

Any hints to get this working jdd?

@jdalton
Copy link
Contributor

jdalton commented Jun 13, 2015

See createCtorWrapper. Support is a side effect of patching it to work with ES6 class constructors.

@jridgewell
Copy link
Collaborator

I have solutions of varying degrees:

  • Revert #2199
    • Slow creation
    • Fast invocation
    • Handles any constructor
  • Switch on Function#bind
    • Slow (but slightly faster) creation
    • Fast invocation
    • Handles any constructor
  • JDD's new
    • Fast creation
    • Fast invocation
    • Handles any native JS constructor
    • Won't handle ES6 class constructors with 7+ parameters

@jdalton
Copy link
Contributor

jdalton commented Jul 2, 2015

I'm not a fan of hybrid (obviously I dig the JDD way :P) but it looks like the Switch on Functin#bind is a preferable hybrid as it forks on definition instead of invocation.

@jashkenas
Copy link
Owner

Switch on bind does look slightly preferable. Creating a new switch/case pyramid of doom would be distasteful, even more so if it puts an upper limit on the number of arguments.

@megawac
Copy link
Collaborator Author

megawac commented Jul 28, 2015

The issue is the fallback for old ie will be inconsistent

@megawac
Copy link
Collaborator Author

megawac commented Jul 28, 2015

The best way to handle this may be to do as jdd decided and drop explicit support for non es5 and defer this stuff to es5-shim

@jdalton
Copy link
Contributor

jdalton commented Jul 28, 2015

@jashkenas

Creating a new switch/case pyramid of doom would be distasteful, even more so if it puts an upper limit on the number of arguments.

I'm OK with the upper limit here as it only affects calls to bound build-ins invoked as a constructor or a bound ES6 class constructors (where supported natively). The upper limit of 7 covers the built-in constructor case and should be reasonable for others. If not it can be revisited/added-to to.

The upside to this approach is consistency across environments and other methods, as I use the bound Ctor behavior for partialed and curried methods as well.

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