-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Need way to express hybrid types that are indexable for a subset of properties #17867
Comments
It seems like you can actually do this by using intersection types. interface I {
[key: string]: string;
}
interface C {
foo: boolean;
bar: number;
}
type T = I & C;
declare let t: T;
t.foo; // boolean
t.bar; // number
t.baz; // string |
@ajafff No, that doesn't work if you actually want to assign to a variable of that type. interface I {
[key: string]: string;
}
interface C {
foo: boolean;
bar: number;
}
type T = I & C;
let t: T = {
asdf: 'foo',
foo: true,
bar: 3,
}; |
I will admit that I still don't fully understand the intent here. Flow advocates are always trumpeting soundness and then this happens
Is the justification that objects initializing these types should never be constructed with dynamic keys? Is that a requirement that should be enforced? What should |
@RyanCavanaugh Sorry, I'm not trying to make this a flow vs typescript soundness debate. I'm trying to type a very specific thing referenced in the OP: interface CSSProperties {
marginLeft?: string | number
[key: string]: CSSProperties
} Arbitrary keys are allowed, but they must be of type Soundness comes in if I attempt to work around Typescript's limitation: interface CSSProperties {
marginLeft?: string | number
[key: string]: CSSProperties | string | number
} In this case, I can do this: const style: CSSProperties = {
margarineLeft: 3
} Which is wrong, but typechecks. In other words, I can typo the property names, which is one of the things I'm trying to protect against with types. For completeness sake, this would not be an error: const style: CSSProperties = {
margarineLeft: {
marginLeft: 3
}
} |
Sorry for the tangent. I just mean, the index declaration means "If you index by a string, here's what you get", so it's contradictory to allow you to write property declarations for which that statement isn't true. We can have some other syntax that means "If a property exists and isn't one of the following known properties, then it must have this type", but then we have to decide what it means when you're indexing (for read or write) by a computed property name which we can't immediately resolve to determine if it should have the index type or a declared property's type. What would you want to happen here? var s = "qqqmarginLeft".substr(Math.random() * 7);
style[s] = 32; |
Ah, yes, I understand the concern. AFAICT flow does what you describe as "If a property exists and isn't one of the following known properties, then it must have this type", but likely falters in the case that you describe. I'd be fine with a separate syntax. In your example, I would want that to be a type error. In that case, I would expect something like this to work, however: interface CSSProperties {
marginLeft?: string | number
marginRight?: string | number
[key: string]: CSSProperties
}
var s: 'marginLeft'|'marginRight' = left ? 'marginLeft' : 'marginRight'
style[s] = 32 |
Oh, and I'd only expect |
Perhaps we could use subtraction types to express this. |
It's a shame this does appear to be being discussed further. It's something that I come up against quite often when wanting to make interface shape type safety tighter. If not for the recursive case, but for the case as in the issue title. |
Just came across the exact same situation, any updates on the issue? |
I came across this limitation when trying to create a type definition for an object like this: import { PropertiesFallback } from 'csstype';
interface CSSPropertiesAndMediaQueryDelimitedCSSProperties<TLength = string | 0> extends PropertiesFallback<TLength> {
[K: string]: PropertiesFallback<TLength>
}
const styles: CSSPropertiesAndMediaQueryDelimitedCSSProperties<string | number> = {
fontSize: 32,
lineHeight: 48,
'@media (min-width: 1280px)': {
fontSize: 48,
lineHeight: 72,
},
};
This matters to me because I don’t want the nested object to contain unknown properties: const invalidNestedStyles: CSSPropertiesAndMediaQueryDelimitedCSSProperties<string | number> = {
fontSize: 32,
lineHeight: 48,
'@media (min-width: 540px)': {
fontSize: 36,
lineHeight: 54,
// This should error, because the nested objects should
// always conform to `PropertiesFallback<TLength>`
'@media (min-width: 1280px)': {
fontSize: 48,
lineHeight: 72,
},
},
}; (Note that I would also prefer to verify that the “media query” keys actually begin with the string — Maybe this would work if we could use a conditional type to describe the value of an index signature: // Note: doesn’t actually work with Typescript 3.4.5
interface CSSPropertiesAndMediaQueryDelimitedCSSProperties<string | number> {
[K: string]: K extends keyof PropertiesFallback ? PropertiesFallback[K] : PropertiesFallback;
} I got very close to this using a generic mapped type with conditional types (source): import { PropertiesFallback } from 'csstype';
export type InferredCSSAndMediaQueryDelimitedCSSProperties<
T extends object,
TLength = string | 0,
V = PropertiesFallback<TLength>
> = { [K in keyof T]: K extends keyof V ? V[K] : V }; However, when I used a generic mapped type like this, along with a generic function, I ran into a separate limitation on type-checking spread objects containing duplicate computed keys (maybe #25758). This gave me enough doubt in the robustness of this workaround that I sought out this issue. — Thanks to these two comments (#17867 (comment) and #17867 (comment)), I also tried an intersection type (source): import { PropertiesFallback } from 'csstype';
export interface MediaQueryDelimitedCSSProperties<TLength = string | 0> {
[K: string]: PropertiesFallback<TLength>;
}
export type CSSAndMediaQueryDelimitedCSSProperties<
TLength = string | 0
> = PropertiesFallback <TLength> &
MediaQueryDelimitedCSSProperties<TLength>; However, as @aaronjensen pointed out in his follow-up comment (#17867 (comment)), this doesn’t correctly support assignment (or in my case, calling a function with an argument of this type). — Anyway, I hope that sheds some light on possible use cases. |
I'm encountering this issue lately, when implementing API schema validation. type HeaderSchema = {
header: {
foo: string,
bar: string
}
};
type ItemSchema = {
[key: string]: {
faz: number,
baz: number
}
} & HeaderSchema;
const value: ItemSchema = {
header: {
foo: 'foobar',
bar: 'foobar'
},
helloworld: { // This fails
faz: 123,
baz: 456
}
} Has any progress been made on this? In the meantime, I can use a |
To tackle the OPs question in concise way, if you want to type some named keys and and allow additional string keys, you can do it with an Intersection but you have to exclude the named keys type MyType = {
namedKey: number | boolean;
} & Record<Exclude<string, "namedKey">, any /* or specific types you choose for string keys */>;
const x: MyType = {
namedKey: "foobar",
// ^^^^^^^^ wont work, must be number | boolean,
other: "foo",
keys: "bar",
} |
No, that doesn't work. |
The explanation in the docs:
seems to describe what happens but not why. It seems a decision has been made that the string index signatures should describe the entire possible dictionary, but the more intuitive understanding of it would be it describing '...rest' members of the object. In the The way it exists now it forces me to rewrite my typing for something like |
Is there workaround for this? |
Is this not a key case of dynamic vs. strongly typed? The potential variability in a CSS properties object is potentially IMMENSE. Is that not a key case where dynamic code and dynamic types are optimally suited? It seems like the goal to strongly type the highly arbitrary nature of a CSS properties object...but in a rather "dynamic" way, is somewhat counterintuitive. Every CSS property has its own distinct rules. Would it not be better to actually fully specify the CSSProperties object with a detailed strongly typed specification, in order to properly enforce the rules for each possible CSS property? If the goal is indeed to have properly checked strong typing for a CSS object, it seems that would be the only way to ensure proper type checking across the board for any CSS property. Yes, there is a non-trivial up-front effort to develop the type for this, however once developed, then you would be able to reuse it reliably throughout even very large codebases. In fact, this seems like a problem that an OSS project could potentially solve, by specifying CSS as a TypeScript type in a simple module that could be installed with say NPM, potentially supporting thousands of projects. |
This is not something Typescript can't do, it's just something we can't tell it to do. If Typescript is inferring the type for a record with mixed fields, it works just fine: const x = Object.assign({ a: 'a' } as Record<string, string>, { b: 42 }) The string representation for the inferred type of x is Typescript allows these types, but it is using an ambiguous representation for them, and is choosing to interpret this type in the least useful way. |
Edit by @DanielRosenwasser: This might be thought of as a "rest index signature" or a catch-all index signature.
This is a feature request.
TypeScript Version: 2.4
Code
Based on the docs, this is not allowed:
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:
But this is not sound. It allows this:
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):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!
The text was updated successfully, but these errors were encountered: