TypeScript
TypeScript copied to clipboard
microsoft
Target ES2022 + Module CommonJS silently produces code with TypeError in runtime
Bug Report
This bug exists in my project, not only playground, but playground seem to also have bugs, because i can't even seem to reliably reproduce it in the playground – sometimes it does not react on Target change. But right now i have an opened playground tab with this bug.
I will duplicate the MRE code right here as a text. This is the source code
class Helper {
create(): boolean {
return true
}
}
export class Broken {
constructor(readonly facade: Helper) {
console.log(this.bug)
}
bug = this.facade.create()
}
new Broken(new Helper)
This is the compiled code with target: ES2022 and module: CommonJS
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.Broken = void 0;
class Helper {
create() {
return true;
}
}
class Broken {
facade;
constructor(facade) {
this.facade = facade;
console.log(this.bug);
}
bug = this.facade.create();
}
exports.Broken = Broken;
new Broken(new Helper);
If run this JS – we will get TypeError
Cannot read properties of undefined (reading 'create')
⏯ Playground Link
Playground link with relevant code
I'm also sharing the playground link, but i assume it could produce the right code for you if you open it first time.
To reliable reproduce – change module to Node16 and then to CommonJS. You should see this result
I believe that this is due to https://github.com/microsoft/TypeScript/pull/36425
@sandersn why don't we give an error for referencing a parameter property in a field initializer when useDefineForClassFields is set?
Is target: ES2022 supposed to produce such code? If i downgrade to target: ES2021 the problem disappears
In newer targets, useDefineForClassFields is set to true because it's spec-compliant. To be honest though, even I'm somewhat surprised that we changed the initialization ordering.
even I'm somewhat surprised that we changed the initialization ordering
It’s impossible for facade to be initialized before bug in this code unless the compiler transpiles bug to a constructor-initialized property (as it does pre-ES2022).
The code generation here is as-expected; the bug is the lack of error message to flag the use-before-init.
this.facade = facade;
Not directly related to the issue but if target is ES2022 (which implies useDefineForClassFields: true) then why isn't this being initialized with defineProperty? Are parameter properties exempt?
Parameter properties are considered to be sugar for a this.e = e assignment in the constructor.
The code generation here is as-expected
Doesn't it reduces readability? I was so happy TypeScript handles initialization ordering in constructor for me. Now i will be forced to rewrite all such parts in the whole project with less accurate code. At least, as i understand, it requires 2 lines of code instead of 1. One line with field type declaration, and second line with value assignment in constructor.
It looks like our expectation (prior to useDefineForClassFields), was that parameter property initializers are assigned before instance initializers, as evidenced by the emit when useDefineForClassFields is false:
constructor(facade) {
this.facade = facade;
this.bug = this.facade.create();
console.log(this.bug);
}
I would contend that the issue is our emit when useDefineForClassFields is true isn't correctly moving the field initializers into the constructor. Really, the emit should be this:
facade;
bug;
constructor(facade) {
this.facade = facade;
this.bug = this.facade.create();
console.log(this.bug);
}
FWIW, babel also produces code where this is a runtime error. If this is the desired initialization order, then it should be a compiler error.
Hi, anything guys? I also faced that, to say more we have ngrx code, which declares multiple effects in a class based on received in the constructor values, but these declared properties are not declared in the constructor. That is overkill.
From other fields, like kotlin or scala - you have a constructor, and later you can use these values from the constructor, to create other properties.
That should be obvious, and IMO it was for many users before ES2022.
