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

More es6-ish syntax #13

Closed

Conversation

robertleeplummerjr
Copy link

No description provided.

@robertleeplummerjr
Copy link
Author

Mostly just opened to improve es6 syntax. Open to feedback, be brutal.

@unbornchikken
Copy link
Member

Why have you mixed up single and double quotes?

@robertleeplummerjr
Copy link
Author

Node (at least es6+) generally uses single quotes, and I didn't want to mess with the license area.
Grabbed at random: https://nodejs.org/dist/latest-v6.x/docs/api/console.html

@unbornchikken
Copy link
Member

Ok, but this project uses double quotes.

@unbornchikken
Copy link
Member

unbornchikken commented Sep 22, 2016

On the other hand, I'm using ES6 syntax supported by the 4.x branch of node. Why? Because, if you take a look at the code, there is a feature testing in the index files. If there is support for ES6 constructs used by this project then native ES6 source used instead of transpiled files. And native means faster.

Once Node 6 stabilizes, and earns its LTS status, I'll will make the source more (which means 100%) ES6ish. But until then, the transitional syntax stays.

And there is a conceptual error in your PR. There is a reason why scope uses functions instead of lambdas. Inside scope, function context's (this) is the scope object, and you can escape values from it by using this.result(...). Kinda jQuery funtions where this is the actual jQuery object.

I know it's not that obvious because those are not very well documented features. But I'm in the middle of the process of a complete rewrite, to make this library more faster and non blocking (#9) so many things and almost all of the current code will get changed or replaced by other construct eventually.

So I'm closing this PR, but please keep in touch, because in the next version my aim is to make this project more contributor friendly, and your help will be appreciated there.

@robertleeplummerjr
Copy link
Author

Understood, and I look forward to contributing in the future. Cannot wait to see your new additions!

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

Successfully merging this pull request may close these issues.

None yet

2 participants