Skip to content

Fixed an issue with spreading generic types with tuple constraints into calls #53615

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Andarist
Copy link
Contributor

fixes #53541

@typescript-bot typescript-bot added the For Backlog Bug PRs that fix a backlog bug label Mar 31, 2023
else if (constraint && isTupleType(constraint)) {
forEach(getTypeArguments(constraint), (_, i) => {
const isVariable = !!(constraint.target.elementFlags[i] & ElementFlags.Variable);
const syntheticArg = createSyntheticExpression(arg, !isVariable ? getTypeOfPropertyOfType(spreadType, "" + i as __String)! : spreadType,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this won't help with variable elements at all - there is no way to express a "deferred" type that would later skip some elements from it so a variable element gets synthesized as the whole spreadType. It isn't ideal but it also isn't worse than the current baseline.

An example code that ideally should work but it doesn't:

interface HasMethod {
  method(second?: number, ...rest: boolean[]): void;
  method2(first?: string, second?: number, ...rest: boolean[]): void;
}

function fn<HasMethodLike extends HasMethod>(
  instance: HasMethodLike,
  ...args: Parameters<HasMethodLike["method"]>
) {
  instance.method2('', ...args);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The best idea I have for fixing that is to add more information to synthetic expressions (like .skipElementCount) and then use that information in checkSyntheticExpression. This is how currently checkSyntheticExpression looks like:

function checkSyntheticExpression(node: SyntheticExpression): Type {
    return node.isSpread ? getIndexedAccessType(node.type, numberType) : node.type;
}

@Andarist Andarist marked this pull request as draft March 31, 2023 22:26
@Andarist Andarist marked this pull request as ready for review April 1, 2023 12:46
@@ -22,7 +22,7 @@ f1(42, ...t2);
f1(42, "hello", ...t1);
f1(42, "hello", true, ...t0);
f1(ns[0], ns[1], true);
f1(...ns, true); // FIXME: Error, since ...ns is considered as string|number here
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

many of those comments in this file were simply outdated so I updated them

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Backlog Bug PRs that fix a backlog bug
Projects
Status: Waiting on reviewers
Development

Successfully merging this pull request may close these issues.

Using Parameters in a class member function gets incorrect result while all parameters are optional.
3 participants