Added support for record and tuple syntax. #10865

rickbutton · 2019-12-13T20:02:44Z

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

TC39 Proposal Info:
Champion: @rricard @rickbutton
Spec Repo: https://github.com/tc39/proposal-record-tuple
Slides (Stage 1 at Oct 2019 TC39 Meeting): https://button.dev/talks/records-and-tuples-tc39-october-2019.pdf

This PR adds support for both the hash style #{ a: 1 } and the bar style {| a: 1 |} record and tuple syntax to babel-parser under the option recordAndTuple.{hash/bar}. I've also added a new plugin babel-plugin-syntax-record-and-tuple for enabling the parser option.

This does not include a plugin for transforming the syntax, we intend to iterate on a transform and polyfill separately for now, similar to the approach taken for BigInt.

This is my first babel PR, so please let me know if I'm missing something!

littledan

At a glance, looks like you added the "hash": true option to use #{ } syntax, and otherwise, the {| |} syntax is used. I'd suggest that you instead make a mandatory parameter which is string-valued, like "syntax": "bar" vs "syntax": "hash". This could be more future-proof.

rickbutton · 2019-12-13T21:55:33Z

@littledan the changes to the tokenzier check for the existence of the bar config options in order to tokenize the {| and |} tokens, the parser itself will consume those tokens regardless of the options provided (but more defense in depth would make sense here, I should probably check for the option at both levels).

I'm also not opposed to switching to syntax: "bar" instead of bar: true so that one or the other must be chosen.

packages/babel-parser/ast/spec.md

JLHwung · 2019-12-16T19:34:54Z

packages/babel-parser/src/parser/expression.js

@@ -1046,11 +1060,27 @@ export default class ExpressionParser extends LValParser {
        this.state.inFSharpPipelineDirectBody = oldInFSharpPipelineDirectBody;
        return this.finishNode(node, "ArrayExpression");
      }
+      case tt.braceBarL:
+      case tt.braceHashL: {


should assert recordAndTuple presents otherwise it is a syntax error.

this.expectPlugin("recordAndTuple");

Can you add a test case when recordAndTuple is not enabled?

JLHwung · 2019-12-16T19:45:02Z

packages/babel-types/src/definitions/experimental.js

+  aliases: ["Expression"],
+  fields: {
+    syntaxType: {
+      validate: assertValueType("string"),


Let's be more explicit

assertOneOf("bar", "hash")

I'm not sure that this information should be there. This seems like syntax-specific information, not related to the AST.
It's similar to how the presence or absence of a semicolon is not represented in the AST.

I would prefer it as an option in @babel/generator, which should throw when printing a RecordExpression node if that option is not specified.
@babel/plugin-syntax-record-and-tuple would set both the parser option and the generator option.

@nicolo-ribaudo do you mean an additional property on the node specifically for the generator to use to pick syntax to output? If so, why wouldn't you just use the existing syntaxType property?

The generator options are independent on AST. By doing so we can have the AST agnostic about the syntax types, which is semantically equivalent.

You can add a new format option here

babel/packages/babel-generator/src/index.js

Line 39 in c35ba3d

const format = {

and reference this option in the generator via this.format.recordTupleSyntaxType.

In the syntax-* you can mutate the generator options via options.generatorOpts.recordTupleSyntaxType.

JLHwung · 2019-12-16T20:11:15Z

packages/babel-plugin-syntax-record-and-tuple/src/index.js

+      parserOpts.plugins.push([
+        "recordAndTuple",
+        {
+          hash: options.hash,


I prefer to expose syntaxType to the user land, because

When either syntax variants is accepted, we can make that as default. We have similar design on pipeline-operator.

We don't have to process option precedence issues like hash: false, bar: false and hash: true, bar: true.

More open to competing proposals, if any

nicolo-ribaudo

Could you also add a parser test with Flow like this?

var a: {| x: number |} = {| x: 2 |}

nicolo-ribaudo · 2019-12-16T20:28:37Z

packages/babel-parser/src/tokenizer/types.js

@@ -93,9 +93,13 @@ export const types: { [name: string]: TokenType } = {

  // Punctuation token types.
  bracketL: new TokenType("[", { beforeExpr, startsExpr }),
+  bracketHashL: new TokenType("#[", { beforeExpr, startsExpr }),
+  bracketBarL: new TokenType("{|", { beforeExpr, startsExpr }),


Suggested change

bracketBarL: new TokenType("{|", { beforeExpr, startsExpr }),

bracketBarL: new TokenType("[|", { beforeExpr, startsExpr }),

rickbutton · 2019-12-18T22:50:54Z

I've pushed a set of commits that I think addresses all of your concerns so far:

I've moved the parser options to use syntaxType: "hash" instead of hash: true cc @littledan @JLHwung
added checks to the parser to check for the recordAndTuple plugin instead of relying only on the tokenizer to pick up the error (and added tests to check for missing plugin + invalid syntaxType
Moved the syntaxType node property into an option to the generator (and added some tests for this)
added a test for flow fixed type syntax with record/tuple bar syntax cc @nicolo-ribaudo

Let me know if I missed anything! Thank you for the super detailed + quick reviews!

edit: looks like i need to account for some lint errors as well, I will follow up on those soon.

rickbutton · 2020-01-09T15:51:16Z

Just pushed a rebase, looks like the standalone tests pass, but e2e and coverage are failing, but they look like unrelated failures. Any ideas? @nicolo-ribaudo @JLHwung

JLHwung · 2020-01-09T15:54:26Z

The CI failure is unrelated. Did you fetch the upstream? If you rebase on the latest upstream the CI error should be gone.

rickbutton · 2020-01-09T16:12:40Z

@JLHwung yep, fetched upstream and rebased. parent commit of this PR is current HEAD of babel/babel#master 8fd532d

JLHwung · 2020-01-10T01:02:25Z

@rickbutton Can you rebase again? The CI error has been fixed in recent commits.

rickbutton · 2020-01-10T16:24:38Z

@JLHwung rebased again, everything is green! (minus test262). Thank you!

nicolo-ribaudo · 2020-01-13T22:06:47Z

packages/babel-generator/src/generators/types.js

+    endToken = "}";
+  } else {
+    throw new Error(
+      `${this.format.recordAndTupleSyntaxType} is not a valid recordAndTuple syntax type`,


This error message should mention the option name, and also be easy to understand when the option is not provided. WDYT about this?

`The "recordAndTupleSyntaxType" generator option must be either "bar" or "hash" (${JSON.stringify(this.format.recordAndTupleSyntaxType)} received).`,

Also, please add a test for an invalid value of the option and for when the option is not set.

@nicolo-ribaudo pushed a change to include this new message, and added the tests you mentioned.

The babel-generator test runner doesn't support the standard throws option in options.json (looks like it isn't using the same infra as everything else). I took the liberty of adding support for throws to the package's test suite, let me know if this should exist somewhere else.

nicolo-ribaudo · 2020-01-13T22:11:54Z

packages/babel-parser/src/tokenizer/index.js

    const start = this.state.pos;
    this.state.pos += 1;

-    let ch = this.input.charCodeAt(this.state.pos);
-    if (ch !== charCodes.exclamationMark) return false;


What is this change for?

I moved this because it will incorrectly eat a # character if the next character isn't a !, I think that the this.state.pos should only be advanced if it is actually going to be an interpreter directive. (This happens if the first line of the file is #{}

packages/babel-parser/src/types.js

nicolo-ribaudo · 2020-01-14T20:13:28Z

...abel-parser/test/fixtures/experimental/record-and-tuple/invalid-type-bar-record/options.json

@@ -0,0 +1,4 @@
+{
+  "plugins": [["recordAndTuple", { "syntaxType": "invalid" }]],
+  "throws": "Unexpected token (1:1)"


Here we should throw an error similar to @babel/generator's for the invalid option.

Good call, how about this? (used the pipelineOperator option checks for a reference here).

'recordAndTuple` requires `syntaxType` option whose value should be one of: 'hash', 'bar'

littledan reviewed Dec 13, 2019

View reviewed changes

nicolo-ribaudo self-assigned this Dec 14, 2019

nicolo-ribaudo added pkg: parser PR: New Feature 🚀 A type of pull request used for our changelog categories labels Dec 14, 2019

JLHwung reviewed Dec 16, 2019

View reviewed changes

packages/babel-parser/ast/spec.md Show resolved Hide resolved

nicolo-ribaudo unassigned nicolo-ribaudo Dec 16, 2019

nicolo-ribaudo self-requested a review December 16, 2019 19:12

JLHwung reviewed Dec 16, 2019

View reviewed changes

nicolo-ribaudo reviewed Dec 16, 2019

View reviewed changes

JLHwung added the Spec: Record Tuple label Dec 16, 2019

rricard mentioned this pull request Dec 23, 2019

Transpiler strategy tc39/proposal-record-tuple#8

Closed

rickbutton mentioned this pull request Dec 30, 2019

Improvements covering a few ongoing issues tc39/proposal-record-tuple#89

Merged

rickbutton force-pushed the rb/record-and-tuple branch from d342e4b to e58cf0c Compare January 9, 2020 15:41

nicolo-ribaudo added this to the v7.9.0 milestone Jan 9, 2020

rickbutton force-pushed the rb/record-and-tuple branch from e58cf0c to f792ffe Compare January 10, 2020 15:22

nicolo-ribaudo reviewed Jan 13, 2020

View reviewed changes

rickbutton force-pushed the rb/record-and-tuple branch from 1437239 to 2ddfcd0 Compare January 14, 2020 16:00

nicolo-ribaudo reviewed Jan 14, 2020

View reviewed changes

rickbutton force-pushed the rb/record-and-tuple branch from 7686bf4 to 34c9fea Compare January 15, 2020 16:33

fisker mentioned this pull request May 29, 2020

Babel: support record and tuple syntax prettier/prettier#8453

Merged

4 tasks

github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jun 30, 2020

github-actions bot locked as resolved and limited conversation to collaborators Jun 30, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added support for record and tuple syntax. #10865

Added support for record and tuple syntax. #10865

rickbutton commented Dec 13, 2019

littledan left a comment

rickbutton commented Dec 13, 2019

JLHwung Dec 16, 2019

JLHwung Dec 16, 2019

nicolo-ribaudo Dec 16, 2019

rickbutton Dec 17, 2019

JLHwung Dec 17, 2019 •

edited

JLHwung Dec 16, 2019

nicolo-ribaudo left a comment

nicolo-ribaudo Dec 16, 2019

rickbutton commented Dec 18, 2019 •

edited

rickbutton commented Jan 9, 2020

JLHwung commented Jan 9, 2020

rickbutton commented Jan 9, 2020

JLHwung commented Jan 10, 2020

rickbutton commented Jan 10, 2020

nicolo-ribaudo Jan 13, 2020

rickbutton Jan 14, 2020

nicolo-ribaudo Jan 13, 2020

rickbutton Jan 13, 2020

nicolo-ribaudo Jan 14, 2020

rickbutton Jan 14, 2020

nicolo-ribaudo Jan 14, 2020

rickbutton Jan 15, 2020

	bracketBarL: new TokenType("{\|", { beforeExpr, startsExpr }),
	bracketBarL: new TokenType("[\|", { beforeExpr, startsExpr }),

Added support for record and tuple syntax. #10865

Added support for record and tuple syntax. #10865

Conversation

rickbutton commented Dec 13, 2019

littledan left a comment

Choose a reason for hiding this comment

rickbutton commented Dec 13, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JLHwung Dec 17, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolo-ribaudo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rickbutton commented Dec 18, 2019 • edited

rickbutton commented Jan 9, 2020

JLHwung commented Jan 9, 2020

rickbutton commented Jan 9, 2020

JLHwung commented Jan 10, 2020

rickbutton commented Jan 10, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JLHwung Dec 17, 2019 •

edited

rickbutton commented Dec 18, 2019 •

edited