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

Allow return type in conditional if body is followed by a colon #48788

Closed
wants to merge 5 commits into from

Conversation

jakebailey
Copy link
Member

This modifies my logic in #47550 allow an arrow function with a return type so long as that arrow function is immediately followed by another colon, which is instead chosen to terminate the conditional expression.

Fixes #48733

@typescript-bot typescript-bot added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels Apr 20, 2022
@jakebailey
Copy link
Member Author

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 20, 2022

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 20, 2022

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/124798/artifacts?artifactName=tgz&fileId=12EB9E2D14C16ED79A119CECACDDB29A6E0083AEC272E7D2968EE4340320600D02&fileName=/typescript-4.7.0-insiders.20220420.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@4.7.0-pr-48788-2".;

@jakebailey
Copy link
Member Author

Demo: Playground Link

@sandersn
Copy link
Member

sandersn commented May 4, 2022

This change is intended for 4.8, right?
At least, I thought we decided to revert the original change at a design meeting (or you recommended and we've been waiting to talk about it there).

@jakebailey
Copy link
Member Author

Yes, 4.8, given we took too long to make a decision. There's a revert PR for the original fix I made (#48940) and I'll rebase these three PRs to make them correct again.

Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

This looks like it does what it says, although I recall that we wanted to parse JS in a spec-compliant way. Does this need a follow-up PR for that?

@jakebailey
Copy link
Member Author

Sorry, yes, I don't have the final fix yet. Likely this PR does what we want, but I need to really check.

@jakebailey jakebailey marked this pull request as draft May 9, 2022 22:00
@jakebailey jakebailey closed this Jun 13, 2022
@jakebailey jakebailey deleted the fix-48733 branch October 20, 2022 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[4.7-beta] Parsing failure for arrow function expr in conditional expr
3 participants