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 4.0: Support labeled tuple elements #11754

Merged

Conversation

@nicolo-ribaudo
Copy link
Member

nicolo-ribaudo commented Jun 27, 2020

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

https://devblogs.microsoft.com/typescript/announcing-typescript-4-0-beta/#labeled-tuple-elements

@codesandbox
Copy link

codesandbox bot commented Jun 27, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@@ -76,6 +76,8 @@ const TSErrors = Object.freeze({
IndexSignatureHasStatic: "Index signatures cannot have the 'static' modifier",
OptionalTypeBeforeRequired:
"A required element cannot follow an optional element.",
LabeledElementBeforeUnlabeled:
"An unlabeled element cannot follow a labeled element.",

This comment has been minimized.

@nicolo-ribaudo

nicolo-ribaudo Jun 27, 2020 Author Member

TypeScript throws Tuple members must all have names or all not have names. However, it accepts this code:

type B = [A, x: C];

Playground Link

@weswigham / @DanielRosenwasser Which is the correct behavior?

This comment has been minimized.

@weswigham

weswigham Jun 27, 2020

Uhh.. we shouldn't accept that (but it is just a grammar error, not a parse error). Care to open an issue?

@babel-bot
Copy link
Collaborator

babel-bot commented Jun 27, 2020

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

@nicolo-ribaudo nicolo-ribaudo changed the base branch from main to feat-7.11.0/ts-4.0 Jun 29, 2020
@nicolo-ribaudo nicolo-ribaudo force-pushed the nicolo-ribaudo:ts-4.0/labeled-tuple-elements branch from ce28128 to 016add4 Jun 29, 2020
@nicolo-ribaudo nicolo-ribaudo mentioned this pull request Jun 29, 2020
4 of 4 tasks complete
Copy link
Contributor

JLHwung left a comment

Can you add a test case on named rest tuple members

type T = [x: A, ...y: B]

and also an invalid case on optional rest named tuple members

type T = [x: A, ...y?: B]

on which tsc throws

A tuple member cannot be both optional and rest.(5085)

@nicolo-ribaudo
Copy link
Member Author

nicolo-ribaudo commented Jun 30, 2020

Oh I completely missed rest labeled elements.

@nicolo-ribaudo
Copy link
Member Author

nicolo-ribaudo commented Jun 30, 2020

Should the AST be a TSNamedTupleMember inside a TSRestElement, or the other way around?

@JLHwung
Copy link
Contributor

JLHwung commented Jun 30, 2020

Should the AST be a TSNamedTupleMember inside a TSRestElement, or the other way around?

extend interface TSRestType {
  typeAnnotation: TSType | TSNamedTupleMember;
}

looks good to me, and it is consistent with how we model type A = [...B] by placing B in the typeAnnotation.

elementTypes: $ReadOnlyArray<TsType | TsTupleElementType>,
};

export type TsNamedTupleMember = TsTypeBase & {

This comment has been minimized.

@JLHwung

JLHwung Jun 30, 2020 Contributor

Since it is a new node type, we should also add support on @babel/generator.

@nicolo-ribaudo nicolo-ribaudo requested a review from JLHwung Jul 5, 2020

const isLabeled = type === "TSNamedTupleMember";
// Flow doesn't support ??=
labeledElements = labeledElements ?? isLabeled;

This comment has been minimized.

@JLHwung

JLHwung Jul 10, 2020 Contributor

Suggested change
labeledElements = labeledElements ?? isLabeled;
labeledElements ?? (labeledElements = isLabeled);

This comment has been minimized.

@nicolo-ribaudo

nicolo-ribaudo Jul 10, 2020 Author Member

Even if it's one operation more, I find the = ... ?? more readable

This comment has been minimized.

@JLHwung

JLHwung Jul 10, 2020 Contributor

😂 But that's not the intended semantics of ||=. They are just nitpicks, feel free to ignore them if current code looks good to you.

This comment has been minimized.

@nicolo-ribaudo

nicolo-ribaudo Jul 10, 2020 Author Member

In this case, without with or MemberExpression, it has exactly the same behavior 😛

seenOptionalElement =
seenOptionalElement ||
(type === "TSNamedTupleMember" && elementNode.optional) ||
type === "TSOptionalType";
Comment on lines +654 to +657

This comment has been minimized.

@JLHwung

JLHwung Jul 10, 2020 Contributor

Suggested change
seenOptionalElement =
seenOptionalElement ||
(type === "TSNamedTupleMember" && elementNode.optional) ||
type === "TSOptionalType";
seenOptionalElement ||
(seenOptionalElement =
(type === "TSNamedTupleMember" && elementNode.optional) ||
type === "TSOptionalType");
@JLHwung
Copy link
Contributor

JLHwung commented Jul 14, 2020

@existentialism I think this can be merged because it targets to feat-7.11.0/ts-4.0 instead of main.

@JLHwung JLHwung removed this from the 7.11.0 milestone Jul 14, 2020
@JLHwung JLHwung merged commit 309536a into babel:feat-7.11.0/ts-4.0 Jul 14, 2020
7 of 8 checks passed
7 of 8 checks passed
build build
Details
test262-pr Workflow: test262-pr
Details
Gitpod Open an online workspace in Gitpod
Details
Travis CI - Pull Request Build Passed
Details
babel/repl REPL preview is available
Details
build-standalone Workflow: build-standalone
Details
ci/codesandbox Building packages succeeded.
Details
e2e Workflow: e2e
Details
@nicolo-ribaudo nicolo-ribaudo deleted the nicolo-ribaudo:ts-4.0/labeled-tuple-elements branch Jul 15, 2020
nicolo-ribaudo added a commit that referenced this pull request Jul 15, 2020
* TypeScript 4.0: Support labeled tuple elements

* More tests

* Disallow mixing labeled and unlabeled elements

* Update AST shape

* Enable test after rebase

* Allow labeled spread types

* Fix flow

* Add types and generator suport

* Update packages/babel-parser/src/plugins/typescript/index.js

* Prettier
JLHwung added a commit that referenced this pull request Jul 29, 2020
* TypeScript 4.0: Support labeled tuple elements

* More tests

* Disallow mixing labeled and unlabeled elements

* Update AST shape

* Enable test after rebase

* Allow labeled spread types

* Fix flow

* Add types and generator suport

* Update packages/babel-parser/src/plugins/typescript/index.js

* Prettier
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.