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

ESnext? #30

Open
tylersticka opened this issue May 6, 2016 · 9 comments
Open

ESnext? #30

tylersticka opened this issue May 6, 2016 · 9 comments
Labels

Comments

@tylersticka
Copy link
Member

I've been totally spoiled by our other repos. In particular, I think Handlebars helpers would benefit a lot from the new parameter stuff.

But, I understand the value of keeping these things lean as well. There could be performance issues or something similar. I'd consider this a low priority nicety.

@erikjung
Copy link
Contributor

erikjung commented May 6, 2016

We've been deferring the syntax conversion until we have an eslint config in place, so that we can simply add the config, then rely on Atom's linter to identity all of the syntax updates needed.

The process would look like this:

  1. Create a repo for our shareable config (e.g. eslint-config-cloudfour)
  2. Add an .eslintrc to this repo that simply extends the shared config (extends: cloudfour)
  3. Update files as needed, using https://github.com/AtomLinter/linter-eslint as a validator

@erikjung
Copy link
Contributor

erikjung commented May 6, 2016

Also, another thing to consider is whether or not we want to rely on Babel. I think it'd be nice to not have to transpile these helpers, since we're only targeting the Node environment for now, and most of the syntax features we want are supported natively in Node 4.0.

@tylersticka
Copy link
Member Author

and most of the syntax features we want are supported natively in Node 4.0.

Ooh! I did not know that. That's cool.

If it ends up being simpler to just keep things ES5 and avoid the dependency, I shan't be upset with that decision either. (FWIW)

@erikjung
Copy link
Contributor

erikjung commented May 6, 2016

Ooh! I did not know that. That's cool.

Well, to be clearer, the specific things you mentioned (default params, rest, spread) wouldn't be supported, but basic things like arrows, let, const, and object literal enhancements would. We'd also get most of the new built-in types like Map, Set, etc.

Node 4: http://kangax.github.io/compat-table/es6/#node4
Node 5: http://kangax.github.io/compat-table/es6/#node5

We might want to target Node 5 instead, but not all of us are running that yet.

@tylersticka
Copy link
Member Author

Good to know!

(That is one ugly table!)

@erikjung
Copy link
Contributor

erikjung commented May 9, 2016

@tylersticka @mrgerardorodriguez

This branch contains an ESLint config: https://github.com/cloudfour/core-hbs-helpers/tree/chore-esnextify

With that in place, we can begin addressing this issue by:

@gerardo-rodriguez
Copy link
Member

(That is one ugly table!)

LOL, yes it is.

This branch contains an ESLint config

Awesome! What is the plan to roll this out? Are we doing a trial run of sorts?

@erikjung
Copy link
Contributor

erikjung commented May 9, 2016

Awesome! What is the plan to roll this out? Are we doing a trial run of sorts?

@mrgerardorodriguez I think anybody can claim it. Maybe a sensible division of labor would be:

  1. the helpers themselves
  2. the associated tests

Separate PRs could be issued for each (or for even smaller bites). Also, we can assess the linter configuration itself during this process. There may be rules that need changing. I took the values from what's being used on another project, since it happened to have the most rules defined. These rules were written by a single person though, so I expect some differing opinions.

Regarding new syntax specifically...

The linter configuration will force some of it, but other things (like when to use arrow functions) will be open-ended. Also, we're currently limited to what's supported by Node 4.0, so that will rule out the application of some features (like default params and destructuring). You'll realize what will or won't work when you attempt to run the tests, since Node will fail.

When we're all running a newer version of Node that supports more of the syntax/features, we can do another round of refactoring that includes the fun stuff.

@tylersticka
Copy link
Member Author

FWIW, I'd consider this a lower priority than any of our Drizzle stuff.

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

No branches or pull requests

3 participants