KeithHenry / chromeExtensionAsync

Promise wrapper for the Chrome extension API so that it can be used with async/await rather than callbacks
MIT License
229 stars 32 forks source link

transpile to ES5 #9

Closed sag1v closed 6 years ago

sag1v commented 6 years ago

At the moment the PR is just a Proof Of Concept for #8

I've created the build script using babel preset env, this should cover most if not all of the latest features of ES and transpile them to ES5.

As you can see it is creating a new file called compiled.js, we should change that and think of a way to keep the name chromeExtensionAsync at the current file structure (root folder) so we won't break the usage.

My suggestion is to move the ES2015 code file (maybe change it's name to index.js ?) to a different folder (/src?) then compile and save it to the root folder with the normal name chromeExtensionAsync.js.

KeithHenry commented 6 years ago

@sag1v Looks good!

Babel output is welcome, but I'm not sure about moving the source/renaming output.

My concern would be that, while the Babel output is better for build tools that assume ES5 (like the React CLI), the core usage of this will be directly in the current version of Chrome. Chrome runs better optimisations for current features (like spread operators) than the transpiler workarounds for older browsers.

Basically - React CLI users are going to be paying a small performance tax for the ES5 legacy transforms, I don't think that should be the default version in a tool designed to bring ES2018 syntax into Chrome plug-ins.

For most users the ES2018 syntax version should be the preferred resource, and the transpiled down version an opt-in for users affected by #8 - is there a convention for that?

sag1v commented 6 years ago

@KeithHenry Fair enough. Maybe we can publish a second file with ES5 version and people can get to choose what version for their usage.

So basically the current status of my PR supports this, we just need to decide what would be the name of the ES5 file (instead of compiled.js). In this line we can change --out-file the_new_name.js

KeithHenry commented 6 years ago

How about execute-async-function.es5.js? Then if you reference it directly in TypeScript the existing definitions should still get picked up.

As part of this PR we could also do with an addition to the README explaining why and when to use the transpiled version.

sag1v commented 6 years ago

@KeithHenry sounds great. I'll try to work on it later on. If you have any specific wording you want me to include in the README let me know :)

sag1v commented 6 years ago

@KeithHenry done 😺

KeithHenry commented 6 years ago

LGTM :-) Will do some testing tomorrow and get this updated. Thanks!