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

Compilation option: stricterPropertyInitialization #60906

Open
6 tasks done
matthew-dean opened this issue Jan 2, 2025 · 3 comments
Open
6 tasks done

Compilation option: stricterPropertyInitialization #60906

matthew-dean opened this issue Jan 2, 2025 · 3 comments
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@matthew-dean
Copy link

matthew-dean commented Jan 2, 2025

πŸ” Search Terms

Keywords: "stricter property initiazliation"

βœ… Viability Checklist

⭐ Suggestion

So, TypeScript offers the strictPropertyInitialization compiler option, which is helpful, but it has this behavior:

class UserAccount {
  name: string; // not an error, initialized in constructor
  accountType = "user"; // initialized
 
  email: string; // this is an error
  address: string | undefined; // NOT an error but ALSO not technically initialized!
 
  constructor(name: string) {
    this.name = name;
  }
}

This is great except for one thing: performance. Uninitialized properties in the class (specifically: address) will, at least in V8, potentially switch the class to dictionary mode vs struct mode when the property eventually gets assigned, leading to slower read/writes. This was confirmed by a V8 developer in this StackOverflow thread.

So the suggestion is that stricterPropertyInitialization sets strictPropertyInitialization to true AND throws an error if the author has added | undefined to the type to implicity define the type. In other words, "strict property initialization" would mean what it says: strict property initialization. If it is not initialized, it is an error. In the above example, from the tsconfig docs, the address is not initialized. It's too late to change that as default behavior, hence the additional flag.

Note: maybe stricterPropertyInitialization is too awkward? Something like disallowImplicitUndefinedClassFields? πŸ€·β€β™‚

πŸ“ƒ Motivating Example

Most devs probably won't need to worry about the performance implications of internal dictionary vs. struct. In my case, in the library I'm maintaining / working on, every millisecond / fraction of a millisecond counts, so I'm trying to determine the fastest path in every scenario. I expected strictPropertyInitialization to mean what it says to prevent properties from not being initialized and avoid any accidental performance pitfalls from non-initialization, but it doesn't. πŸ€·β€β™‚

Meaning, in the above example:

class UserAccount {
  name: string; // not an error, initialized in constructor
  accountType = "user"; // initialized
 
  email: string; // this is an error
  address: string | undefined; // NOT an error but ALSO not technically initialized!
 
  constructor(name: string) {
    this.name = name;
  }

  someMethod() {
    // The class will now possible de-optimize its performance to Dictionary mode, since V8
    // thinks that the object needs to have arbitrary properties added. It doesn't know about `address`
    this.address = '123 fake street' 
  }
}

So, most simply, stricterPropertyInitialization would throw a compilation error if | undefined is added to a property and the property is not explicitly initialized with a field initializer or in the constructor.

If the code author wishes to actually initialize the field, to undefined, it must be something like:

class UserAccount {
  // ...
  address: string | undefined = undefined;
 
  constructor(name: string) {
    this.name = name;
  }
}

πŸ’» Use Cases

  1. What do you want to use this for?
    A high-performance TS/JS library

  2. What shortcomings exist with current approaches?
    There are no workarounds as far as I know. You simply have to let all devs on your team know to not write this and hope people catch it.

  3. What workarounds are you using in the meantime?
    Vigilance

@jcalz
Copy link
Contributor

jcalz commented Jan 2, 2025

If "most devs probably won't need to worry about the performance implications of internal dictionary vs. struct", why would you expect TS to add a whole new compiler option for it? I'd expect this to be something people would do with a custom typescript-eslint rule, not a TS language feature.

But: is there a reason you're not compiling with --useDefineForClassFields so that TS class fields emit as ES/JS class fields when targeting modern runtimes? If you enable that option, then you get the normal JS behavior where declaring a class field automatically initializes it, as shown in this playground link.

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature labels Jan 2, 2025
@matthew-dean
Copy link
Author

@jcalz

But: is there a reason you're not compiling with --useDefineForClassFields so that TS class fields emit as ES/JS class fields when targeting modern runtimes? If you enable that option, then you get the normal JS behavior where declaring a class field automatically initializes it

πŸ€” Oh, okay, I wasn't aware that useDefineForClassFields had an effect on explicit definition of fields. I originally read it as changing initialization order if it had assigned values. I guess you're right that this would effectively solve the performance problem without needing the restriction of | undefined on types.

I guess one small request would be that in the docs for strictPropertyInitialization, a note would be added that properties are not actually initialized unless useDefineForClassFields is also used. I would additionally add useDefineForClassFields to the strict flag in the future to clear up this behavior. (One small side-effect is that if you're using a compile target of ES2021 or below, it looks like the output of useDefineForClassFields is extremely verbose, I guess because it looked like the JS version would end up differently at the time.)

@MartinJohns
Copy link
Contributor

Note that the default value for useDefineForClassFields depends on your ES target. Have a modern target and it is enabled by default.

Adding it to strict would be a massive breaking change, very unlikely to happen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants