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 returning values from constructor functions #15959

Closed
kwpeters opened this issue May 19, 2017 · 3 comments
Closed

Support returning values from constructor functions #15959

kwpeters opened this issue May 19, 2017 · 3 comments
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@kwpeters
Copy link

TypeScript Version: 2.3.2

Code

// example.js

var Person = function (firstNameOrPojo, lastName) {

    if (typeof firstNameOrPojo === "string") {
        this.firstName = firstNameOrPojo;
        this.lastName = lastName;
    } else {
        return new Person(firstNameOrPojo.firstName, firstNameOrPojo.lastName);
    }
};

Person.prototype.greet = function greet() {
    return `Hello, I am ${this.firstName} ${this.lastName}.`;
};

var fred = new Person({firstName: "Fred", lastName: "Flintstone"});

console.log(fred.greet());

Expected behavior:

I would expect tsc to compile this code without errors or warnings, especially considering that the returned type always matches the expected type.

In JavaScript, it is possible for a constructor function to return an object. When this is done, the returned object becomes the result of the whole new expression (see step 3 in this MDN documentation for the new operator).

Being concerned with types, I can see why the TypeScript compiler would be hesitant to support this quirk of the JavaScript language. After all, the compiler would have to verify that the value explicitly returned is the same type that would be returned from the constructor had it returned undefined (the usual case). I think this should be supported, because:

  1. TS claims to be a superset of JS.
  2. This is a useful and popular JavaScript technique

The above code runs just fine in Node.js and in the browser:

$ node example.js
Hello, I am Fred Flintstone.

Actual behavior:

tsc issues an error when checking this code:

$ tsc --allowJs --checkJs --outDir ./dist example.js
example.js(8,16): error TS2350: Only a void function can be called with the 'new' keyword.
@kwpeters kwpeters changed the title Support returning values of the expected type from constructor functions Support returning values from constructor functions May 19, 2017
@mhegazy
Copy link
Contributor

mhegazy commented May 19, 2017

The type is actually figured out correctly, regardless what you return; the compiler knows to use the return type when called as a function, and the instance type when used as a constructor. the error is what needs to be addressed for .js file. the pattern is still not allowed for a .ts file (since we error on the conservative side there).

@mhegazy mhegazy added Bug A bug in TypeScript Salsa labels May 19, 2017
@mhegazy mhegazy added this to the TypeScript 2.4 milestone May 19, 2017
@SylvainEstevez
Copy link

+1

Especially now that we have Proxy, there are nice behaviors that we could implement by returning a proxy instead of the class instance directly. But that won't be possible unless we can infer or specify the return type of the constructor.

@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Jun 6, 2017
@MasterJames
Copy link

MasterJames commented Sep 26, 2017

if you could put
get(target, name) {}
and
set(target, name, value) {}
inside the class constructor to override property assignment operators for (any all properties of) Object/Array, then well... javascript wouldn't be falling short.
[Otherwise ya returning the new Proxy with the handler functions would suffice.]

@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

5 participants