Skip to content
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

Defer conditional types with parenthesized multi-element tuple types in extends clause #56271

Conversation

Andarist
Copy link
Contributor

fixes #56270

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Oct 31, 2023
src/compiler/checker.ts Outdated Show resolved Hide resolved
@ahejlsberg
Copy link
Member

As per my comments here I don't think we want this to change.

@Andarist Andarist closed this Oct 31, 2023
@Andarist
Copy link
Contributor Author

Reopening on @jakebailey’s request

@Andarist Andarist reopened this Oct 31, 2023
@jakebailey
Copy link
Member

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Nov 1, 2023

Heya @jakebailey, I've started to run the tarball bundle task on this PR at ec113c0. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Nov 1, 2023

Hey @jakebailey, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/158427/artifacts?artifactName=tgz&fileId=3F0CBC6645E4050DB9EB24473D159E55AE9EC546D964A9872D9BE8BC0677FB1802&fileName=/typescript-5.3.0-insiders.20231101.tgz"
    }
}

and then running npm install.


There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@5.3.0-pr-56271-4".;

@jakebailey jakebailey merged commit 41ec497 into microsoft:main Dec 8, 2023
19 checks passed
@craigphicks
Copy link

If we were resolving from the inside out, rather than the outside in, we would also cover cases like [...[<something>],...[<something>]] which may resolve to simple tuples.

@jakebailey
Copy link
Member

I'm not sure if this is what you mean, but potentially a better fix would have been to strip parens around anything passed into isSimpleTupleType and any node it examines.

@jakebailey
Copy link
Member

Well, that can't be the actual implementation, but almost. I'll send a PR for discussion as the parens inside are still significant (and my intention was to not have that be the case).

@jakebailey
Copy link
Member

jakebailey commented Dec 8, 2023

I am confusing myself; this doesn't matter; isSimpleTupleType looks at the elements and only cares about optional/rest/named members, which cannot be parenthesized. So maybe you meant something else?

Basically, the intent is just "parens don't affect semantics"; anything else is out of scope.

@Andarist Andarist deleted the fix/defer-conditional-types-with-parenthesized-multi-element-tuples branch December 8, 2023 22:33
@craigphicks
Copy link

craigphicks commented Dec 8, 2023

I think "out of scope" covers it. But what I meant was an inside-to-outside order processing of nodes to types. That would include stripping parens, but also more.

c0sta pushed a commit to c0sta/TypeScript that referenced this pull request Dec 20, 2023
…in `extends` clause (microsoft#56271)

Co-authored-by: Jake Bailey <5341706+jakebailey@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Adding parentheses changes the result of type inference
5 participants