Function declaration with aliased this not treated as a constructor

[uniqueiniquity]

TypeScript Version: 3.9.7

Search Terms:
this, alias

Code

// @filename: a.js
var libObjects = {}

libObjects.dataService = function (url) {
	var scope = this;

	/**
	 * updates some data
	 */
	scope.updateData = function () {
		//...
	}

	/**
	 * stop all processing
	 */
	scope.disable = function () {
		//...
	}
}

// @filename: b.js
var service = new libObjects.dataService("");
service
//   ^?

Expected behavior:
service: dataService and should offer updateData and disable in completion.

Actual behavior:
service: any

Playground Link: Workbench Repro

Related Issues:
#36618

[uniqueiniquity]

cc: @DanielRosenwasser, with whom I discussed this earlier
cc: @minestarks, who is interested in grouping JS issues
cc: @sandersn, who Daniel suggested might be interested

[DanielRosenwasser]

The object isn't relevant in this case - you just need the function declaration and the aliased this.

function Foo(url) {
  var scope = this;

  scope.callbackFn = function () {
    //...
  }
}

var foo = new Foo();
//  ^?

[typescript-bot]

👋 Hi, I'm the Repro bot. I can help narrow down and track compiler bugs across releases! This comment reflects the current state of the 2 repros in this issue running against the nightly TypeScript. If something changes, I will post a new comment.

---

Issue body code block by @uniqueiniquity

⚠️ Assertions:

• var service: any

Comment by @uniqueiniquity

‼️ Exception: TypeError - Cannot read property 'ES2016' of undefined

TypeError: Cannot read property 'ES2016' of undefined
    at Object.twoslasher (/home/runner/work/_actions/microsoft/TypeScript-Twoslash-Repro-Action/master/dist/index.js:820:29)
    at /home/runner/work/_actions/microsoft/TypeScript-Twoslash-Repro-Action/master/dist/index.js:3764:29
    at Array.map (<anonymous>)
    at Object.runTwoslashRuns (/home/runner/work/_actions/microsoft/TypeScript-Twoslash-Repro-Action/master/dist/index.js:3730:43)
    at run (/home/runner/work/_actions/microsoft/TypeScript-Twoslash-Repro-Action/master/dist/index.js:1267:43)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)

Historical Information

Issue body code block by @uniqueiniquity

Version	Reproduction Outputs
3.5.3, 3.6.4, 3.7.5, 3.8.3, 3.9.5, Nightly	⚠️ Assertions: var service: any

Comment by @uniqueiniquity

Version	Reproduction Outputs
Nightly	‼️ Exception: TypeError - Cannot read property 'ES2016' of undefined TypeError: Cannot read property 'ES2016' of undefined at Object.twoslasher (/home/runner/work/_actions/microsoft/TypeScript-Twoslash-Repro-Action/master/dist/index.js:820:29) at /home/runner/work/_actions/microsoft/TypeScript-Twoslash-Repro-Action/master/dist/index.js:3764:29 at Array.map (<anonymous>) at Object.runTwoslashRuns (/home/runner/work/_actions/microsoft/TypeScript-Twoslash-Repro-Action/master/dist/index.js:3730:43) at run (/home/runner/work/_actions/microsoft/TypeScript-Twoslash-Repro-Action/master/dist/index.js:1267:43) at processTicksAndRejections (internal/process/task_queues.js:93:5)
3.5.3, 3.6.4, 3.7.5, 3.8.3, 3.9.5	⚠️ Assertions: var foo: any

[sandersn]

Currently constructor functions are identified in the binder using syntactic markers like this-property assignment: this.x = 1. Making aliased this work would require shifting most of the work to the checker, or else more hacks and heuristics in the binder.

[uniqueiniquity]

Makes sense.
Do we know how common a pattern this is?
@DanielRosenwasser pointed me to this style guide for Angular 1 which explicitly recommends it and has 24k stars.
Alternatively, as inspired by that guide - is there a JSDoc annotation we may want to recognize to help us here?

[sandersn]

Just based on our user tests, I believe it's pretty common. It is quite an old pattern by now, so I'm not sure how many editor-hours it still takes up. That is, the code written like this may never be opened in an editor anymore.

Existing jsdoc we have that could be added:

1. @constructor (or its alias @class) allows the construct call new Foo()
2. @this {{ callbackFn: Function }} allows the assignment scope.callbackFn = function () { ... }
3. Closure property declarations allow property access on instances of Foo:

function Foo() {
  /** @type {Function} */
  this.callbackFn
}
new Foo().callbackFn

However, there aren't likely to be any of those -- I've only ever seen @this annotations in webpack when they were trying to be noImplicitThis safe, and I've only seen @constructor in Closure codebases. And of course Closure property declarations are only used in Closure codebases.

[sandersn]

@uniqueiniquity and I prototyped a hacky solution in the binder, which looks like it would work pretty well. Basically, when binding a variable declaration that looks like var self = this, save the identifier self as thisAlias. Then when checking whether to bind binary expressions as AssignmentDeclarationKind.ThisProperty, allow this or thisAlias.

The prototype immediately pointed out some places to fix:

1. thisAlias needs to be scoped exactly like container in the binder, but stricter. It should only work inside a function, constructor or method. It shouldn't work at the top level.
2. The checker also needs to have information about thisAlias in order to check correctly. I'm not yet sure how to store or reproduce this non-local information. There are a number of options, like asking for the type of self and seeing whether it's the same as the type of this.
3. The language probably has the same requirement as the checker in a few places.