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: Support type arguments on tagged templates #7754

Merged
merged 4 commits into from Jul 26, 2018
Merged

TypeScript: Support type arguments on tagged templates #7754

merged 4 commits into from Jul 26, 2018

Conversation

ghost
Copy link

@ghost ghost commented Apr 18, 2018

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

@JamesHenry This changes the AST format. CC @DanielRosenwasser for review.
Supports parsing type arguments on tagged template calls.
Should wait on microsoft/TypeScript#23430 to be merged so we're sure we have the final syntax.

@babel-bot
Copy link
Collaborator

babel-bot commented Apr 18, 2018

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

@existentialism existentialism added the PR: New Feature 🚀 A type of pull request used for our changelog categories label Apr 27, 2018
@existentialism
Copy link
Member

LGTM, but will wait for @JamesHenry/@DanielRosenwasser to chime in!

@DanielRosenwasser
Copy link
Member

I feel like I commented this before, but I guess not. You need a test like

new foo<A, B>
`hello world`

To show ASI doesn't kick in.

@DanielRosenwasser
Copy link
Member

I feel like I commented this before, but I guess not. You need a test like

new foo<A, B>
`hello world`

To show ASI doesn't kick in.
I feel like I commented this before, but I guess not. You need a test like

new foo<A, B>
`hello world`

To show ASI doesn't kick in.
I feel like I commented this before, but I guess not. You need a test like

new foo<A, B>
`hello world`

To show ASI doesn't kick in.
I feel like I commented this before, but I guess not. You need a test like

new foo<A, B>
`hello world`

To show ASI doesn't kick in.

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented May 3, 2018

Christ, GitHub on a bad connection can be a real piece of work.

@existentialism
Copy link
Member

@DanielRosenwasser well, at least you made your point :)

}
},
"tag": {
"type": "NewExpression",
Copy link
Member

@DanielRosenwasser DanielRosenwasser May 3, 2018

Choose a reason for hiding this comment

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

This looks incorrect. The tagged template should have higher precedence than the NewExpression. As an example, in the following:

new C
``

here's what I currently get:

{
  // ...
  "expression": {
    "type": "NewExpression",
    "start": 0,
    "end": 8,
    "loc": {
      "start": {
        "line": 1,
        "column": 0
      },
      "end": {
        "line": 2,
        "column": 2
      }
    },
    "callee": {
      "type": "TaggedTemplateExpression",
      "start": 4,
      "end": 8,
      "loc": {
        "start": {
          "line": 1,
          "column": 4
        },
        "end": {
          "line": 2,
          "column": 2
        }
      },
      "tag": {
        "type": "Identifier",
        "start": 4,
        "end": 5,
        "loc": {
          "start": {
            "line": 1,
            "column": 4
          },
          "end": {
            "line": 1,
            "column": 5
          },
          "identifierName": "C"
        },
        "name": "C"
      },
      "quasi": {
        "type": "TemplateLiteral",
        "start": 6,
        "end": 8,
        "loc": {
          "start": {
            "line": 2,
            "column": 0
          },
          "end": {
            "line": 2,
            "column": 2
          }
        },
        "expressions": [],
        "quasis": [
          {
            "type": "TemplateElement",
            "start": 7,
            "end": 7,
            "loc": {
              "start": {
                "line": 2,
                "column": 1
              },
              "end": {
                "line": 2,
                "column": 1
              }
            },
            "value": {
              "raw": "",
              "cooked": ""
            },
            "tail": true
          }
        ]
      }
    },
    "arguments": []
  }
}

@kachkaev
Copy link

Given that styled-components accept types in template literals (styled-components/styled-components#1798), lot's of people would love to see this merged!

TypeScript is transpiled with babel in Next.js, which is pretty popular and is often used with styled-components. Here is a downstream issue that's pending this PR: vercel/next-plugins#203. 🙏

@existentialism existentialism self-assigned this Jul 10, 2018
@EItanya
Copy link

EItanya commented Jul 13, 2018

Any update on this being merged, Would love to be able to start using this feature!

@existentialism
Copy link
Member

@EItanya I made a note to get this rebased and fix the issue above, but if you'd like to tackle it... go for it!

@existentialism
Copy link
Member

existentialism commented Jul 18, 2018

Finally got around to rebasing/updating this PR.

Changes: fceb066

/cc: @andy-ms @DanielRosenwasser @nicolo-ribaudo

Note, I wound up dropping tsTryParseTypeArgumentsInExpression as the eatNextToken arg seemed misleading since it was just checking via this.match.

@timche
Copy link

timche commented Jul 23, 2018

Is there anything blocking from merging this? Thanks for all ur hard work to support this tho ❤️

@existentialism existentialism merged commit 8ee24fd into babel:master Jul 26, 2018
@hzoo hzoo mentioned this pull request Jul 26, 2018
2 tasks
storyn26383 pushed a commit to UniSharp/babel that referenced this pull request Aug 6, 2018
| Q                        | A
| ------------------------ | ---
| Fixed Issues?            | babel#7747 (partly)
| Patch: Bug Fix?          | 
| Major: Breaking Change?  | 
| Minor: New Feature?      | Yes
| Tests Added + Pass?      | Yes
| Documentation PR         |
| Any Dependency Changes?  |
| License                  | MIT

@JamesHenry This changes the AST format. CC @DanielRosenwasser for review.
Supports parsing type arguments on tagged template calls.
Should wait on microsoft/TypeScript#23430 to be merged so we're sure we have the final syntax.
@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 PR: New Feature 🚀 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants