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

ES6 Arrow Functions #264

Closed
koalabz opened this issue Oct 18, 2016 · 12 comments
Closed

ES6 Arrow Functions #264

koalabz opened this issue Oct 18, 2016 · 12 comments

Comments

@koalabz
Copy link

koalabz commented Oct 18, 2016

First of all, thanks for this amazing piece of code.
I'm starting playing with stampit and i'm stuck trying to use arrow functions in methods() function like this :

const stamp = stampit().methods({
  error: console.error,
  incorrect: (value) => {
    this.error(`value ${value} is incorrect`);
  }
});
stamp().error('bad value') // ok 
stamp().incorrect('bad value') // TypeError: Cannot read property 'error' of undefined

I understand that this do not point to the stamp object itself.
Is there anyway to get the same arguments : { instance, stamp, args } like the init() function ?

@koresar
Copy link
Member

koresar commented Oct 18, 2016

No worries :)

This is not a valid code. Here I fixed it:

const stamp = stampit().methods({
  error: console.error,
  incorrect(value) { // < -THAT LINE
    this.error(`value ${value} is incorrect`);
  }
});

Now the this is pointing to the object instance. Basically, that's how JavaScript works.

Cheers!

@koalabz
Copy link
Author

koalabz commented Oct 18, 2016

Thanks @koresar for your quick answer but why not use the same approach than init() for methods() func like this :

const stamp = stampit().methods(({ instance, stamp, args }) => ({
   error: console.error,
   incorrect: (value) => {
      instance.error(`value ${value} is incorrect`);
   }
));

methods() accept also a function with the same init() options object and return methods, but we can use ES6 Arrow Func

@danielkcz
Copy link
Contributor

@koalabz You really like that better than what @koresar showed you? It's terribly verbose :) Personally I don't use fat arrows with stampit at all to avoid confusion and errors.

Either way it's not really possible because in the time when you are calling methods, there is yet no instance to refer to. These methods are attached to descriptor and used for any future instance of that stamp. You could only solve this by using configuration and creating these methods within initializer dynamically, but that's hardly a same thing.

If you are familiar with AST and writing Babel plugins, you can perhaps make the one that can handle this properly

@koalabz
Copy link
Author

koalabz commented Oct 18, 2016

@koalabz You really like that better than what @koresar showed you?

I don't prefer one or the other i just try to understand why we can't use ES6 Arrow in other methods than init function.

Either way it's not really possible because in the time when you are calling methods, there is yet no instance to refer to.

It's the same when you call initmethod right ?
As reference in doc API: create(), object is created when you invoke createor execute your stamp.

Without doubt the arrow function is a great addition. When used correctly it brings simplicity in places where earlier you had to use .bind() or trying to catch the context. It also makes the code lighter.
Advantages in some situations brings disadvantages in others.

:)

@koresar
Copy link
Member

koresar commented Oct 18, 2016

So, yeah. Daniel is correct.
If you want to use stamp and args in the other methods you'd need to attach them to the object instance. And then use them as this.stamp or this.args in your methods.

Also, don't put arrow functions everywhere. ☺️ It's not a silver bullet.

@koresar
Copy link
Member

koresar commented Oct 18, 2016

We can't use arrow functions as methods because arrow functions bind the "this" context to the upper scope. In your case it's the "undefined".
You need to remove that arrow function as I suggested and you'll be get access to the "instance" as the "this" context inside your methods.

@koalabz
Copy link
Author

koalabz commented Oct 18, 2016

Thanks @koresar @FredyC for your answers i will use the old style this as mentioned above 😄
Try to understand deeper stamp-specification when have more time .

@danielkcz
Copy link
Contributor

danielkcz commented Oct 18, 2016

@koalabz Check this out :)

const stamp = stampit().methods({
  oldStyle: function() { ... },
  fatStyle: () => { ... },
  newStyle() { ... },
});

So regarding your quote about arrow function bringing simplicity, I am sure you can see that in this case it only brings chaos :)

Closing as it's not possible to solve this on library level.

@neverfox
Copy link

I just wanted to second @koalabz's suggestion because I would prefer never to use this and I'm willing to pay a price in verbosity to avoid it. I'm curious why, @FredyC, it's not possible on a library level to allow that form. Wouldn't it just be a matter of seeing if the argument to methods is a function and, if it is, use the returned value?

@koresar
Copy link
Member

koresar commented May 18, 2017

@neverfox could you please show us code example of the proposed syntax? Otherwise, I struggle to understand code not seeing code. :)

@ericelliott
Copy link
Contributor

Arrow functions do not bind this -- in fact, they can't bind this because they don't have their own slot for binding this at all. Arrow functions always delegate this to the lexical scope.

In other words, if you want to declare a method that uses the instance, you should chose one of these options:

  1. Define it in an init function (which gets the instance passed in).
  2. Define it with a standard function or method syntax (which can access the instance as this).

If you want to avoid this, you should also completely shun instance state, and instead use closures for state -- which means you should be using initializers to define privileged methods.

@ericelliott
Copy link
Contributor

ericelliott commented May 18, 2017

@neverfox You can already avoid this -- simply define all your methods as privileged methods inside init functions, and you'll get the instance passed in. You can't do that with methods because by definition in JavaScript, methods use dynamic this binding. There's no way the library could assure that the instance is available without forcing some convention, double-delegating all method calls, and injecting an instance parameter.

That would be a breaking change, it would dramatically alter the behavior of all methods, cause a performance hit for every method call, and cause a TON of confusion. In other words, it's not gonna happen.

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