Skip to content

Remove babel#2

Open
confused-Techie wants to merge 5 commits intomasterfrom
remove-babel
Open

Remove babel#2
confused-Techie wants to merge 5 commits intomasterfrom
remove-babel

Conversation

@confused-Techie
Copy link
Copy Markdown
Member

This PR removes babel from event-kit.
Since we are now in an ES6 compatible environment there's really no need to keep around multi-stage build process' and the extraneous extra dependencies that come with that. And it just makes sense to make an effort to simplify things where we can and start to remove the bloat we've inherited.

The one big concern with doing this though is it breaks ES5 style class support.

Now when creating this PR originally I didn't think it'd be that big of a deal and would just be a pain point of trying to modernize things. But now I'm not so sure.

Looking into it, the files here were originally CoffeeScript until Sep 11, 2018 atom/event-kit#38. Then shortly after decaffing the code the community created an issue atom/atom#18157 saying that this change breaks some packages, such as linter-spell & atom-latex.

So then this support was added back via babel in atom/event-kit#44, and later support was added in atom/event-kit#51 to be able to require the lib file here to get ES6 classes, with the default provided ES5 classes.

Since apparently, when a package's code is processed by babel elsewhere it will use ES5 style classes.

So that unfortunately potentially means that my PR here can break numerous packages. Which I didn't realize until I started drafting this PR summary.

So maybe we can use this as a way to open up a dialogue about how we want to handle this. Since in my mind, part of our original mission has been to make things modern here, while breaking as little as possible.

So really we have a few options:

  1. Don't modernize something if it will break packages unless we have to: Which in this case, this very much falls under "we don't have too."
  2. Modernize and understand some things will break: Which this may just be something that breaks.
  3. Modernize and find workarounds for outdated packages: Meaning, we modernize, and just try to find ways to make the package's code work, rather than keep our stuff old to work with them.

In this case, I'd choose option 3. Especially, since my understanding is that Pulsar itself controls babel parsing for community packages, so maybe that means we could find a way to use that babel parsing to update a community package to use ES6 classes. Or we could potentially even use a codemod to do that selectively, where in a perfect world the codemod could check specifically for requiring Disposable from atom and modify it to be an ES6 require.

But curious what others may think on this front.

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

Labels

None yet

Projects

Status: Ready

Development

Successfully merging this pull request may close these issues.

1 participant