Add lo-dash

[gruehle]

We should add Lo-dash to the Brackets core code. There are many places where we've re-implemented underscore-style functionality or could improve our code by using Lo-dash.

If you find a piece of code that could be better expressed using Lo-dash, please add it as a comment to this issue.

[gruehle]

Dang, I wrote backbone in the original bug report, but really meant underscore 😊. Title and description updated.

[gruehle]

Async.whenIdle() is the same as underscore debounce(), and could be eliminated.

[iwehrman]

1. utils/StringUtils.htmlEscape could be replaced by _.escape.
2. utils/NumberUtils contains only getRandomNumber, which could be replaced by _.random.
3. Better yet, DocumentManager._untitledDocumentPath could use _.uniqueId instead of the aforementioned random number facility.
4. There are a number of instances of setTimeout(fn, 0) that could be replaced by _.defer(fn). (But asap would be even better.)
5. There are a few of instances of array.slice(0) that could be replaced by _.clone(array).
6. CollectionUtils.hasProperty could be replaced by _.has.
7. CollectionUtils.some could be replaced by _.some.
8. CollectionUtils.forEach could be replaced by _.forEach.

[dangoor]

We spoke about this issue today and collectively decided that the extra features in Lo-Dash make it worthwhile to add to Brackets. Specifically, we would take the full "modern browser" version of Lo-Dash.

Also of note: we are going to separately consider the issue of using Lo-Dash for templating rather than Mustache. For now, we're just going to use the non-templating features of Lo-Dash.

[njx]

Removing "needs review". Assigning to me/medium priority for now as I'm interested in doing this, but if someone else would like to grab it feel free.

[iwehrman]

Do we want to load this as an AMD module or as a global? (My vote is for an AMD module, since we'll probably have to clean up our use of globals eventually anyway.)

[njx]

That seems fine to me. I assume that means we have to do _ = require("thirdparty/lodash/lodash") or some such at the beginning of each file? A little bit of a pain, but I've never liked having globals in general anyway.

[iwehrman]

If we want we can use the module name, or name it ourselves, so that we just have to do _ = require("lodash"), which might be slightly less annoying. Actually I think having "logical" module names for third-party libraries will be good for modularity too because it doesn't tie us to a particular library location and file name.

[njx]

👍

[iwehrman]

I don't know if I'll have time to finish this before I leave for PTO tomorrow, but here's a branch that adds Lo-Dash as a module named "lodash" and takes care of a single VanillaJS-to-Lo-Dash conversion. I generated the lodash file using lodash-cli as follows:

lodash modern --minify exports=amd

This generates a minified AMD module that assumes modern browsers.

[njx]

One thought: we might want to deprecate the utility functions we no longer need rather than removing them right away, in case other folks are using them. (I would guess no one is using this NumberUtils one, but if we started using _.escape() instead of StringUtils.htmlEscape(), for example, that might break some stuff.) Or we could just make our utility functions call the lodash versions.

[dangoor]

I think we should deprecate our existing functions. Hopefully people will test with the essentially equivalent lodash function.

[njx]

@iwehrman - any interest in updating your branch to deprecate the old function (rather than removing it for now) and making a pull request?

[iwehrman]

Sure. By "deprecate" do you mean redefine, e.g., StringUtils.htmlEscape to call _.escape and then console.warn about its usage?

Also: do you have a problem with removing NumberUtils in particular instead of deprecating it? I can't find any uses of that module using @dangoor's BracketsExtensionGrabber utility.

Reassigning to @iwehrman.

[njx]

If nobody is using NumberUtils, then yeah, we can probably just remove it (and release-note it). In general, though, I think it would be fine to make existing functions just call the lodash version (and warn) as long as the semantics are the same.

[iwehrman]

Fixed by #5539.