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

Cannot build Typescript AST #6933

Closed
dpoindexter opened this Issue Nov 29, 2017 · 6 comments

Comments

Projects
None yet
4 participants
@dpoindexter
Contributor

dpoindexter commented Nov 29, 2017

Is this a bug report or feature request?
Bug report

I'm writing a plugin to add Typescript type annotations to existing code. Some of the @babel/types builders seem to be configured to expect Flow nodes as arguments instead of the Typescript equivalents. In particular, nodes with typeParameters fields expect TypeParameterDeclaration.

Input Code

path.node.init.returnType = path.node.init.returnType = t.tSTypeAnnotation(
  t.tSTypeReference(
    t.tSQualifiedName(t.identifier('React'), t.identifier('SFC')),
    t.tSTypeParameterInstantiation([t.tSAnyKeyword()])
  )
);

Babel/Babylon Configuration (.babelrc, package.json, cli command)

{
    sourceType: 'module',
    allowImportExportEverywhere: false,
    allowReturnOutsideFunction: true,
    allowSuperOutsideMethod: true,
    plugins: [
        "typescript",
        "jsx",
        "asyncGenerators",
        "classProperties",
        "doExpressions",
        "exportExtensions",
        "functionBind",
        "functionSent",
        "objectRestSpread",
        "dynamicImport"
    ]
}

Expected Behavior

I would expect the code above to generate a type annotation like this:

... React.SFC<any> ...

Current Behavior

When running the plugin via @babel/traverse, I get the following stack trace:

TypeError: Property typeParameters of TSTypeReference expected node to be of a type ["TypeParameterInstantiation"] but instead got "TSTypeParameterInstantiation"
    at Object.validate (C:\Users\dpoindexter\Dev\Experiments\babel-codemod\node_modules\@babel\types\lib\definitions\index.js:86:13)
    at validate (C:\Users\dpoindexter\Dev\Experiments\babel-codemod\node_modules\@babel\types\lib\index.js:512:9)
    at Object.builder (C:\Users\dpoindexter\Dev\Experiments\babel-codemod\node_modules\@babel\types\lib\index.js:476:7)

Possible Solution

It looks like @babel/types hasn't been comprehensively updated, but I don't know it well enough to be sure of intent. It seems that node types like TypeParameterDeclaration need to either become a shared alias, or everywhere that expects a TypeParameterDeclaration needs to become a union TypeParameterDeclaration | TSTypeParameterDeclaration.

I'm happy to create a PR with a fix if that seems like the right approach!

Your Environment

software version(s)
Babel @babel/core 7.0.0-beta.32
node 8.4.0
yarn 1.3.2
Operating System Windows 10
@babel-bot

This comment has been minimized.

Show comment
Hide comment
@babel-bot

babel-bot Nov 29, 2017

Collaborator

Hey @dpoindexter! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community that typically always has someone willing to help. You can sign-up here
for an invite.

Collaborator

babel-bot commented Nov 29, 2017

Hey @dpoindexter! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community that typically always has someone willing to help. You can sign-up here
for an invite.

@existentialism

This comment has been minimized.

Show comment
Hide comment
@existentialism

existentialism Nov 29, 2017

Member

@dpoindexter I think we just need to update the TS-related types with TSTypeParameterDeclaration since we split the types in babel/babylon#710.

Would love a PR for this, let us know if you hit any snags! (Or join our slack!)

Member

existentialism commented Nov 29, 2017

@dpoindexter I think we just need to update the TS-related types with TSTypeParameterDeclaration since we split the types in babel/babylon#710.

Would love a PR for this, let us know if you hit any snags! (Or join our slack!)

@loganfsmyth

This comment has been minimized.

Show comment
Hide comment
@loganfsmyth

loganfsmyth Nov 29, 2017

Member

@existentialism Seems like a good first issue?

Member

loganfsmyth commented Nov 29, 2017

@existentialism Seems like a good first issue?

@existentialism

This comment has been minimized.

Show comment
Hide comment
@existentialism

existentialism Nov 29, 2017

Member

@loganfsmyth yep, was just taking up @dpoindexter's offer for a PR... but we can open it up as GFI if he's not up for it

Member

existentialism commented Nov 29, 2017

@loganfsmyth yep, was just taking up @dpoindexter's offer for a PR... but we can open it up as GFI if he's not up for it

@loganfsmyth

This comment has been minimized.

Show comment
Hide comment
@loganfsmyth

loganfsmyth Nov 30, 2017

Member

Woops! Missed

I'm happy to create a PR with a fix if that seems like the right approach!

@dpoindexter it's all yours if you want it :P

Member

loganfsmyth commented Nov 30, 2017

Woops! Missed

I'm happy to create a PR with a fix if that seems like the right approach!

@dpoindexter it's all yours if you want it :P

@dpoindexter

This comment has been minimized.

Show comment
Hide comment
@dpoindexter

dpoindexter Nov 30, 2017

Contributor

@loganfsmyth Yep, I'll take a shot at it.

Contributor

dpoindexter commented Nov 30, 2017

@loganfsmyth Yep, I'll take a shot at it.

@existentialism existentialism added claimed and removed help wanted labels Nov 30, 2017

@lock lock bot added the outdated label May 3, 2018

@lock lock bot locked as resolved and limited conversation to collaborators May 3, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.