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

Constructor functions are not recognized as equivalent to classes #27328

Closed
joma74 opened this issue Sep 24, 2018 · 10 comments
Closed

Constructor functions are not recognized as equivalent to classes #27328

joma74 opened this issue Sep 24, 2018 · 10 comments
Labels
Bug A bug in TypeScript checkJs Relates to checking JavaScript using TypeScript Domain: JSDoc Relates to JSDoc parsing and type generation
Milestone

Comments

@joma74
Copy link

joma74 commented Sep 24, 2018

TypeScript Version: 3.1.0-dev.20180922

Search Terms:

js constructor function

Code

// clazz1c.js
function clazz1c(){
	this.constructorOnly = 0 // (1.) [ts] 'this' implicitly has type 'any' because it does not have a type annotation.
	this.constructorUnknown = undefined // (1.) [ts] 'this' implicitly has type 'any' because it does not have a type annotation.
}

clazz1c.prototype.method = function() {
	this.constructorOnly = 3
	this.constructorUnknown = "plunkbat"
}

module.exports=clazz1c
// node-global.ts
/// <reference types="node" />
export {}
import Clazz1cType = require("../clazz1c") // (2.) (alias) function Clazz1cType(): void
var clazz1bGlobalVar: Clazz1bType // (2.) var clazz1bGlobalVar: any
declare global {
	namespace NodeJS {
		interface Global {
			clazz1bGlobalVar: Clazz1cType // (4.) (property) NodeJS.Global.clazz1bGlobalVar: any
		}
	}
}
// index.js
 var clazz1c = require("./clazz1c") // (3.) var clazz1c: typeof clazz1c
 var clazz1cInstance = new clazz1c(); // (3.) var clazz1bGlobalVar: any
global.clazz1bGlobalVar=clazz1bInstance // (4.) (property) NodeJS.Global.clazz1bGlobalVar: any
// jsconfig.json
{
    "compilerOptions": {
		"allowSyntheticDefaultImports": true,
		"checkJs": true,
		"noEmit": true,
		"module": "commonjs",
		"moduleResolution": "node",
		"strict": true,
		"noImplicitAny": true,
		"noImplicitThis": true,
		"target": "es5",
        "baseUrl": ".",
        "lib": [
            "es2015",
            "dom"
		]
	},
    "exclude": [
        "node_modules"
    ],
    "typeAcquisition": {
        "enable": false
    }
}

Expected behavior:

  1. this inside the constructor is infered based on this-property assignments
  2. function is recognized as constructor function; assignment gives type of class?
  3. type equivalent to a class t.i. (alias) class clazz1c (per default type is exported too)
  4. typed equivalent to type of constructor function of clazz1c, sth like new() => clazz1c?

Actual behavior:

  1. this inside the constructor is not infered based on this-property assignments
  2. function is not recognized as constructor function; assignment gives any type
  3. type is not recognized as equivalent to a class
  4. typed as any

Playground Link:
For (1) see http://www.typescriptlang.org/play/#src=function%20clazz1c()%7B%0D%0A%09this.constructorOnly%20%3D%200%0D%0A%09this.constructorUnknown%20%3D%20undefined%0D%0A%7D%0D%0A%0D%0Aclazz1c.prototype.method%20%3D%20function()%20%7B%0D%0A%09this.constructorOnly%20%3D%203%0D%0A%09this.constructorUnknown%20%3D%20%22plunkbat%22%0D%0A%09%7D%0D%0A%0D%0Amodule.exports%3Dclazz1c

(2), (3) and (4) not in playground link, but doable as github tag on request

Related Issues:
None found

@ghost
Copy link

ghost commented Sep 25, 2018

I think this is the intended behavior under noImplicitThis. There's no good way for us to tell what's a constructor function and what's meant to be a method. If we just assume that any function might be used as a constructor we'll miss out on a lot of errors, as all assignments to an untyped this become legal. If you don't care about that and just want your constructor functions to work you could set "noImplicitThis": false in your jsconfig.

@ghost ghost added the Working as Intended The behavior described is the intended behavior; this is not a bug label Sep 25, 2018
@joma74
Copy link
Author

joma74 commented Sep 25, 2018

@andy-ms Quoting https://github.com/Microsoft/TypeScript/wiki/Type-Checking-JavaScript-Files#constructor-functions-are-equivalent-to-classes, i interpreted that said hold without further hinting or limitations?

Before ES2015, Javascript used constructor functions instead of classes. The compiler supports this pattern and understands constructor functions as equivalent to ES2015 classes. The property inference rules described above work exactly the same way.

If not, i could be fine with annotating the constructor function with the TS comment hint @constructor. If this is the case, one should additionally PR the above documentation.

In any case, according to the title of this issue, i would like to discuss (2) (3) and (4) as not "working as intended". Neither the resulting TS signature nor the compiler's evaluated/exported type of such a constructor function are currently handled equivalent to a class. On any usage in said ts or js files, the constructor function is still interpreted as sth like (alias) function clazz1c(): void, not as a type of a class's equivalent constructor function. So when I assign that as a type to a variable, the variable gets typed as any. Guess that the compiler does not bring in the proper class-like type, but should do that. What bugs me is that else there is currently no way to type a variable to be of type clazz1c in e.g. node-global.ts, and also all the other effects described above in (2) (3) and (4).

P.S. If i rewrite clazz1c to be a proper es2015 class, the exact same usage in said ts or js files works as intended. So this issue aims to get constructor functions as equivalent to ES2015 classes recognized by TS in js and ts files

@joma74
Copy link
Author

joma74 commented Sep 28, 2018

See also #18171 relating to (1)

@ghost
Copy link

ghost commented Oct 1, 2018

That document is titled "Type Checking JavaScript Files" -- in JS files the rules are looser by default. But when you set noImplicitThis you are opting into stricter typescript-style checking.

@joma74
Copy link
Author

joma74 commented Oct 2, 2018

@andy-ms Okay, got that. So it has to be

// clazz1c.js
/**
 * @constructor
 */
function clazz1c(){
	this.constructorOnly = 0
	this.constructorUnknown = undefined
}
module.exports=clazz1c

to make noImplicitThis give no error. But for the other why parts of Constructor functions are not recognized as equivalent to classes, please consider

// std-globals.ts
export {}

import Clazz1cType = require("../clazz1c") // (A)
                                           // (C)
declare global {
	var clazz1CGlobalVar: Clazz1cType // (B)
}

Clazz1cType is just a (alias) function Clazz1cType(): void at (A). At (B) it defaults to any. Because it seems to me that Clazz1cType gets not conotated as a class-like type, as it would be if it was recognized as equivalent to classes.

Even no hand-written declaration liketype Clazz1cType = typeof Clazz1cType at (C) does make this right, as on the usage in the next file e.g.

// index.js
var clazz1cInstance = new clazz1c();

clazz1CGlobalVar=clazz1cInstance // (D)

gives [ts] Type 'clazz1c' is not assignable to type 'typeof clazz1c'. Type 'clazz1c' provides no match for the signature '(): void'. at (D)

@joma74
Copy link
Author

joma74 commented Oct 4, 2018

See also #27550.

I think the reason is that constructor functions are special-cased in resolveCallExpression -- they still only have call signatures, not construct signatures.

Although other worded, but may apply here: As in

"Clazz1cType is just a (alias) function Clazz1cType(): void call signature at (A). Because it seems to me that Clazz1cType gets not conotated as construct signature, as it would be if it was recognized as equivalent to classes.

@ghost ghost added Bug A bug in TypeScript Domain: JSDoc Relates to JSDoc parsing and type generation checkJs Relates to checking JavaScript using TypeScript and removed Working as Intended The behavior described is the intended behavior; this is not a bug labels Oct 8, 2018
@ghost ghost assigned sandersn Oct 8, 2018
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 3.3 milestone Oct 10, 2018
@sandersn
Copy link
Member

This should all work as expected now. @joma74 do you mind trying typescript@next to check?

joma74 added a commit to joma74/ts-in-js-1 that referenced this issue Aug 21, 2019
joma74 added a commit to joma74/ts-in-js-1 that referenced this issue Aug 21, 2019
@joma74
Copy link
Author

joma74 commented Aug 21, 2019

@sandersn Checked, but sorry can't see it fixed. For reference of testcases see joma74/ts-in-js-1 on branch master and branch ts-issue-global-demo. On both then run yarn & yarn ci to see tsc validation failure.

@joma74
Copy link
Author

joma74 commented Aug 21, 2019

P.S. would expect noImplicitAny to error if it would be an error :)

// std-global.d.ts
export {}

import clazz1a = require("../clazz1a")
// type clazz1a = typeof clazz1a // is i comment this "why extra"

declare global {
	var aglobalstringFromTsInJs1: string = "aglobalstringFromTsInJs1"
	var aglobalClazz1aFromTsInJs1: clazz1a <= this turns any
}

@sandersn sandersn reopened this Aug 21, 2019
@sandersn
Copy link
Member

sandersn commented Jul 27, 2020

This should be fixed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript checkJs Relates to checking JavaScript using TypeScript Domain: JSDoc Relates to JSDoc parsing and type generation
Projects
None yet
Development

No branches or pull requests

3 participants