-
Notifications
You must be signed in to change notification settings - Fork 12.8k
feat(51870): Add a Quick Fix to add an additional parameter to a method or function #56411
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
Conversation
The TypeScript team hasn't accepted the linked issue #51870. If you can get it accepted, this PR will have a better chance of being reviewed. |
1811cf5
to
3b31c70
Compare
I'll bring this PR up at the vscode-TS weekly meeting to decide if we want to add this. |
@sandersn Thanks |
Here are my notes from the meeting:
|
3b31c70
to
9338f6a
Compare
@sandersn Thanks for the feedback.
Are you suggesting implementing a Quick Fix to address this case? I think function f(a: number) {}
f(); // QF to remove a
Yes, the Quick Fix attempts to utilize the longest overload declaration.
Yes, I think we can attempt to use
It's a topic open for discussion. The safest route is to use optional parameters (it depends on the arg position), however, I think if a user calls the function with a new argument, there's an expectation that they want this parameter to be explicitly declared in the signature.
Yes.
Need to check, I think there's a utility to handle that.
Could you clarify this question? |
Here's an example for the last question: function f(x: number, y: number, unused?: number) {
return x + y
}
f(1, 2)
f(1, 2, 3)
In summary, there's nothing to do here; the existing quick fix is smart enough already. |
Most of the other questions from the design meeting don't require changing anything, as far as I can see. The only 2 open ones are
|
Should it be provided for both overload+signature declarations and signature declarations? function f(a: string): string;
function f(a: string, b: number): string;
function f(a: string, b?: number): string {
return "";
}
f("", 1, "");
1. Add missing parameter to "f"
2. Add optional parameter to "f" function f() {}
f("");
1. Add missing parameter to "f"
2. Add optional parameter to "f"
I came across this utility in another Quick Fix. Need to check if it works correctly, especially in the fourslash context. TypeScript/src/services/codefixes/fixAddMissingMember.ts Lines 366 to 368 in 2c14a1c
|
|
@sandersn I've added |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the design. Just comments and suggestions on the code.
Fixes #51870