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

Support super.apply from child constructor #1790

Closed
basarat opened this issue Jan 23, 2015 · 12 comments
Closed

Support super.apply from child constructor #1790

basarat opened this issue Jan 23, 2015 · 12 comments
Labels
Needs More Info The issue still hasn't been fully clarified Suggestion An idea for TypeScript

Comments

@basarat
Copy link
Contributor

basarat commented Jan 23, 2015

TypeScript is really stingy about the way you call super from the child constructor. I have seen questions on SO e.g. this one, where people want to splice and dice arguments they receive from varargs (rest parameters) before passing it on to super. My workarounds have been ugly:

class Parent {
    constructor (required1, required2, ...rest) { //...
        if(required1 === undefined) return;
    }
}

class Child extends Parent {
    constructor (...args) {
       super(undefined,undefined); // make the compiler happy
       Parent.apply(this,[val1,val2].concat(args)) // the actual call  
    }
}

Suggestion: Support super.apply(this, /*any|arr[]*/) as a part of the language grammar.

Also tagging @Steve-Fenton as he's answered similarly in the past.

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Jan 23, 2015
@RyanCavanaugh
Copy link
Member

This seems like a reasonable variant to support

@basarat basarat changed the title Support super.apply from base contructor Support super.apply from child constructor Jan 23, 2015
@RyanCavanaugh RyanCavanaugh added Needs More Info The issue still hasn't been fully clarified and removed In Discussion Not yet reached consensus labels Apr 28, 2015
@RyanCavanaugh
Copy link
Member

Is the spread operator enough to support this scenario?

@basarat
Copy link
Contributor Author

basarat commented Apr 28, 2015

Is the spread operator enough to support this scenario

Yes 🌹

@jbrantly
Copy link

Was this closed as "the spread operator will work here sometime in the future" or "the spread operator will work here now"?

I just using the spread operator in the super call and I get Supplied parameters do not match any signature of call target.

Edit: To clarify, I'm using the nightly.

@mhegazy
Copy link
Contributor

mhegazy commented Aug 20, 2015

@jbrantly can you provide more information, this works today:

class Parent {
    constructor(...rest) { 
    }
}

class Child extends Parent {
    constructor(...args) {
        super(...args); 
    }
}

@jbrantly
Copy link

Sorry, I should have been much more specific. I couldn't exactly duplicate the issue I was seeing earlier, but my question boils down to mimicking the Parent.apply(this, arguments) concept that is pretty popular in JS-land where you're not changing the parameters to the constructor. For instance:

class Parent {
    constructor(foo?: string, bar?: number) {
    }
}

class Child extends Parent {
    constructor(foo?: string, bar?: number) {
        super(...arguments); // this currently fails
    }
}

So, for my purposes, I care less about rest parameters than I do about a shorthand to just pass all parameters.

@mhegazy
Copy link
Contributor

mhegazy commented Aug 20, 2015

@jbrantly, this is a separate issue. can you file another one for this.

@jbrantly
Copy link

Yea, I was thinking the same thing. Will do.

@oriSomething
Copy link

Would you reconsider to reopen this issue?
it can make really easy to support future version of classes without manually changing super arguments

@RyanCavanaugh
Copy link
Member

What are you doing where the spread operator isn't sufficient?

@oriSomething
Copy link

@RyanCavanaugh it is possible to use spread operator, but it feels of a "boilerplate" that doesn't make code so elegant. for example, React and context which is experimental thing that added and maybe tomorrow will be removed. i might not using its API, but some third party library using it and i don't want to break it.
it would be more elegant to write it with ...arguments:

// You can do:
class SomeComponent extends React.Component<IProps, IState> {
  constructor(...args) {
    // I want to keep everything which is future related like unstable `context`
    // without the need to re-factor
    super(...args);

    // The way to extract props
    const [props] = args;

    // do something with props;
  }
}

export default someThingThatInhiritAndModifyComponent(SomeComponent);

// But this is way more elegant:
class SomeComponent extends React.Component<IProps, IState> {
  constructor(props) {
    super(...arguments);

    // do something with props;
  }
}

// And a bonus, a way which is more elegant to describe a signature
class A extends B {
  constructor(x: number, y: number) {
    super(...arguments);
    // ···
  }
}

of corse i might be wrong, but this is my view

@AJamesPhillips
Copy link

AJamesPhillips commented Jun 16, 2017

I'm confused why this issue has not received much attention (in terms of thumbs up or comments). Particularly the scenario with subclassing React.Component raised by @oriSomething above which is what I've run into before and have now finally investigated.

Just to clarify, this does not work:

export class SomeComponent extends React.Component<{}, {}> {
    constructor(...args) {
        // errors with: 
        //  [ts] Supplied parameters do not match any signature of call target.
        //  (parameter) args: any[]
        super(...args);
    }
}

@microsoft microsoft locked and limited conversation to collaborators Jun 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Needs More Info The issue still hasn't been fully clarified Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

6 participants