-
Notifications
You must be signed in to change notification settings - Fork 12.8k
add new compiler option #54441
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
add new compiler option #54441
Conversation
veryStirctArity by default false when turn on more strictly check arity of methods
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
@microsoft-github-policy-service agree |
The issue was closed for a reason. It's not a bug. It does not need "fixing." |
Well you are right it is not a bug it is improvement ;-) |
Code bellow with produce errors with new flag function mymap<T, U>(arr: T[], callback: (value: T, index: number, array: T[]) => U): U[] {
const result: U[] = [];
for (let i = 0; i < arr.length; i++) {
result.push(callback(arr[i], i, arr));
}
return result;
}
const numbers = [1, 2, 3, 4, 5];
const doubled = mymap(numbers, (value) => value * 2); // error with new flag veryStrictArity
console.log(doubled); // Output: [2, 4, 6, 8, 10]
const squared = mymap(numbers, (value) => value * value); // error with new flag veryStrictArity
console.log(squared); // Output: [1, 4, 9, 16, 25] |
but if we define function more strict there will be no error in code bellow function mymap<T, U>(arr: T[], callback: (value: T, index?: number, array?: T[]) => U): U[] {
const result: U[] = [];
for (let i = 0; i < arr.length; i++) {
result.push(callback(arr[i], i, arr));
}
return result;
}
const numbers = [1, 2, 3, 4, 5];
const doubled = mymap(numbers, (value) => value * 2); // no error with new flag veryStrictArity
console.log(doubled); // Output: [2, 4, 6, 8, 10]
const squared = mymap(numbers, (value) => value * value); // no error with new flag veryStrictArity
console.log(squared); // Output: [1, 4, 9, 16, 25] |
How is this an improvement? Your type states a parameter could be undefined even though it never can be, and if I want to use the index, I have to do unnecessary optional chaining, undefined check, or non null assertion. |
You can see in my example above that you do not need to do any of this if function is defined more strict |
with new flag veryStrictArity function handler(arg: string) {
// ....
}
function doSomething(callback: (arg1: string, arg2: number) => void) {
callback('hello', 42);
}
// Expected error because 'doSomething' wants a callback of
// 2 parameters, but 'handler' only accepts 1
doSomething(handler); |
error TS2345: Argument of type '(arg: string) => void' is not assignable to parameter of type '(arg1: string, arg2: number) => void'. 11 doSomething(handler); |
with new flag veryStrictArity let items = [1, 2, 3];
items.forEach(arg => console.log(arg)); // error like expected
type Callback<T> = (value: T, index?: number, array?: T[]) => void;
interface Array<T> {
myForEach(callback: Callback<T>): void;
}
Array.prototype.myForEach = function <T>(callback: Callback<T>): void {
for (let i = 0; i < this.length; i++) {
callback(this[i], i, this);
}
};
items.myForEach(arg => console.log(arg)); // no error because index and array are optional parameters |
and finally on the last point of https://github.com./Microsoft/TypeScript/wiki/FAQ#why-are-functions-with-fewer-parameters-assignable-to-functions-that-take-more-parameters |
so basically with this flag turn on |
Create new issue #54449 so we can discuss it. |
For clarity, we are unable to review pull requests not addressing approved feature work |
Dear @RyanCavanaugh, I wanted to share my thoughts regarding the closed PR. I believe it was closed rather quickly. Upon reviewing the comments, I noticed they were from 2017 and 2018. I believe this PR could address both those issues, as well as other related concerns, as you mentioned. You can find more details about it in the following GitHub issues: #17868 by @inad9300 Regarding your comment (#17868 (comment)) and a similar comment by @xiBread, I wanted to highlight that it doesn't directly affect this PR. As I explained, it depends on how the type of map, forEach, filter, or sort is defined. To illustrate this, here's an example: type SelfOrPartial<T> = T | Partial<T>;
type Callback<T> = (value: T, index: number, array: T[]) => void;
interface Array<T> {
forEach(callbackfn: SelfOrPartial<Callback<T>>): void;
}
const items = [1, 2, 3];
items.forEach(arg => console.log(arg));
items.forEach(() => console.log("just counting")); This code compiles without any issues, even when the veryStrictArity flag is activated. Perhaps it would be beneficial to seek the opinion of @ahejlsberg as well. Given his extensive experience in both OOP and functional programming, he might have a better understanding of this PR and its implications. Thank you for considering my input. |
I think you're under the impression that we just haven't considered this before. We have considered it many, many times. |
Thank you for your response. I apologize for any misunderstanding. Could you kindly explain the potential drawbacks of implementing this as an optional parameter? I have read the provided information but am still trying to fully understand the reasoning behind the decision. I mainly see the positive aspects of this PR and its potential benefits. Your clarification would be greatly appreciated. |
I wanted to bring to your attention that this PR also addresses an issue raised by @jesseschalken back in 2016 (#13043). Additionally, it seems that the problem I am currently facing is similar to the one encountered by Google, as mentioned in this comment by mbrobst (#13043 (comment)). I also came across another related issue raised by @ORESoftware (#20274). While I understand that the TypeScript team may not consider this as a high-priority matter, I believe that implementing it as an optional flag could greatly enhance TypeScript's object-oriented programming capabilities. Many other developers, including myself, see significant value in this proposed enhancement. I still find myself struggling to comprehend the underlying concerns or objections to this PR. I sincerely appreciate your time and consideration. Thank you for allowing me to express my perspective. |
What you're proposing here is that function subtyping not uniformly act like function subtyping -- that a bare function you can pass to class A implements B { /* ... */ } has different logic than const x: B = new A(); then there should be some strong theoretical justification for doing this, since that sort of check is the entire reason to have an implements clause in the first place. But there isn't a strong theoretical justification, because we know that in JavaScript, excess arguments do not create any runtime problems. We also know from inspection that these two functions do not have different correctness: // zero parameters
function x1() {
return 0;
}
// one unused parameter
function x2(n: number) {
return 0;
} So trying to say that it's correct for x2 to implement some contract, but not x1, doesn't make any sense. x1 is a subtype of x2 according to any reasonable definition of JavaScript function rules you might have. If you want to claim that class methods should have different rules than functions, that's just a weird asymmetry that we have to explain everywhere and doesn't actually provide any concrete value in terms of creating a more consistent and predictable type system. |
@RyanCavanaugh My original problem was this one So when this parameter is turn on TypeScript is more powerful I think it is good improvement.
with a new flag but if it is forEach callback redefine like this
it all depends of type definition. |
error message: 18 const some : ((arg: string) => boolean) = () => true; |
@RyanCavanaugh
Indeed if you declare function x2 as bellow with optional parameter
but if you specify veryStrictArity true if veryStrictArity is false (default option) it will still be compiled without error So I do not understand what is still problem with this PR? I see this PR similar like when your team fix null/undefined problem |
// one unused parameter
function x2(n?: number) { You have the definition of optionality backwards. This is covered in the FAQ At this point we are well into the middle of territory that has been trod many times over in past discussions, and I really ask that you read them all comprehensively before making points that have been covered before. I am not having a 12th iteration of a conversation about what the definition of parameter optionality is. |
So, in fact, there are dual meanings of the optional parameter: one when describing functions and another when describing callbacks. I personally find this confusing, and that's why I proposed this PR to address the issue. Allow me to share some comments from the FAQ: function handler(arg: string) {
// ....
}
function doSomething(callback: (arg1: string, arg2: number) => void) {
callback('hello', 42);
}
// Expected error because 'doSomething' wants a callback of
// 2 parameters, but 'handler' only accepts 1
doSomething(handler); I expected the same behavior, and with this PR, one could enable a flag to receive a warning. If the intention was to avoid the warning, the function should be defined like this: function doSomething(callback: (arg1: string, arg2?: number) => void) {
callback('hello', 42);
} Regarding // Invoke the provided function with 0 or 1 argument
function maybeCallWithArg(callback: (x?: number) => void) {
if (Math.random() > 0.5) {
callback();
} else {
callback(42);
}
} The above code compiles with the In the FAQ, there is also a statement that says: "There is currently not a way in TypeScript to indicate that a callback parameter must be present. Note that this sort of enforcement wouldn't ever directly fix a bug." However, with the Thank you for your time! I won't bother you any longer. I will use my fork with the new flag and publish it on npm. This will allow me to catch some errors at compile time, which is not possible with the current version. Once again, thank you for your assistance! |
In reference to this comment, type JavaScriptCallback<
T extends (...args: any) => any,
P = Parameters<T>
> = P extends [...infer Rest, infer _Last]
? ((...args: Rest) => ReturnType<T>) | JavaScriptCallback<T, Rest>
: T;
interface Array<T> {
map<U>(
callbackfn: JavaScriptCallback<(value: T, index: number, array: T[]) => U>,
thisArg?: any
): U[];
}
or not to use veryStrictArity option at all |
I would like to respectfully voice a different perspective in response to your comment here. I've actually published a fork of TypeScript available here, which has this feature permanently enabled. I've found it to be greatly beneficial in my work. In addition, I recently stumbled upon an older blog post here and the ensuing discussion here. Notably, the code snippet I want to take this opportunity to express my gratitude for your help and insights on this matter! One of the wonderful aspects of open-source projects is the ability to fork and publish if the original version doesn't entirely meet one's requirements. While my preference would have been for my PR's to be accepted, I'm content with using my own version until a more satisfactory solution that addresses this issue is introduced. |
No, it always means the same thing - the caller of that function may omit the argument (which means you, as the person implementing it, have to deal with the possibility of the value being |
@fatcerberus Thank you for clarification, however it is still contra intuitive to me, |
By analogy—you’re basically arguing that I should always walk up to you with my back turned just so that my right hand appears on your right, which is not at all a logical request. |
IMHO bad analogy |
implement new optional type checking flag veryStirctArity
by default false
when turn on more strictly check arity of methods
Actually add opportunity to fix https://github.com./microsoft/TypeScript/wiki/FAQ#why-are-functions-with-fewer-parameters-assignable-to-functions-that-take-more-parameters
so if new compilations flag is applied we would get error when try to assign function with fewer parameters to
function that take more parameters
This will help to catch some errors during compile time like explained in playground and in
comments bellow
Fixes #
#46881
although it is closed , maybe will this PR reconsider issue
First issue I open as a bug but probably should be improvement
I found this very useful improvement
playground link
https://www.typescriptlang.org/play?ts=5.1.0-beta#code/JYOwLgpgTgZghgYwgAgJLIN4FgBQzkAWwAFHAFzIDOYUoA5gDTIBGF1tIdAlBQG4D2wACYBuXAF9cuBABs4lSsgCCyYAFsADjIhqI4Remw4peQiXJUa9HsgHDMyAPSPkAOQDyyAKIAlH+59kAAlfL2QAd2AwAmQQCHDkGDk6ZGjoFGBFX38fXHx8aKh+BLiErygiqGIAIgBZCGj+IVj+MFVNbV1wCCEAOmrkAGpkOC4xU0ljHEnpOQVkACF2rR09MANMEzyzUjYrTiZWSw5GZAQ9k5s7ZqN81IIikvjvCv4quoaCJpa29RWuyB9AbDOBDFhjbYzaZbHBwZjsRBtWTyRRKEDqOAyTamOEIhBtNRwADWEAAyvwAK4gITESiU6kXax8QSiEz4NT8XgQYhXFnYu4IfggOnaXoyfh0Go+fhwNT0e4oCBwKDRXpq6oQiYSEzI+YAEQlyAgAA9INTUejCVjbshCSTyVSaVwHM43J5sgFgqEIlEYqVEskFVAMlk-AFtvhBcL+KLxZLqgB1fj8GAAQmQSZTqY143wkyhUeoyCEhoAvLFngbJZqS3RenayfSnSInC5M2mM8m07ha-XOdyxq3kNLZfK0kblaq1SZcTREWc5ooAFrAARgUAAK1BNtnUHnDYd1NpTYA-IzOLzhONthyuTzmfYbZGhSKIGKJVKZXLOAqJyqCGq-Sanm2pTLqigAAryEapp6EIy6rq0m6gvytrEo2jo8i6LgeN4YaBCEPhhJE0QVgkSRwCkzAQAgcAUpQKB0o6qiKPwGjriAmIRmcL4xm+cY1O26ZCTmkKgbgQA