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

Feature-Request: Support an autobinding-attribute for this in class methods #18526

Closed
ststeiger opened this issue Sep 16, 2017 · 9 comments
Closed
Labels
Declined The issue was declined as something which matches the TypeScript vision Out of Scope This idea sits outside of the TypeScript language design constraints Suggestion An idea for TypeScript

Comments

@ststeiger
Copy link

ststeiger commented Sep 16, 2017

Suppose in a typescript-class you have this method:

public calculateVolume():number {
    return (this.m_Length * this.m_Width * this.m_Height);
}

Now, if you pass that function into a callback, it will return undefined.
As the C# example below shows, this is not what you'd get in a proper programming language.

Now I understand "this" in JavaScript has different semantics.
But since TypeScript requires writing this.m_Length, etc, it should use the correct this.

There is a suggested work-around in
https://daveceddia.com/avoid-bind-when-passing-props/

Rather than using arrow-functions (which don't always work, btw), one could bind the method in the constructor, the methods should be bound to "this" in the constructor.

It would be nice if TypeScript supported an "attribute" like @autoBind out of the box.
Because right now, I always have to remember to add the constructor-binding for every method I add.
This is very error prone.
An attribute that would instruct the transpiler to do this "automagically" at compile-time would be better.

But I would prefer not to have to import a module at runtime, like described by Dave Ceddia.
import autobind from 'autobind-decorator';
I would prefer if this were done at compile/transpile time.
Maybe in addition to that, it would be nice to have another attribute like @ignore_autobind, which could be set at an individual method that should not be auto-bound.

In addition, this should also support properities.
Because of the way typescript transpiles, I can't manually add a constructor-binding to "this" for a PROPERTY.

Doing constructor-binding would have the additional advantage over arrow-functions in that it isn't 30% slower.

class foo
{
    private m_Width: number = 0.0;
    private m_Length: number = 0.0;
    private m_Height:number = 0.0;
    constructor(x, y, z) { 
        this.m_Width = x;
        this.m_Length = y;
        this.m_Height = z;

        this.calculateVolume = this.calculateVolume.bind(this);
    }

    public calculateVolume():number {
        return (this.m_Length * this.m_Width * this.m_Height);
    }
}

Same/similar thing in C# works correctly, since "this" works as a instance-of-class-reference, and not an "arbitrary" context.


    public class foo
    {
        private bool m_bar = false;


        public delegate string callback_t();

        public bool bar{
            get { return this.m_bar; }
            set { this.m_bar = value; }
        }

        public string getBar()
        {
            return this.bar.ToString();
        }


        public void showMessage(callback_t callback)
        {
            System.Console.WriteLine(callback());
        }
    }



    class Program
    {
        

        [System.STAThread()]
        static void Main(string[] args)
        {


            var x = new foo();
            var y = new foo();
            x.bar = true;
            y.bar = false;

            x.showMessage(x.getBar);
        }
    }
@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Sep 16, 2017

Unfortunately this isn't in our scope - check out non-goals for the project:

Provide additional runtime functionality or libraries. Instead, use TypeScript to describe existing libraries.

TypeScript likely won't ever provide decorators or any other code that another library could provide instead.

@DanielRosenwasser DanielRosenwasser added Declined The issue was declined as something which matches the TypeScript vision Out of Scope This idea sits outside of the TypeScript language design constraints labels Sep 16, 2017
@ststeiger
Copy link
Author

Stupid.

@DanielRosenwasser
Copy link
Member

I'm sorry if I've upset you, but that sort of discourse isn't really helpful and isn't welcome here. Please keep our code of conduct in mind when participating in our repo.

@ststeiger
Copy link
Author

Just to clariy:
What I mean by "stupid" is, it makes no sense to require the TS user to download 85kb of require, and 5 kb of autobind-script for a 1-3kb script. Therefore I would like to have it as TS compile-time feature, and not as external library at runtime.

Now, if this is solved via decorater, keyword, command-line-switch/option or whatever, I really don't care.

All I care about is that it's very error-prone to have to do it manually in every constructor of every class for every method, including every change anybody makes to it, and therefore it shouldn't be done manually - ever.

Having policies/guidelines that will lead to requiring mobile users to download 100 kb of additional superfluous javascript is stupid, and a great way to to make your websites load slow in no time, especially if this code-bloat occurs multiple-times.

Also, it's a great way to destroy the web for millions of users with slow internet connections and/or metered connections.

As is requiring dependencies to 2 additional external libraries for something that can/could be done at compile-time.

But have it your way.
I will be looking for something with more reasonable design-decisions than TypeScript, then.
Looks like Babel would do the trick.
Have a nice day.

@kitsonk
Copy link
Contributor

kitsonk commented Sep 18, 2017

Binding of methods is very specific to their use case. It is always better/more efficient to not bind unless you have to (as discussed by Ryan in this comment). You seem to imply there is a one size that fits all. You also argue for a decorator that could easily be written by yourself and applied as you see fit. Given the narrow use case (a method in a class that access this being passed into another scope) for something that could be created by yourself, it being "bundled" with TypeScript makes it wholly immaterial your argument about code size. You seem to be assuming that there is some magic way that TypeScript can emit tighter code than you can author yourself, for your specific use case.

Good luck with Babel.

@RyanCavanaugh RyanCavanaugh added the Suggestion An idea for TypeScript label Sep 18, 2017
@RyanCavanaugh
Copy link
Member

If the difference between

class C {
  fn() {
  }
}

and

class C {
  fn = () => {
  }
}

is too much to manage, I'm not sure Babel will solve your problems either.

@ststeiger
Copy link
Author

ststeiger commented Sep 19, 2017

@RyanCavanaugh: Thank you very much for your very nice comment, but I'm actually aware what arrow-functions are.
Now appart from the ugly syntax, if you don't see why 30% slower or 100kb larger is a problem, why don't you go back coding using jQuery or Angular, if you don't mind. Good luck updating to v3 in advance (that is, if you didn't use v1), when and if it comes. By the way, I highly recommend you try jQuery-UI and/or jQuery-mobile. You'll love it. Note that you'll get the best results if you combine it with bootstrap and/or Angular and requireJS. You'll get additional bonus points if you manage to combine that with asp.net web-forms and viewstate(s) (using .NET Framework 2.0).

Oh, and by the way: Try call a base-class method from a derived class using super if you use arrow-functions. Good luck, because it isn't possible.

@kitsonk
Copy link
Contributor

kitsonk commented Sep 19, 2017

👍 your own comments and then using a shadow account to reinforce your viewpoint... priceless...

@ststeiger
Copy link
Author

LoL

@microsoft microsoft locked and limited conversation to collaborators Sep 19, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Declined The issue was declined as something which matches the TypeScript vision Out of Scope This idea sits outside of the TypeScript language design constraints Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants