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 way to express hybrid types that are indexable for a subset of properties #15151

Closed
aaronjensen opened this issue Apr 12, 2017 · 9 comments
Closed
Labels
Question An issue which isn't directly actionable in code

Comments

@aaronjensen
Copy link

TypeScript Version: 2.2.1

Code

interface CSSProperties {
  marginLeft?: string | number
  [key: string]: CSSProperties
}

Based on the docs, this is not allowed:

While string index signatures are a powerful way to describe the “dictionary” pattern, they also enforce that all properties match their return type. This is because a string index declares that obj.property is also available as obj[“property”]. In the following example, name’s type does not match the string index’s type, and the type-checker gives an error:

Unfortunately, it seems to make this type (which is common amongst jss-in-css solutions) not expressible. Coming from flow, which handles index types by assuming they refer to the properties that are not explicitly typed, this is frustrating.

As it stands, you can workaround it with:

interface CSSProperties {
  marginLeft?: string | number
  [key: string]: CSSProperties | string | number
}

But this is not sound. It allows this:

const style: CSSProperties = {
  margarineLeft: 3
}

This could potentially be solved with subtraction types if they allowed subtracting from string (and you were allowed to specify key types in this way):

interface CSSProperties {
  marginLeft?: string | number
}

interface NestedCSSProperties extends CSSProperties {
  [key: string - keyof CSSProperties]: CSSProperties
}

I asked about this on stackoverflow to confirm that I wasn't missing something. It seems I'm not, so I guess I'd consider this a suggestion/discussion starter, since it's probably not a "bug". Thanks!

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label May 24, 2017
@bilalq
Copy link

bilalq commented Jun 1, 2017

I've run into the same issue. With the type system as it stands, I'm unable to express the type of a UISchema from the react-jsonschema-form library.

interface UiSchemaMap {
  [propertyName: string]: UiSchema;
}

interface UiSchemaItemOptions {
  addable?: boolean;
  orderable?: boolean;
  removable?: boolean;
}

interface UiSchema {
  classNames?: string;
  items?: UiSchemaMap;
  'ui:disabled'?: boolean;
  'ui:dirty'?: boolean;
  'ui:field'?: string;
  'ui:options'?: UiSchemaItemOptions;
  'ui:order'?: string[];
  'ui:readonly'?: boolean;
  'ui:rootFieldId'?: string;
  'ui:widget'?: string;
  [other: string]: UiSchema | undefined; // This is failing to compile
}

So far, I've found two possible workarounds, but they're both far from ideal. Let's use this minimal failing example:

interface NotWorking {
    name: string;
    age: number;
    [other: string]: NotWorking;
}

One way to get it passing is to instead declare the type of the indexed type as any. This allows us to keep detailed type information when accessing the name and age properties, but we end up having to cast for others.

interface Working {
    name: string;
    age: number;
    [other: string]: any;
}

The other way is to declare all the other types as union types and cast those. This also pains me to do.

interface Working {
    name: string | Working;
    age: number | Working;
    [other: string]: Working;
}

It would be really great if indexed properties could have a consistent type that differed from the named and explicitly declared properties.

@mhegazy
Copy link
Contributor

mhegazy commented Jun 13, 2017

An index signature adds a constraint on the type; since named properties are just a subset of what can be used to index into an object, they need to fit into the constraint defined by the index.

use intersection types to achieve what you are looking for:

type T = { a: number, b: string } & { [x: string]: boolean };

@mhegazy mhegazy added Question An issue which isn't directly actionable in code and removed Needs Investigation This issue needs a team member to investigate its status. labels Jun 13, 2017
@aaronjensen
Copy link
Author

@mhegazy Have you tried what you are suggesting? It does not work. It is no different from combining the types as described above. Please adjust the labels accordingly, this is not a question, consider this a feature request or a bug.

http://www.typescriptlang.org/play/#src=type%20T%20%3D%20%7B%20a%3A%20number%2C%20b%3A%20string%20%7D%20%26%20%7B%20%5Bx%3A%20string%5D%3A%20boolean%20%7D%3B%0A%0Aconst%20t%3A%20T%20%3D%20%7B%0A%20%20%20%20a%3A%204%2C%0A%20%20%20%20b%3A%20'hi'%2C%0A%20%20%20%20c%3A%20false%0A%7D

@mhegazy
Copy link
Contributor

mhegazy commented Jun 13, 2017

i did not try that to be frank. i was more thinking of using the type to access the properties.. e.g.

declare const t: T;
t.a // number;
t["b"] // string
t["somother"] // boolean

@aaronjensen
Copy link
Author

My particular case doesn't have much use for a type that can only be read. Honestly, it's a bit surprising that typescript doesn't realize that an impossible type is being made.

I don't know all of the ramifications, but I think that this design decision should be reconsidered. Flow does it the other way (the map type is the fallback, not an intersection) and I do not know of any issues with that.

@bilalq
Copy link

bilalq commented Jul 6, 2017

Could the label on this be updated to "feature request" instead of "question" as @aaronjensen suggested?

@mhegazy
Copy link
Contributor

mhegazy commented Aug 17, 2017

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.

@mhegazy mhegazy closed this as completed Aug 17, 2017
@aaronjensen
Copy link
Author

@mhegazy Uh, the issue labels are wrong. See the actual issue. This is a feature request and it is quite actionable.

@aaronjensen
Copy link
Author

Reopened as #17867

@microsoft microsoft locked and limited conversation to collaborators Jun 21, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Question An issue which isn't directly actionable in code
Projects
None yet
Development

No branches or pull requests

4 participants