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

No excess property error for computed property #36920

Open
jcalz opened this issue Feb 21, 2020 · 7 comments
Open

No excess property error for computed property #36920

jcalz opened this issue Feb 21, 2020 · 7 comments
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@jcalz
Copy link
Contributor

jcalz commented Feb 21, 2020

TypeScript Version: 3.7.5 and 3.9.0-dev.20200220

Search Terms: excess property, computed property, index signature

Code

interface Foo {
	a: string;
} 

const bar: Foo = {
	a: "",
	z: "" // error, excess prop
}

const baz: Foo = {
	a: "",
	["Z".toLowerCase()]: "" // this is okay I guess?
}

Expected behavior:
The computed property in the definition of baz should maybe produce an excess property error, since Foo has no string index signature and "Z".toLowerCase() is only known to be a string.

Actual behavior:
The computed property is allowed.

Playground Link: Here

Related Issues: #22427

This issue is a re-opening of #22427, which was marked as a bug and closed as "fixed", but I don't see an actual fix referenced anywhere in that issue and the behavior described in that issue persists. Any ideas? Thanks!

There's an SO question asking this, which brought me to #22427, which has me scratching my head 😕.

@RyanCavanaugh
Copy link
Member

Is the proposed rule here that all computed properties are disallowed, or just that any computed property is disallowed if all the known keys of the target type have already been written?

@RyanCavanaugh
Copy link
Member

e.g. this doesn't seem wrong per se:

interface Foo {
  a: string;
  b: string;
} 

const baz: Foo = {
  a: "",
  b: "",
  [Math.random() > 0.5 ? "a" : "b"]: "overwritten"
}

@jcalz
Copy link
Contributor Author

jcalz commented Feb 21, 2020

If computed property keys are string or widened to string then I'd think there should be an excess property warning since the compiler can no longer track whether the properties are valid at all.

In the "a" | "b" case, currently these are widened to string, which is itself a bug: #13948 If it stayed "a" | "b" then I'd expect {[Math.random()>0.5 ? "a" : "b"]: ""} to be of type {a: string} | {b: string} and whether that triggers any warning when combined with other properties is an interesting question but maybe not directly relevant for this particular issue. Still, my instinct is:

  • don't issue a "multiple property" warning like you'd get with {a: 1, a: 1};
  • if any of the keys in the union are unknown then issue an excess property warning; const f: Foo = {a: "", b: "", [Math.random()<0.5 ? "c" : "d"]: ""}; // error! c and d are extra, you are bad
  • if any of the keys are not otherwise set then issue a "this property might be missing" warning: const f: Foo = {"b": "", [Math.random()<0.5 ? "a" : "b"]: ""}; // error! a might be missing, not necessarily a Foo

which means no warning for your above example.

@RyanCavanaugh RyanCavanaugh added In Discussion Not yet reached consensus Suggestion An idea for TypeScript labels Feb 21, 2020
@jcalz
Copy link
Contributor Author

jcalz commented Feb 23, 2020

So can I take it that #22427 was misclassified as a bug since we're treating it here as a suggestion? I'm fine either way; I mostly want to have an as-authoritative-as-possible answer to the linked SO question.

@timonson
Copy link

timonson commented Mar 6, 2020

@RyanCavanaugh I also would like to know if this is considered to be a bug or not.

It seems kinda strange and inconsistent to me that bar throws an error and baz doesn't. Because the same thing happens in both examples in the end:

interface Foo {
	a: string;
} 

const bar: Foo = {
	a: "",
	z: ""
}

const baz: Foo = {
	a: "",
	["Z".toLowerCase()]: "" 
}

@jcalz
Copy link
Contributor Author

jcalz commented Jul 17, 2023

Is #44794 related to this or not?

@jcalz
Copy link
Contributor Author

jcalz commented Jul 17, 2023

Analogous behavior happens with symbol computed properties:

interface Foo {
  a: string;
}

const s = Symbol("abc");

const bar: Foo = {
  a: "",
  [s]: "" // error, excess prop
}

const t: symbol = s;

const baz: Foo = {
  a: "",
  [t]: "" // this is okay I guess?
}

Playground link

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

3 participants