Skip to content

Improve babel integration #523

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

Merged
merged 3 commits into from
Aug 10, 2015
Merged

Improve babel integration #523

merged 3 commits into from
Aug 10, 2015

Conversation

s-panferov
Copy link

Hello. I am developing https://github.com/s-panferov/awesome-typescript-loader and I decided to implement a feature to pick-up precompiled files for .ts modules if any. I need this to speed-up TypeScript + Babel use-case, because this pair is really slow on a big codebases. Unfortunately, atom-typescript externalTranspiler functionality has several issues:

  1. There is no way to pass options into Babel.
  2. There is no way to use .babelrc file, because filename property is not passed into babel options and babel doesn't know where to look for the .babelrc file.
  3. Generated source maps are possibly incorrect, because I get an error while using them in my workflow:
RROR in ../arui/src/icon/common/icon.tsx
Module build failed: Error: No element indexed by 1
    at ArraySet_at [as at] (/Users/panferov-s/Workspace/arui-nyc/node_modules/react-hot-loader/node_modules/source-map/lib/source-map/array-set.js:83:11)
    at SourceMapConsumer_parseMappings [as _parseMappings] (/Users/panferov-s/Workspace/arui-nyc/node_modules/react-hot-loader/node_modules/source-map/lib/source-map/source-map-consumer.js:225:44)
    at SourceMapConsumer.Object.defineProperty.get (/Users/panferov-s/Workspace/arui-nyc/node_modules/react-hot-loader/node_modules/source-map/lib/source-map/source-map-consumer.js:160:14)
    at SourceMapConsumer_eachMapping [as eachMapping] (/Users/panferov-s/Workspace/arui-nyc/node_modules/react-hot-loader/node_modules/source-map/lib/source-map/source-map-consumer.js:455:24)
    at Function.SourceNode_fromStringWithSourceMap (/Users/panferov-s/Workspace/arui-nyc/node_modules/react-hot-loader/node_modules/source-map/lib/source-map/source-node.js:78:26)
    at Object.module.exports (/Users/panferov-s/Workspace/arui-nyc/node_modules/react-hot-loader/index.js:87:16)
 @ ../arui-nyc/src/icon/icon.ts 20:29-65

I've fixed all the stuff in this PR. All useful changes are in the first commit. Please take a look.

Related issues:

#521 Implemented
#522 Hacked locally by adding typescript.d.ts into tsconfig.json by hand, but I think that it is not a good solution, so I don't include compiled files into this PR.

@s-panferov s-panferov changed the title Feature/improve babel integration Improve babel integration Aug 9, 2015
@basarat
Copy link
Member

basarat commented Aug 10, 2015

Marked as breaking change as it changes a key in tsconfig.json

@basarat
Copy link
Member

basarat commented Aug 10, 2015

Marked as breaking change as it changes a key in tsconfig.json

I see that you tried to maintain backward compat. Thanks 🌹

@basarat
Copy link
Member

basarat commented Aug 10, 2015

I am okay with this. But since I don't use it personally, would like a review by @nycdotnet 🌹

@s-panferov
Copy link
Author

@basarat

I see that you tried to maintain backward compat. Thanks 🌹

Yes. The first thought was to write such type:

externalTranspiler?: string | { name: string; options?: any };

It looks more self-describing, but leads to type guards or ugly type-casting inside building.ts. I can return the variant, if you want proper-looking types.

@basarat
Copy link
Member

basarat commented Aug 10, 2015

I can return the variant, if you want proper-looking types.

That would be better please 🌹 don't want magic meanings :)

@s-panferov
Copy link
Author

@basarat

Union type and type guard for externalTranspiler: https://github.com/s-panferov/atom-typescript/commit/87bc64f69616b2d33d06bee0a1fe2069a94d685e

All three cases tested locally:

  1. With string value
  2. With object value without options
  3. With object value with options

basarat added a commit that referenced this pull request Aug 10, 2015
@basarat basarat merged commit de477a5 into TypeStrong:master Aug 10, 2015
basarat added a commit that referenced this pull request Aug 10, 2015
@basarat
Copy link
Member

basarat commented Aug 10, 2015

Thanks v5.3.0 🌹

@nycdotnet
Copy link
Contributor

Thanks. I know this was merged already, but I will take a look at this soon. I'm a bit busy at the moment due to the birth of my son. :-)

@nycdotnet
Copy link
Contributor

Was pretty straight-forward. Nice job.

@johnnyreilly
Copy link
Member

Congratulations @nycdotnet!!!! Hope all is well

@nycdotnet
Copy link
Contributor

Thanks. :-) Mom and baby are good.

@johnnyreilly
Copy link
Member

Excellent! Yourself? I'm guessing: tired (I was this time last year)

@basarat
Copy link
Member

basarat commented Aug 12, 2015

@nycdotnet 👶 🌹

@TypeStrong TypeStrong locked and limited conversation to collaborators Jan 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants