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

make readme examples for ES2015 friendly #879

Closed
wants to merge 1 commit into from

Conversation

zeke
Copy link
Contributor

@zeke zeke commented Jun 17, 2016

$(this) will not work within "arrow" functions, where this refers to the lexical this value in the scope of the declaration.

This PR updates removes $(this) from the README in favor of $(el), so the code examples can be rewritten in ES2015 and will still work.

cc @gajus, #762

@coveralls
Copy link

Coverage Status

Coverage remained the same at 98.832% when pulling 99cedd4 on zeke:replace-this-with-el into 4ccb41b on cheeriojs:master.

3 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 98.832% when pulling 99cedd4 on zeke:replace-this-with-el into 4ccb41b on cheeriojs:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 98.832% when pulling 99cedd4 on zeke:replace-this-with-el into 4ccb41b on cheeriojs:master.

@coveralls
Copy link

coveralls commented Jun 17, 2016

Coverage Status

Coverage remained the same at 98.832% when pulling 99cedd4 on zeke:replace-this-with-el into 4ccb41b on cheeriojs:master.

@zeke
Copy link
Contributor Author

zeke commented Jun 17, 2016

whoa, @coveralls 🏇

@tomByrer
Copy link

Plus el reads much better than this.

@gajus
Copy link

gajus commented Nov 17, 2016

This PR updates removes $(this) from the README in favor of $(el), so the code examples can be rewritten in ES2015 and will still work.

While you are at it, I'd change use of regular functions to arrow expressions.

@zeke
Copy link
Contributor Author

zeke commented Dec 16, 2016

Bump.

I still think this PR is worth landing, for the sake of making the readme friendly for users of arrow functions.

@jugglinmike
Copy link
Member

This change makes the documentation less expressive overall because the this
value of those functions is no longer demonstrated. It's beyond the scope of
this project to help folks learn to use new language features; if consumers use
arrow functions, we have to assume they understand the implications that has on
their code and any examples they find elsewhere. The second parameter is already
listed here (and given the conventional name el), so I think that should be
sufficient.

@zeke zeke deleted the replace-this-with-el branch January 15, 2017 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants