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

TypeScript Constant contexts #9534

Merged

Conversation

tanhauhau
Copy link
Member

Q                       A
Fixed Issues? Fixes #9502
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

@tanhauhau tanhauhau changed the title WIP: Add as const fixtures WIP: TypeScript Constant contexts Feb 18, 2019
@babel-bot
Copy link
Collaborator

babel-bot commented Feb 19, 2019

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/10453/

@danez
Copy link
Member

danez commented Feb 22, 2019

Looks very promising, let us know when you are done.

@danez danez added PR: WIP pkg: parser area: typescript PR: Spec Compliance 👓 A type of pull request used for our changelog categories labels Feb 22, 2019
@tanhauhau tanhauhau changed the title WIP: TypeScript Constant contexts TypeScript Constant contexts Feb 22, 2019
@tanhauhau
Copy link
Member Author

@danez it's ready for code review 😄

case "UnaryExpression":
return this.tsCheckLiteralForConstantContext(node.argument);
default:
throw this.unexpected(node.start);
Copy link
Member

Choose a reason for hiding this comment

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

We can throw better error messages 😉

Suggested change
throw this.unexpected(node.start);
this.raise(node.start, "Only literal values are allowed in constant contexts");

case "NumericLiteral":
case "BooleanLiteral":
case "ObjectExpression":
case "ArrayExpression":
Copy link
Member

Choose a reason for hiding this comment

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

For arrays and objects, don't we need to check their children?

e.g.

[ foo() ] as const

Copy link
Member Author

Choose a reason for hiding this comment

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

updated and added new test case

// but need `tsInType` to satisfy the assertion in `tsParseType`.
node.typeAnnotation = this.tsInType(() => this.tsParseType());
const _const = this.tsTryNextParseConstantContext();
node.typeAnnotation = _const ? _const : this.tsNextThenParseType();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
node.typeAnnotation = _const ? _const : this.tsNextThenParseType();
node.typeAnnotation = _const || this.tsNextThenParseType();

@tanhauhau tanhauhau force-pushed the tanhauhau/parser-typescript-as-const branch from cc8eb28 to 76f1c61 Compare February 25, 2019 10:09
@nicolo-ribaudo nicolo-ribaudo added this to the v7.5.0 milestone Mar 10, 2019
let o5 = { ...o4 } as const;
let o6 = { ...o5 };
let o7 = { ...d } as const;
let o8 = { ...o7 };
Copy link
Member

Choose a reason for hiding this comment

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

What are these tests without as const needed for? I think that you copied them from https://github.com/Microsoft/TypeScript/blob/08fe06f5275bae91a2b2d7eb6387218b95ce3566/tests/baselines/reference/constAssertions.js, but since Babel doesn't type-check they aren't needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated the test cases

@nicolo-ribaudo
Copy link
Member

Some tests are failing on travis/circle because of regenerator, but they will work if you rebase this PR on master.

@tanhauhau tanhauhau force-pushed the tanhauhau/parser-typescript-as-const branch from 76f1c61 to 735d539 Compare March 11, 2019 02:18
@tanhauhau
Copy link
Member Author

@nicolo-ribaudo cool.

@nicolo-ribaudo
Copy link
Member

You also need to update the output 😛

@tanhauhau tanhauhau force-pushed the tanhauhau/parser-typescript-as-const branch from 3acf446 to fcb060f Compare March 11, 2019 15:30
@nicolo-ribaudo nicolo-ribaudo added PR: Ready to be Merged A pull request with already two approvals, but waiting for the next minor release spec-update labels Mar 12, 2019
@nicolo-ribaudo nicolo-ribaudo modified the milestones: v7.5.0, v7.4.0 Mar 12, 2019
@nicolo-ribaudo nicolo-ribaudo merged commit 25a3825 into babel:master Mar 16, 2019
@tanhauhau tanhauhau deleted the tanhauhau/parser-typescript-as-const branch April 18, 2019 00:34
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 4, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: typescript outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: parser PR: Ready to be Merged A pull request with already two approvals, but waiting for the next minor release PR: Spec Compliance 👓 A type of pull request used for our changelog categories spec-update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeScript 3.4 meta-issue
6 participants