-
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
Stricter interfaces/literal types #6352
Comments
Obviously we'd need a description of some desired semantics to make a call, but I think the bar here is relatively high. The type of interface SupportedStyles { backgroundColor?: string }
function applyStyles(el: Element, style: SupportedStyles) {}
const styles: { [key: string]: SupportedStyles } = {
header: {
backroundColor: '#eee', // Error issued here
},
}; |
That seems like a reasonable workaround for now. Thanks. I'll come back with any more relevant info after I try that. |
@RyanCavanaugh So I tried out your suggestions, but I get an error when I try to access interface StyleProps {backgroundColor: string;}
function applyStyles(el: Element, style: StyleProps) {}
const styles: {[key:string]: StyleProps} = {
header: {
backroundColor: '#eee',
},
};
// Error: unknown property "header" on {[key:string]:StyleProps}
applyStyles(document.getElementById('#header'), styles.header); If I switch to |
You could do this? I'm not sure I'm handling all your use cases here, though: interface StyleProps {backgroundColor: string;}
function applyStyles(el: Element, style: StyleProps) {}
const styles = {
header: {
backgroundColor: '#eee',
} as StyleProps
};
// OK
applyStyles(document.getElementById('#header'), styles.header); |
Circling back: The workaround I'm thinking of at this point is to just have a
Then this will be type-checked: const styles = { It's just unfortunate that this kind of boilerplate is required since it relies on callers being well-behaved. It doesn't matter too much in the StyleProps use-case since the issue of passing unknown properties is relatively low impact. However, this becomes more "interesting" in a security-sensitive context where I want to whitelist what properties can be assigned to a dom node:
That being said, I suppose the only truly safe way to defend against this kind of thing is runtime whitelists, since all the type-checking can just be trivially defeated by introducing |
Maybe the same problem related to union types: interface X {
}
interface Y {
z: Z;
}
interface Z {
a: string;
b: string;
}
let z: Z = { // Error, which is ok: "Type: '{}' is not assignable to type 'Z'. Property 'a' is missing in type '{}'."
};
let foo: X | Y = {
z: { // Not an error but it should be: if foo is X then it shouldn't contain field 'z', if foo is Y then 'z' field should contain 'b'
a: "abc"
}
} Can I force the compiler to show an error message if "b" field is not provided at "foo.z.b"? |
This is an upstream wish for exact types or the "non-casting type assertion" operator |
I love #3823 and the way it catches lots of errors early. Unfortunately, it doesn't quite go as far as I'd like it to. Suppose you have code like this, where inlining all properties of styles.header is undesirable for whatever reason (e.g., you can imagine such a block getting very large with many style options to set):
Here, the
applyStyles
call will not catch the extra "typo" property (backroundColor
instead ofbackgroundColor
) in the shape of styles.header, which has all optional properties (corresponding to Element.prototype.style properties).My only option to make sure these things checked right now is I guess to maintain a whitelist of accepted properties that is verified at runtime for each call to the function in question. A way to check this at compile time instead seems like it would be pretty valuable. I know this was discussed before and turned down in favor of only checking inline object literals, but I didn't see this kind of use-case clearly articulated in that discussion so I figured I'd open another issue and see if anyone else still thinks there should be a way to say "no, truly, do not ever pass me anything other than this set of options".
The text was updated successfully, but these errors were encountered: