Skip to content

Implementing classes using the new ES6 syntax (branch 5.0)#3070

Closed
leonardo-silva wants to merge 2 commits intoexpressjs:5.0from
leonardo-silva:5.0
Closed

Implementing classes using the new ES6 syntax (branch 5.0)#3070
leonardo-silva wants to merge 2 commits intoexpressjs:5.0from
leonardo-silva:5.0

Conversation

@leonardo-silva
Copy link

I created a new PR to change the branches (5.0 -> 5.0).
Initial PR: #3069

Original message:
Hi,

In my PhD research, I am using express in some experiments.

As a result, I detected the existence of at least 3 main "classes" in the current codebase (View, Route, Layer).

So, I decided to rewrite this code to follow the new syntax for classes provided by ECMAScript 2016, which I think is much more readable. All changes are just syntactical. I was also very careful to preserve the code style.
Finally, all tests are green in the migrated code.

Do you think it is worth to integrate these changes to the project?

Thank you

@Twipped
Copy link

Twipped commented Aug 30, 2016

Change for change's sake 👎

@TrejGun
Copy link

TrejGun commented Sep 6, 2016

why only one class? change all and add babel config to transpile to work on old nodejs, otherwise it is useless

@leonardo-silva
Copy link
Author

It seems that classes Route and Layer were removed in branch 5.0

@dougwilson
Copy link
Contributor

I'm 👎 and looks like a lot of the TSC and contributors are as well. At the very least, it causes failures on the currently-supported platforms and, as stated above, really just change for change :) To expand on that, we keep our source in ES5 to support the largest user base as is reasonable, being a core library.

@dougwilson dougwilson closed this Jan 21, 2017
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.

4 participants