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

ESNext emits optional class members #47350

Open
jods4 opened this issue Jan 8, 2022 · 9 comments
Open

ESNext emits optional class members #47350

jods4 opened this issue Jan 8, 2022 · 9 comments
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@jods4
Copy link

jods4 commented Jan 8, 2022

Bug Report

πŸ”Ž Search Terms

optional class members

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about optional class members

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

// Consider this class
class C { 
  x?: number; 
}

πŸ™ Actual behavior

// This is the emitted JS if you target ESNext and have --useDefineForClassFields
class C {
  x;
}

x is emitted as a class member, even though it was optional.
This feels wrong, but to make a stronger point consider --exactOptionalPropertyTypes.
With this compiler option, we should have the typing invariant !("x" in c) || typeof c.x == "number"; but with this emit, a new instance has the opposite "x" in c && typeof c.x == "undefined".

πŸ™‚ Expected behavior

Optional members should probably not be emitted:

class C { }

I am not sure / have not thought about what the implications with respect to useDefineForClassFields semantics are.

@DanielRosenwasser
Copy link
Member

I believe that this is intended. If you want to signal that this was truly not present (i.e. "this thing will potentially not be set"), then you'll need to use the declare modifier.

class C { 
  declare x?: number; 
}

@jods4
Copy link
Author

jods4 commented Jan 8, 2022

@DanielRosenwasser I think that's a good enough workaround in practice.

That said, it means x?: number (without declare) is actually an impossible contract with the compiler options mentionned in issue. Should it be an error or at least warning?

@DanielRosenwasser DanielRosenwasser added Bug A bug in TypeScript Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. labels Jan 8, 2022
@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Jan 8, 2022

Sounds like this is possibly a bug in --exactOptionalPropertyTypes.

@DanielRosenwasser
Copy link
Member

Specifically, under both --strictPropertyInitialization and --exactOptionalProperties, I think either we must require that x is initialized or given | undefined in its annotation, or we avoid narrowing obj.x to number.

class C {
  x?: number;
  y = 3;
}

function foo(obj: C) {
  if ("x" in obj) {
    obj.x.valueOf();
  }
}

@DanielRosenwasser DanielRosenwasser added In Discussion Not yet reached consensus Suggestion An idea for TypeScript and removed Bug A bug in TypeScript Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. labels Jan 8, 2022
@fatcerberus
Copy link

fatcerberus commented Jan 8, 2022

Isn’t this for consistency w.r.t. [[Define]] vs. [[Set]] semantics for fields? [[Define]] semantics require the field to be declared in the class body, or defineProperty’d in the constructor. Otherwise you get [[Set]] semantics, which can, e.g. call parent class setters.

It’d be confusing, IMO, to have some properties using [[Define]] and some using [[Set]] depending on whether they were optional or not.

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Jan 10, 2022

It's more about the fact that property declarations unconditionally add a property even if there's no initializer. If you want the type system to correctly encode whether something is possibly-missing vs. possibly-undefined, then --strictPropertyInitialization is slightly diverged in its expectations from --exactOptionalProperties.

@fatcerberus
Copy link

Right, but my point was that if foo?: string; doesn’t emit the property because it’s optional and uninitialized (as suggested in the OP), then that also changes the runtime semantics for that property ([[Set]] vs. [[Define]]), which would be confusing. useDefineForClassFields and exactOptionalPropertyTypes are thus fundamentally at odds with each other.

@pushkine
Copy link
Contributor

pushkine commented Jul 5, 2022

The discussion is so cluttered for such a clear cut bug 😭

class A { foo?: 0; }

// target: "ES2020" | new A() is { foo?: 0 } βœ…
class A {}

// target: "ESNext" | new A() is { foo: 0 | undefined } ❌
class A { foo; }

@darlanalves
Copy link

Bumped into this one today.

Extending @pushkine example, if I have a decorator to define a property with a getter, that won't work in esnext target.

Example:

class Foo {}
class C {
  @dependency() foo: Foo;
}

function dependency() {
  return function(target, property) {
    Object.defineProperty(target, property, {
        get() {
          return new Foo();
        }
    });
  }
}

// target: "ES2020" βœ…
class C {}
new C().foo instanceof Foo === true;

// target: "ESNext" ❌
//foo is `undefined` because it is a property in every instance of C, overriding the getter from C.prototype
class C { foo; }
new C().foo instanceof Foo === false;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants