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

Upgrade to @babel/parser; Apply Babel to TS transform files #287

Merged
merged 4 commits into from Dec 5, 2018

Conversation

brieb
Copy link
Contributor

@brieb brieb commented Nov 19, 2018

  • Upgrade babylon (deprecated) to @babel/parser
  • Upgrade other babel deps
  • Support transpiling TS transform files via @babel/preset-typescript
    • For example, jscodeshift -t sample/reverse-identifiers.ts all-the-code

Reviewers: @fkling @cpojer

"env": {
"jasmine": true
}
}
Copy link
Contributor Author

@brieb brieb Nov 19, 2018

Choose a reason for hiding this comment

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

Converted this file from yml to json for consistency

babelrc: false,
presets: [
require('babel-preset-es2015'),
require('babel-preset-stage-1'),
Copy link
Contributor Author

@brieb brieb Nov 19, 2018

Choose a reason for hiding this comment

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

@babel/preset-stage-1 is deprecated (see https://babeljs.io/docs/en/babel-preset-stage-1).
It was added in 28eb515.
Are there any experimental features we'd like to include?

Copy link
Contributor

@fkling fkling Nov 30, 2018

Choose a reason for hiding this comment

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

Good question. I'll take a look at the current proposal.

Another question though: Can @babel/preset-flow and @babel/preset-typescript be both loaded at the same time? It feels like this should be incompatible (at least you cannot set both options in babylon).
cc @hzoo

Copy link
Contributor

@hzoo hzoo Nov 30, 2018

Choose a reason for hiding this comment

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

Yeah, you shouldn't be able to because they would both try to set options in babylon and fail as incompat

Copy link
Contributor Author

@brieb brieb Dec 3, 2018

Choose a reason for hiding this comment

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

Oh interesting. Thank you so much for pointing this out! I didn't get an error from babel register about it and it appeared to run fine on the TS file I tested. But, I realize that my test TS file was valid flow so I didn't see any issues. I tried adding a TypeScript-only construct and it does indeed fail. I updated this PR to only load the typescript preset if it's a .tsx? transform.

@brieb brieb changed the title Upgrade to @babel/parser Upgrade to @babel/parser; Apply Babel to TS transform files Nov 19, 2018
@fkling
Copy link
Contributor

@fkling fkling commented Dec 4, 2018

Thank you! I will try to get this in in the next few days.

"@babel/preset-flow": "^7.0.0",
"@babel/preset-typescript": "^7.1.0",
"@babel/register": "^7.0.0",
"babel-core": "^7.0.0-bridge.0",
Copy link
Contributor

@fkling fkling Dec 4, 2018

Choose a reason for hiding this comment

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

Is this needed or can I just remove it? @brieb

Copy link
Contributor

@hzoo hzoo Dec 4, 2018

Choose a reason for hiding this comment

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

it's needed if as a dep (it just links to @babel/core) for any dependency that uses babel-core as a dep like jest and hasn't explicitly upgraded to @babel/core themselves (https://github.com/facebook/jest/tree/master/packages/babel-jest#usage)

Copy link
Contributor

@fkling fkling Dec 4, 2018

Choose a reason for hiding this comment

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

Ok, thank you @hzoo!

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

Successfully merging this pull request may close these issues.

None yet

4 participants