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

Switch default for useDefineForClassFields in ESNext #34787

Closed
DanielRosenwasser opened this issue Oct 28, 2019 · 17 comments
Closed

Switch default for useDefineForClassFields in ESNext #34787

DanielRosenwasser opened this issue Oct 28, 2019 · 17 comments
Assignees
Labels
Breaking Change Would introduce errors in existing code ES Next New featurers for ECMAScript (a.k.a. ESNext) Rescheduled This issue was previously scheduled to an earlier milestone Suggestion An idea for TypeScript

Comments

@DanielRosenwasser
Copy link
Member

Since this targets 3.7.1, I'm going to close this bug. When class fields reaches stage 4, I'll do the final two steps.

Originally posted by @sandersn in #27644 (comment)


useDefineForClassFields should be switched to true when targeting ESNext.

@DanielRosenwasser DanielRosenwasser added ES Next New featurers for ECMAScript (a.k.a. ESNext) Suggestion An idea for TypeScript Breaking Change Would introduce errors in existing code labels Oct 28, 2019
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 3.8.0 milestone Oct 28, 2019
@sandersn
Copy link
Member

sandersn commented Dec 30, 2019

Class fields aren't stage 4 yet. Moving to 3.9.

Edit: Still not stage 4. I'll move to 4.4 when that milestone gets created.

@jelbourn
Copy link

jelbourn commented May 1, 2020

This isn't mentioned in the RC notes for 3.9- has it been bumped to a later version?

@DanielRosenwasser
Copy link
Member Author

Don't think this was done in 3.9 - @sandersn?

@sandersn
Copy link
Member

sandersn commented May 1, 2020

No, class fields still aren't at stage 4. For 4.0, I did make #37894, which makes suspicious accessor/property override patterns an error everywhere. Together with the included codefix, that finishes off all the planned work for #27644 except this issue.

@RyanCavanaugh RyanCavanaugh added the Rescheduled This issue was previously scheduled to an earlier milestone label Aug 31, 2020
@CarterLi
Copy link

CarterLi commented May 27, 2021

2484210 changed default for useDefineForClassFields which broke my code and took me more than 1 hour to find the reason.

// TS 4.3.0-beta
class A { a: string } // => class A { }

// TS 4.3.2
class A { a: string } // => class A { a }

Such patterns are commonly used with decorators

class MyComponent {
  @property() a: string;
}

It's ok to introduce break changes and it's easy to revert to the old behavior too. The problem is that the release note said nothing about this.

If it's expected, please note it somewhere. Thanks.

@ExE-Boss
Copy link
Contributor

This has shipped in TypeScript 4.3.1‑rc, so it probably should’ve been kept in the TypeScript 4.3.1 milestone.

@brainkim
Copy link

brainkim commented Jun 13, 2021

Just ran into this as a bug. Was using target: ESNEXT and class properties to define types for classes. I had defined a property on the prototype as an optimization, and when trying to do an upgrade of TypeScript started noticing that the property was overwritten to be undefined. Really would appreciate this kind of change appear in the release notes.

Alternatively, if there were a way to define the types of class properties without buying into the whole TC39 class properties spec, that would be fantastic.

@nicolo-ribaudo
Copy link

nicolo-ribaudo commented Jun 13, 2021

Alternatively, if there were a way to define the types of class properties without buying into the whole TC39 class properties spec, that would be fantastic.

You can write declare before the property name.

class A {
  declare x: string;
}

@brainkim
Copy link

brainkim commented Jun 13, 2021

@nicolo-ribaudo
I consider declare and most modifiers to be a little too noisy, but yeah that’s what I’m doing. Thanks for the tip!

One potential option would be to allow us to define properties via sibling interface:

interface Point {
  x: number;
  y: number;
}

class Point {
  constructor() {
    this.x = 0;
    // the absence of this line should cause an error
    // this.y = 0;
  }
}

@ExE-Boss
Copy link
Contributor

@brainkim

One potential option would be to allow us to define properties via sibling interface:

interface Point {
	x: number;
	y: number;
}

class Point {
	constructor() {
		this.x = 0;
		// the absence of this line should cause an error
		// this.y = 0;
	}
}

You can do that already.

It’s used in the @types/node package: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/d8e2ccd84ce78d70e48863e874653e2d8f9b75ae/types/node/events.d.ts#L21-L22

@brainkim
Copy link

@ExE-Boss
But you’re losing the strict property initialization checks right?

@sandersn
Copy link
Member

sandersn commented Aug 3, 2021

Class fields moved to stage 4 in the April 2021 meeting, slated for publication in ES2022.

@sandersn sandersn closed this as completed Aug 3, 2021
@robpalme
Copy link

robpalme commented Aug 3, 2021

For reference, this was fixed by PR #42663 and released in TypeScript 4.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change Would introduce errors in existing code ES Next New featurers for ECMAScript (a.k.a. ESNext) Rescheduled This issue was previously scheduled to an earlier milestone Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests