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

Documentation on moving off of Underscore to ES6/7 #28

Merged
merged 7 commits into from
Oct 29, 2015

Conversation

jeresig
Copy link
Member

@jeresig jeresig commented Oct 14, 2015

This goes through all the methods that we used in shared-package.js and provides modern alternatives to the methods. Can be expanded in the future!

@xymostech
Copy link
Contributor

I really like the table that @csilvers made here: https://github.com/Khan/style-guides/blob/master/style/javascript.md#es67-rules Some of them need extended descriptions but for some of the simple cases having 4-5 paragraphs of text is cumbersome.

should instead become:

```js
() => { ... }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With args, do we prefer (...args) => { someFn(...args) }? There was some talk about this the other day.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you do obj.someFn.bind(this) that has the same effect (the args are still passed in). I'm not sure I really see the benefit of having to write another wrapper function!

@jeresig
Copy link
Member Author

jeresig commented Oct 14, 2015

@xymostech hmm, good point! I'm not entirely sure what the best way to format it, though as even if some can fit in a table, and some won't, having them remain in alphabetical order seems to be the most important aspect. (and even for many of the "simple" cases it's still multiple lines of code for the change - I suspect that trying to fit it into a table will probably end up cramping the result)

@@ -27,6 +27,7 @@
* [Do not use async/await or generators](#do-not-use-asyncawait-or-generators)
* [Do not use Set or Map ](#do-not-use-set-or-map)
* [Use let and const for new files; do not use var ](#use-let-and-const-for-new-files-do-not-use-var)
* [Move off Underscore](#move-off-underscore)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should go into the 'library rules' section.

Also, most of this audience is written with the point of view that the audience is about to write new code, so 'move off' might be better phrased as "Avoid underscore" or something like that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@jeresig
Copy link
Member Author

jeresig commented Oct 16, 2015

@xymostech I moved to using a table instead of the longer list. Thanks for the suggestion!

Method | Use... | ...instead of
--------- | ------------------------------------- | ----------------------
bind | `fn.bind(someObj, args)` | `_.bind(fn, someObj, args)`
bind | `() => { ... }` | `_.bind(function() { ... }, this)`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I totally understand the difference between these two. Is the second one used when binding this, and the first one used in other situations?

In particular, it might be useful to have the second example have some args, if appropriate, something like (a, b) => { ... }. Otherwise I might assume this is only to be used in the no-arg case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct - the second one is specifically when you are just binding to this (and ideally when you're declaring the function there, as well). I'll add a footnote and add some args, too.

@csilvers
Copy link
Member

Is looking great -- I love the tabel! You did miracles with the formatting.

I had a bunch of line notes, but were all for low-level details. High level, I think it's looking great.

jeresig added a commit that referenced this pull request Oct 29, 2015
Documentation on moving off of Underscore to ES6/7
@jeresig jeresig merged commit 2ccb3a6 into master Oct 29, 2015
@jeresig jeresig deleted the move-off-underscore branch October 29, 2015 14:47
isArray | `Array.isArray(someObj)` | `_.isArray(someObj)`
isFunction | `typeof fn === "function"` | `_.isFunction(fn)`
isString | `typeof obj === "string"` | `_.isString(obj)`
keys | `Object.keys(obj)` | `_.keys(obj)`
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keys | Object.keys(obj) | _.keys(obj)

_.keys(obj) also smooths over inconsistencies in environments handling strings, arguments objects, and arrays (treating them as dense). For example, in ES5 Object.keys('hi') would throw, while in ES6 Object.keys('hi') returns ['0','1'].

@Khan Khan locked and limited conversation to collaborators Oct 30, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants