Function declaration with aliased `this` not treated as a constructor
uniqueiniquity opened this issue · 7 comments
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
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
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();
// ^?👋 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
var service: any
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 |
|
Comment by @uniqueiniquity
| Version | Reproduction Outputs |
|---|---|
| Nightly |
|
| 3.5.3, 3.6.4, 3.7.5, 3.8.3, 3.9.5 |
|
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.
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?
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:
@constructor(or its alias@class) allows the construct callnew Foo()@this {{ callbackFn: Function }}allows the assignmentscope.callbackFn = function () { ... }- Closure property declarations allow property access on instances of Foo:
function Foo() {
/** @type {Function} */
this.callbackFn
}
new Foo().callbackFnHowever, 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.
@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:
thisAliasneeds to be scoped exactly likecontainerin the binder, but stricter. It should only work inside a function, constructor or method. It shouldn't work at the top level.- The checker also needs to have information about
thisAliasin 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 ofselfand seeing whether it's the same as the type ofthis. - The language probably has the same requirement as the checker in a few places.