Skip to content
This repository has been archived by the owner on Aug 7, 2023. It is now read-only.

Utilize Atom's built in transpilation #890

Merged
merged 1 commit into from
May 3, 2017

Conversation

Arcanemagus
Copy link
Member

@Arcanemagus Arcanemagus commented Apr 20, 2017

Atom v1.13.0 introduced support for package's defining their own transpilation pipeline. Utilize this to remove the transpiled code from the repo and let Atom generate it on the fly.

NOTE: Based on the Task API branch, will need rebasing once #889 gets merged into master!

Fixes #857.

@Arcanemagus
Copy link
Member Author

Note that this has a detrimental effect on the "activation time" (as TimeCop calls it) of the package since the transpiler engine must be loaded:

image

That's ~29 ms added due to this.

@Arcanemagus
Copy link
Member Author

Filed atom/atom#14300 about the overhead from utilizing this. I still feel this is the direction we should go as it massively simplifies the code base, and the overhead there is coming from Atom itself.

@Arcanemagus Arcanemagus force-pushed the arcanemagus/atom-transpile branch 2 times, most recently from 5b191a2 to 4eb69e8 Compare May 2, 2017 16:22
@Arcanemagus Arcanemagus changed the base branch from arcanemagus/task-api to master May 2, 2017 16:23
@Arcanemagus Arcanemagus requested a review from IanVS May 2, 2017 16:24
@Arcanemagus
Copy link
Member Author

Looks like Travis-CI and AppVeyor didn't like the base branch change, attempting a close/re-open.

@Arcanemagus Arcanemagus closed this May 2, 2017
@Arcanemagus Arcanemagus reopened this May 2, 2017
@Arcanemagus
Copy link
Member Author

I think the AppVeyor failure is due to atom/ci#66, will update to my test version of the script to check.

@Arcanemagus
Copy link
Member Author

Arcanemagus commented May 2, 2017

Hmm, that fixes the usage of the system node for installing the dev modules, but it's still failing as if it can't find the babel-preset-node5 module.

Atom v1.13.0 introduced support for package's defining their own transpilation pipeline. Utilize this to remove the transpiled code from the repo and let Atom generate it on the fly.
@Arcanemagus
Copy link
Member Author

AppVeyor seems to have had a temporary issue causing the build to fail for a reason that should be completely out of their control, but changing nothing here has caused it to work properly.

On the plus side, I found that .babelrc was still in the project and have added removing that to this PR.

Copy link
Member

@IanVS IanVS left a comment

Choose a reason for hiding this comment

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

This seems to work great, and you've gotta love that line count diff! This will greatly improve the dev experience working in this package, not worrying about running the compiler and checking in compiled code.

Big 👍 from me.

@Arcanemagus Arcanemagus merged commit d2c2112 into master May 3, 2017
@Arcanemagus Arcanemagus deleted the arcanemagus/atom-transpile branch May 3, 2017 03:12
@4cm4k1 4cm4k1 mentioned this pull request May 23, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants