All Optional Object Interface Converts to any
kitsonk opened this issue · 24 comments
I couldn't find a relevant issue, but this does seem rather fundamental and against the principle of:
- Statically identify constructs that are likely to be errors.
TypeScript Version:
1.8.7
interface Optional {
foo?: string;
}
function foo(o: Optional): void {}
foo('bar'); // should throw
foo(1); // should throw
foo({ bar: 'baz' }); // should (and does) throwExpected behavior:
Expected behaviour is that you should be able to type guard against other non-objects and only accept Objects with no properties or declared properties.
Actual behavior:
Passing anything other than an object literal that contains extra properties is acceptable.
We talked about this briefly and think the idea in #3842 is worth trying out. I ported the PR forward but the code was written incorrectly around this specific case:
interface NotAllOptional {
x: string;
}
interface AllOptional {
y?: number;
}
type mixed= NotAllOptional & AllOptional;
let z: mixed = { x: 'ok' };The code I had failed because we check intersection assignability by seeing if the source is assignable to each constituent in sequence; the first type succeeds and the second type fails (even though it shouldn't). A more complex check is needed here but I don't have time to implement it right now.
If someone wants to figure it out and send an updated PR that we could try against our internal suite of partner code, that'd be great. No guarantees it's going to be worth taking a breaking change over, but seeing what kind of issues (and non-issues) it finds might help us down the path to a more complete solution if needed.
I'm a bit confused, you just said that when you check assignability in an intersection you check the assignability of the source to both constituents, but if I have something like that:
interface A { a: string }
interface B { b: string }
const test: A & B
test = {a: 'a', b: 'b'}It works fine (as expected) even though test isn't assignable to either A or B?
What's the status on this? In this something we can reasonably expect to be part of the 2.0 release?
I've been continously porting my app to Typescript over the past month or two and this is the one feature I desperately miss, as I'm using a lot of weak-typed objects to represent the state of a React component or for DTOs.
What's the status on this? In this something we can reasonably expect to be part of the 2.0 release?
The labels indicate that it a potentially good idea (Suggestion), but that the TypeScript team have limited resources and do not feel it is important enough to address (Accepting PRs) and are currently leaving it up to the community to address (Milestone = Community).
I believe this is a bigger issue because of TypeScript named parameters.
class Foo {
bar({a = <string|undefined>void 0, b = <number|undefined>void 0} = {}) {
// Do something
}
}
let foo = new Foo();
/*
This explicitly violates the method signature,
which states that it only takes optional named parameters
"a" and "b".
*/
foo.bar("hello"); The named parameters feature for methods and functions is broken until the type system can support it.
Is this issue solved by the new object type?
Is this issue solved by the new object type?
No. a type with only optional properties is treated as {} from assignment-compatibility perspective, i.e. you can pass in almost any thing to it.
This is kind of a big deal when using the "optional options" pattern:
interface IOptions {
upsert?: boolean;
debug?: boolean;
}
function questionableBehaviour(id: string, options?: IOptions, callback?: Function) {
// code
}
questionableBehaviour('testId', 'invalid string', (err) => console.log(err));
This IMHO should definitely not compile.
Any chance the TS team could revisit addressing this? I feel that the introduction of Partial<T> has increased the surface of this problem. It's an extremely convenient operator that you don't really realize can be a footgun until you get errors in production ( 🙋 ).
To add one more 'wat' to the OP, this is the case that keeps biting us in our codebases:
foo({ bar: 'baz' }); // should (and does) throw
const wat = { bar: 'baz' };
foo(wat); // doesn't :(@mhegazy Would the ability to extend the object type address this issue?
interface Optional extends object {
foo?: string;
}
function foo(o: Optional): void {}
foo('bar'); // should throw, because 'bar' does not extend object
foo(1); // should throw, because 1 does not extend object
foo({ bar: 'baz' }); // should (and does) throw@mhegazy Would the ability to extend the object type address this issue?
not sure i see how this helps. @RyanCavanaugh has a description of the approach we would like to try.
@mhegazy If I understand the issue right, you can get safety with an intersection type today. I believe supporting extend object will make this easier. That way it's not a breaking change (as @RyanCavanaugh's approach seems to be):
type A = object & { foo?: number }
let a: A = {}
let b: A = { foo: 1 }
let c: A = { bar: 1 } // error
let d: A = { foo: 1, bar: 2 } // error
let e: A = 1 // errorExcluding the primitives and incorrect object literals is nice but doesn't go far enough to be "solved" IMO. You can still e.g. use a class constructor when you meant to use a class instance.
e.g. use a class constructor when you meant to use a class instance.
Does that happen for anything other than the pathological case of an empty class? Supporting extends object seems like a cheaper way to the cover a lot of cases.
I just asked a related question on SO and was directed to this thread. Here is an example where the object & intersection doesn't solve the problem:
type Right = { right?: boolean } & object
type Wrong = { wrong: {fail: number} }
function returnWrongX(p: Right, callback: (param: Right) => Right) {
return callback(p).right
}
// throws a runtime error
returnWrongX({}, (param: Wrong) => param.wrong.fail ? {} : {})@bijoutrouvaille I see what you mean, in that the typing is more fragile than it should be. On the last line you're explicitly broadening param's type to Wrong, which compiles.
If you don't type param on the last line and let TS infer it as Right, then you get a compile time error as expected.
If the fix breaks some corner cases, can we still have the fix under a new compiler flag?
Example, strictNullChecks, it found tones of things that were missed before this strict check was available. Can we have the same here. Please ...
As usual, @RyanCavanaugh is right :) A few cases that compile, but shouldn't:
type A = object & { foo?: number }
let a: A = {}
let b: A = [1]
let c: A = new class { bar = 42 }
let d: A = () => 'foo'And a few more:
let e: A = null;
let f: A = /foo/;
let g: A = class A { };It has to at least work somewhat correctly for it to be considered @3n-mb.
It would be nice if you could specify that the object have at least one of the optional members. This way if you specify something that doesn't match at least one of the expected properties, it will get caught at compile time.
It would be nice if you could specify that the object have at least one of the optional members.
That can already be modelled by an intersection type:
type AtLeastOne = { foo: string; bar?: string } | { foo?: string; bar: string};
const a: AtLeastOne = { foo: 'bar' }; // ok
const b: AtLeastOne = { bar: 'baz' }; // ok
const c: AtLeastOne = {}; // error on `c`While it can be be done with an intersection, it quickly becomes unwieldy for something even relatively small like a mailing address.
While it can be be done with an intersection, it quickly becomes unwieldy for something even relatively small like a mailing address.
Even so, I suspect that would be a different request than what this issue is about.