-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Excess property checks for destructured arguments #42419
Comments
|
I see one minor breakage: interface User {
id: string;
name: string;
email: string;
}
function stringifyUser({ name, email }: User): string;
// `id` should be removed.
stringifyUser({ id: '1', name: 'Developer', email: '[email protected]' })
// This is still fine though
stringifyUser({ id: '1', name: 'Developer', email: '[email protected]' } as User) @MartinJohns what else would this break? |
I didn't think my comment through. I think a better explanation of what you want would be "excess property checks for destructured arguments". |
You're proposing that something currently OK to be made an error instead, which is sort of the definition of breaking change. Parameter destructuring is considered to be an implementation detail, not an externally-visible facet of the function signature, so we don't modify the callee's view of the call based on whether or not the function uses destructuring. This is desirable both from a predictability perspective, and because if you really did want someone to pass a Using |
This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes. |
It's a "breaking change" only for things that were provably incorrect to begin with. π€ If we explicitly wrote out the type-hint, like so: interface User {
id: string;
name: string;
email: string;
}
function stringifyUser({ name, email }: Pick<User, "name" | "email">): string {
return `${name} (${email})`
} What you've declared here, is that the function only depends on a subset of properties - which is exactly what you're declaring when you destructure. Passing unused extra properties should be an error: stringifyUser({ id: '1', name: 'Developer', email: '[email protected]' }) Why would the function demand values it can't access? It's only a breaking change for cases where you are provably passing things you don't need, and can't actually access at all. In any other case, such as when a compatible object originates from anywhere else than the call site, it's fine:
Even in strict mode, this is fine.
But that's the intention. Why would you force someone to pass a As I wrote in #48220:
The way I see it, asking for things you can't use or access is bad practice. When But there's no guarantee either way - an If we wanted to get around that problem, we would need nominal types, one of the earliest feature requests in the history of TypeScript. Short of that, type-checks in TS are structural - everything is "duck typed", so, in my opinion, this proposal perfectly aligns with the philosophy of being able to call a function with whatever the function actually needs, without having to "prove" anything. In the example used here, passing a literal Plenty of changes have been introduced over the years that made certain code "invalid" according to new and stricter standards - if improved strictness helps identify dead or incorrect code, as it does in the example here, those aren't "breaking changes", are they? If those are breaking changes, those are the kind of breaking changes I want. π Can you point to anything else about this feature that would point to an real problem? Please give this issue more careful consideration. π |
What we have now is the desired behavior, and that we consider the proposed alternative to be extremely confusing at best. If you want the other behavior, it's there with It fails several major predictability tests: It's generally accepted that constructs should work like their desugared equivalents, which this definitely does not. Everywhere else in TypeScript, a type annotation is the "last word" in terms of typing, and this would be a special case for that. It's a major refactoring hazard, because code using destructuring can't opt back in to using properties on the declared type later without incurring the risk that callers of the function written in the meantime didn't provide those properties. If we did this, it would quickly become accepted guidance to never use destructuring in TypeScript because it would create such unpredictable effects on checking. |
This sounds a bit hypothetical.
Who would need to opt back into dependencies on properties they don't actually depend on and why?
The way I see it, destructuring says: "my function depends on an object with these properties" - with the type annotation, you're saying "the destructured properties use types from this object type". This would be exactly equivalent to the what destructuring actually means and does. (I don't know what you mean by "desugared equivalents"? We're talking about destructuring, which is a language feature and not sugar - and types, which have no run-time footprint. What "desugars"?)
So it's better for functions to require a lot of values now, in case you need them in the future? I just don't follow. That's just not the way I do programming. If something is not a dependency, I do my best not to make it a dependency. Introducing new dependencies to a function is and should be a breaking change. If you see this in plain JS:
Are you going to start passing In JS, destructuring is the closest analog we have to named arguments - adding a new argument is a breaking change. In JS, destructuring is also the closest thing we have to typed arguments - I don't know why TS should demand more of a type than JS itself demands. More precision in what it demands, yes - but demanding properties that the underlying JS code doesn't ask for? That doesn't seem to add precision. If anything, it's less accurate than vanilla JS on this point.
In a language with structural type-checking, these effects are completely predictable. We're talking about making the type-system accurately reflect what JS does when you destructure arguments. I think this comes back to the nominal vs structural issue. You've chosen structural type-checking (for which I love TS) but it seems like you're using the lack of a nominal type-checking option as an argument to keep types wider than necessary? In my opinion, this provides, at best, a false sense of security. (the I'm not trying to say there aren't cases where those concerns are valid - there most definitely is. But wider than necessary type checking doesn't solve that - or if and when it does, that's just lucky coincidence. The reality with structural type-checking is it's predicated on having "probabilities of correctness" - as opposed to "guarantees", which only a nominal type designation (or perhaps nominal type-hints) could address that. Can you provide an example of where and how this change would be harmful in practice? My hope is that either you realize while trying to do so that your concerns are imagined - or that you prove me wrong. π I would be happy with either outcome. I just haven't seen anything yet that points to a practical problem with this idea - what was initially described as "breakage" just looks to me like bad code getting caught by better type-checking, which is why I use TS. π |
Sure: function isEmptyArray({ length }: unknown[]) {
return length === 0;
}
// No error??? TypeScript is broken?
isEmptyArray("hello world"); You could also imagine something like this: // Duration for for some number of milliseconds
type Millisecond = { units: "ms", length: number };
// Duration for for some number of seconds
type Second = { unit: "s", length: number };
const oneSecond: Second = { unit: "s", length: 1 };
// Why this only sleeps for 1ms? TypeScript bug?
sleepFor(oneSecond);
function sleepFor({ length }: Millisecond) {
Thread.sleep(length);
} etc. It's just not the case that every property stands alone as a singularly interpretable thing. Often, other properties are critical to the interpretation of data, whether that be about format, meaning, or context. |
If you destructure every property into a free standing variable, that implies each property can stand alone as "a singularly interpretable thing". It literally is.
Isn't that the trade-off you make for the convenience of structural type-checking though? Essentially, every type-check is probabilistic. You've showed two examples essentially with code that hopes for a stricter type-check than what's actually required to implement a working function - and surely this has better odds of achieving type-safety, but both of those examples will produce what I think is a misleading kind of safety:
You want the the type-system to check for assumptions or implications about certain types - you're essentially trying or hoping for some way to verify the identity of the type, but what you're getting is still only some degree of likelihood and not a guarantee. You're never really going to get that with structural type-checking - only nominal type-checks truly give you that. To be clear, I do see the practical problems you're pointing out with this change. I don't agree that it's a breaking change, because your code would still work - some of your type-checks wouldn't be as narrow as you'd have liked, but nothing would break. If this feature were implemented, there would need to be an opt-in to check for unused properties in those cases where you were hoping for more type-checking than what is actually required by the function implementation - maybe something like As argued, this is not really safe or explicit either though. In many cases, a type like nominal type Millisecond = { unit: "ms", length: number };
// π structural compatibility is insufficient for this type
const time = {
unit: "ms",
length: 1,
weekday: "monday", // π some unrelated data in this inferred type
};
sleepFor(time); // untyped structure - could be Millisecond or just happens to be compatible π
sleepFor(time as Millisecond); // type cast required to indicate the expected interpretation π I think, at the end of the day, this kind of discussion comes down to opinions. There are those of us who would have wished for soundness - and maybe it seems odd for me, being one of those people, arguing for less strictness in this case. But soundness is not only about being strict, it's also about being accurate - it's hard to argue the inference is accurate here, and it definitely gets in the way of writing tests. There are practical reason why I've spent time debating this issue. I do think maybe this feature is at an impasse - with so much code already in the wild, destructuring out of convenience, rather than with any real consideration for whether each property is a "singularly interpretable thing", maybe it is too late to fix this without causing too much upset... (Fingers crossed for a |
This is the root of the question-begging: You're making the claim that if a property gets destructured, then it has context-free meaning, therefore a function using destructuring should have different semantics. But it presupposes the thing being debated - that destructuring is a semantic differentiator rather than a syntactic convenience. |
Suggestion
π Search Terms
List of keywords you searched for before creating this issue. Write them down here so that others can find this suggestion more easily and help provide feedback.
pick destructured
β Viability Checklist
My suggestion meets these guidelines:
β Suggestion
If a function accepts destructured parameters, the type parameter can be treated as if itβs a
Pick<>
ed version of the type annotation.When destructuring an argument in a function declaration, TypeScript can statically determine any unused properties can be omitted safely.
For example, the following two examples would be equivalent:
Of course itβs already possible to combine destructured properties with
Pick<>
, but having to synchronize this manually is cumbersome and error prone.π Motivating Example
TypeScript now automatically picks destructured properties from a call signature argument.
For example:
π» Use Cases
A function signature may adhere to an interface where some properties exist. Using this interface as a type parameter would be the logical thing to do. However, there may be use cases where itβs desirable to call the function with only the parameters that are actually used. The first use case that comes to mind is when writing tests.
The text was updated successfully, but these errors were encountered: