TypeScript Version: 2.1? (Playground)
Code
interface Props {
foo: string;
bar: string;
}
const p: Props = {
foo: 'hello',
bar: 'world'
}
const say = (props: Props) => console.log(`${props.foo.length} ${props.bar.length}`);
say(p);
delete p.bar;
say(p); // works, but should throw
See here.
Expected behavior:
Don't compile, because we delete
bar
from p
.
Actual behavior:
Compiles. Resulting in a runtime error: Uncaught TypeError: Cannot read property 'length' of undefined
.
To track effects of delete
you need to compile with --strictNullChecks
and declare the property optional. Upon seeing delete p.bar
the control flow analyzer will then consider p.bar
to have the value undefined
.
I do use --strictNullCheck
, but I don't see why this (and declaring the property) is optional. Why is this working as intended? E.g. this won't compile:
interface Props {
foo: string;
bar: string;
}
const p: Props = { // an error happens here, because `p` is not assignable to type `Props`
foo: 'hello'
}
const say = (props: Props) => console.log(`${props.foo.length} ${props.bar.length}`);
say(p);
Using delete p.bar
changes the p
from my initial example essentially to a p
without bar
as in my second example. And a p
without bar
doesn't match Props
, so it shouldn't be possible to call say
with it.
Or am I wrong?
this compiles too without --strictNullChecks
:
const p: Props = {
foo: 'hello'
bar: undefined
}
and will result in the same runtime behavior.
I try to summarise:
Given we have the following interface, function and using --strictNullChecks
:
interface Props {
foo: string;
bar: string;
}
const say = (props: Props) => console.log(`${props.foo.length} ${props.bar.length}`);
This is invalid:
const p = {
foo: 'hello',
bar: undefined
};
say(p);
This is invalid:
const p = {
foo: 'hello'
};
say(p);
This is invalid:
const p = {
foo: 'hello',
bar: 'world'
};
p.bar = undefined;
say(p);
But this is valid _and intended_?:
const p = {
foo: 'hello',
bar: 'world'
};
delete p.bar;
say(p);
That means I'd need to mark every property in every interface as optional, if I'd want nobody to accidentally delete a required property. Isn't that more of a bug instead of an intended solution?
Note that this works, too. But it is probably also a bug.
const p1 = {
foo: 'hello',
bar: 'world'
};
const p2 = {
...p1,
bar: undefined
};
say(p2);
@mhegazy you closed the issue, so this works as intended? Looking at my examples from three months ago this still looks wrong to me.
The last case is tracked by https://github.com/Microsoft/TypeScript/issues/13195.
for this case, with --strictNullChecks
:
const p = {
foo: 'hello',
bar: 'world'
};
delete p.bar;
say(p);
I do not think control flow analysis is the right tool here, but we could disallow deleting a property whose type does not include undefined
; treeing it the same way as p.bar = undefined;
.
Thank you!
I think treating delete p.bar
as p.bar = undefined
is the right approach since they both do the same thing.
I think treating
delete p.bar
asp.bar = undefined
is the right approach since they both do the same thing.
No, they have different semantics.
var x = {}
'y' in x // false
x.y = undefined
'y' in x // true
delete x.y
'y' in x // false
TypeScript currently lets you delete properties from objects even if they are non-optional, and even if you have every kind of option on like --strictNullChecks
, --noImplicitAny
, or whatever.
interface Foo {
y: number
}
const x: Foo = { y: 3 }
delete x.y
// Now x is no longer a 'Foo', with no warnings or errors.
I'd class that as a bug.
TypeScript currently lets you delete properties from objects even if they are non-optional, and even if you have every kind of option on like --strictNullChecks, --noImplicitAny, or whatever.
i agree. delete should no be allowed under --stricitNullChecks
if the property does not include undefined
(the type system does not make a distinction between properties that are optional and those that are possibly undefined).
The best way I found to remove a prop from the object and infer a new object type is to use es6 destruct and rest features:
interface Props {
foo: string;
bar: string;
}
const p: Props = {
foo: 'hello',
bar: 'world'
}
const say = (props: Props) => console.log(`${props.foo.length} ${props.bar.length}`);
say(p);
const { bar, ...newP } = p;
say(newP);
But I agree delete on a non optional property should be forbidden.
This issue still appears at 3.2.4 version. Any chances to get it fixed atm?
I ran into this today and was surprised:
const myObject = {
myProperty: 'content',
myOtherProperty: 'other content',
};
delete myObject.myProperty;
myObject.myProperty.padStart(2); // => Runtime error
TypeScript currently lets you delete properties from objects even if they are non-optional, and even if you have every kind of option on like --strictNullChecks, --noImplicitAny, or whatever.
This behavior surprised me, too.
Another example:
interface IMyObject {
value1: number;
}
const myObject: IMyObject = {
value1: 5
}
delete myObject.value1;
console.log(myObject.value1); // undefined
console.log(myObject.value1 === undefined); // true
myObject.value1 = undefined; // Type 'undefined' is not assignable to type 'number'.
http://www.typescriptlang.org/play/index.html#code/JYOwLgpgTgZghgYwgAgJIFkCeB5ARgKwgTGQG8AoZZANzgBsBXCARgC5kQGBbXaAbnIBfcuQQB7EAGcSXHASJh2GOYWLIAvGUo16TNsgCsQkQBMIdCJGSy8qsADpajFgNETJYi-bpiA5gAobeWJHXRYASj5kAHpo5AYQMxhQCBM3KU8Ibz9AlQVQ52YNdU0EpJSTSJi4sCgmciC7Ar0NeMSIZJBUqNjkABVMAAcUAHIyjoqR5GBJDjESOElJYF8QOFwLZDAxLaHRzh5oEfsgA
I'm really surprised this bug hasn't been picked up in over 2 years.
@ryanberckmans what feedback do you need for this issue?
+1 I didn't want to say just +1 but, I definitely think the only thing I can contribute to this is +1.
I have already given it a thumbs up, but I have very little confidence that thumbs upping the OP does anything in the eyes of anyone in charge. Hence my extremely long explanation of why I am making a +1 post.
So sad that typescript gives you a false sense of security.
Given this:
type SomeType = { a: string };
const someObject: SomeType = {
a: string;
}
This code
delete someObject.a;
should be treated the same as casting someObject
to Omit<SomeType, 'a'>
and assigning to a variable of SomeType
So sad that typescript gives you a false sense of security.
Given this:type SomeType = { a: string }; const someObject: SomeType = { a: string; }
This code
delete someObject.someProperty;
should be treated the same as casting
someObject
toOmit<SomeType, 'a'>
and assigning to a variable ofSomeType
That rule would then fail with
delete someObject.a;
someObject.a= 'a'; // someProperty does not exist on Omit<SomeTime, 'a'>;
That rule would then fail with
delete someObject.a;
someObject.a= 'a'; // someProperty does not exist on Omit;
Nope, someObject
type shouldn't be changed as a result of the delete
operation. Type casting should only be used to validate the delete operation. I.e. whether someObject
would be valid if we assign it to an object without a
property.
So if a
is not optional:
delete someObject.a; // Error: deleting non-optional property `a`
// (e.g. can't assign an object of type
// Omit<SomeType, 'a'> to `SomeType`)
someObject.a= 'a'; // `a` can be safely re-assigned, so this operation is valid
If a
is optional:
delete someObject.a; // Success: deleting optional property `a`.
// Omit<SomeType, 'a'> is essentially the same as `SomeType`
// if a is optional
someObject.a= 'a'; // `a` can be safely re-assigned, so this operation is valid
If the type is not specified:
const someObject = {};
someObject.a = ''; // Property 'a' does not exist on type '{}'.
So, delete someObject.a
boils down to:
const discardedVar: SomeType = someObject as Omit<SomeType, 'a'>;
Which would succeed if a
exists in SomeType and is optional. And would fail if a
doesn't exist in SomeType
or is not optional.
So, if this is an implementation of the delete type checking:
function checkDeleteExpression(node: DeleteExpression): Type {
checkExpression(node.expression);
const expr = skipParentheses(node.expression);
if (!isAccessExpression(expr)) {
error(expr, Diagnostics.The_operand_of_a_delete_operator_must_be_a_property_reference);
return booleanType;
}
if (expr.kind === SyntaxKind.PropertyAccessExpression && isPrivateIdentifier((expr as PropertyAccessExpression).name)) {
error(expr, Diagnostics.The_operand_of_a_delete_operator_cannot_be_a_private_identifier);
}
const links = getNodeLinks(expr);
const symbol = getExportSymbolOfValueSymbolIfExported(links.resolvedSymbol);
if (symbol && isReadonlySymbol(symbol)) {
error(expr, Diagnostics.The_operand_of_a_delete_operator_cannot_be_a_read_only_property);
}
return booleanType;
}
The proper delete validation should be something like this:
...
const symbol = getExportSymbolOfValueSymbolIfExported(links.resolvedSymbol);
if (symbol && isReadonlySymbol(symbol)) {
error(expr, Diagnostics.The_operand_of_a_delete_operator_cannot_be_a_read_only_property);
}
if (symbol && !isOptionalSymbol(symbol)) {
error(expr, Diagnostics.The_operand_of_a_delete_operator_cannot_be_a_non_optional_property);
}
Where isOptionalSymbol
should be similar to isReadonlySymbol
. Although, optional is probably a type inference property, not a syntax flag?
Well, we should probably try implementing some ESLint rule first using the discarded variable casting approach above. But I'm not sure whether typescript exposes required API. And where to even start.
I am probably in the minority here but I think that delete object.a;
should be allowed _but_ it should be invalid to pass/assign object
as FooType unless you add back that required a
property.
i.e. it should throw on the next bar(object);
or const qux: FooType = object;
not on the delete line which should be considered a transitionary state
AFAIK, TypeScript doesn't have a notion transitionary state. I.e. there's no Schrodinger type.
A variable value never changes type. Types are immutable.
You can't assign a field a type that is not in the list of allowed types. You can only "remove" types via typeguards but even typeguards don't change the type of a variable. It's still the same type when you pass an object to a function.
It makes sense that types are immutable and don't change in the course of the program. They're checked statically, during the build. That's the reason why we have unions: we need to specify all the types a give variable can be of. Because can't add them. We can only reduce (typeguard) them. But even then we can't assign an existing variable a new type. We can only create a new variable with new type.
The delete
operator doesn't create a new variable. It's more like a function call on existing reference. Reference doesn't change its value. The value of reference is static and can't be changed.
Of course you can use typecasting, but that would be cheating.
It's more like a function call on existing reference.
I disagree with your analogy.
Until the object is used to do something that _actually_ needs to re-evaluate the type of that object, nothing is actually invalid in my book.
@Mouvedia ok, what would be a type of a deleted property in this case, when passing it to console.log
? At which line a type error should be shown?
const printNumber = (n: number) => console.log(n.toFixed());
interface ANumber {
a: number;
}
const obj: ANumber = { a: 42 };
delete obj.a;
printNumber(obj.a);
If I was using an IDE Id expect a wavy line under the argument of printNumber
l8.
Not at l7 where the deletion is operated.
Because this
// snip
const obj: ANumber = { a: 42 };
delete obj.a;
obj.a = 42;
printNumber(obj.a);
should be allowed.
If such property isn't optional I don't think you should be able to delete it, if you need to change the value just assign the new value. Delete is "the same" of assign undefined to the property.
This could be easily fixed like this:
type AnOptionalNumber { a?: number; }
const obj: ANumber = { a: 42 };
delete (obj as AnOptionalNumber).a;
obj.a = 42;
printNumber(obj.a);
That is if you know what you're doing you're welcome to shoot yourself in the foot. But by default, TypeScript should aim to prevent runtime errors, not tolerate some temporary untyped state of a variable.
Deleting a property from an object is not exactly the same as setting undefined
. Deleted properties are no longer enumerable.
interface ExampleType {
foo: number,
bar: number | undefined
}
const obj: ExampleType = { foo: 1, bar: 2 };
console.log(Object.keys(obj)); // [ 'foo', 'bar' ]
obj.bar = undefined; // This is allowed
console.log(Object.keys(obj)); // [ 'foo', 'bar' ]
delete obj.bar; // This should not be allowed as `bar` won't be enumerable
console.log(Object.keys(obj)); // [ 'foo' ]
Most helpful comment
for this case, with
--strictNullChecks
:I do not think control flow analysis is the right tool here, but we could disallow deleting a property whose type does not include
undefined
; treeing it the same way asp.bar = undefined;
.