Skip to content
This repository has been archived by the owner on Nov 30, 2018. It is now read-only.

Lodash 4.0 release breaks some functions #1682

Closed
david-meza opened this issue Jan 13, 2016 · 31 comments
Closed

Lodash 4.0 release breaks some functions #1682

david-meza opened this issue Jan 13, 2016 · 31 comments

Comments

@david-meza
Copy link
Contributor

No description provided.

@david-meza
Copy link
Contributor Author

From backtrace:

TypeError: _.contains is not a function
at angular-google-maps.js:6538

TypeError: _.object is not a function
at angular-google-maps.js:6771

@armyofda12mnkeys
Copy link
Contributor

Just noticed this myself. angular-google-maps 2.2.1 depends on lodash 4... but lodash 4 repaced _.contains w/ _.includes i think i read.

@nmccready
Copy link
Contributor

Well crap the versioning should have been 3.X . Any suggestions on dealing with both 3 and 4 ?

@armyofda12mnkeys
Copy link
Contributor

Can you maybe define the alias yourself if it doesn't exist before rest of angular-google-maps runs?
Dunno something simple like so:

if( typeof _.contains === 'undefined' ) {
  _.contains = _.includes;
} //else it exists and we good for lodash3 without doing anything else

@david-meza
Copy link
Contributor Author

It seems _.objectwas also replaced with _.zipObject but the functionality is the same. I agree that aliasing would be a good solution to deal with both versions.

@nmccready
Copy link
Contributor

#1683

@nmccready
Copy link
Contributor

So it is locked down on master and on 2.2.X , as for a solid fix to use 4.0. That will have to come later or by some other soul (PR). I am currently not using this library for work anymore and can only work on it in my spare time (which is not much with work and 5 month old).

@armyofda12mnkeys
Copy link
Contributor

I can maybe take a look later (since im using other projects that use lodash 4). Will let u know if a PR happens.

@armyofda12mnkeys
Copy link
Contributor

Might need a little help nmccready...
Where would be a good place to put the lodash alias code (before lodash is referenced/used in angular-google-maps)? I can start playing with throwing some code in there and seeing if any more lodash methods (like _.object above) need to get re-aliased.

Maybe one of the init/load methods I see or wasnt sure if uiGmapLodash is the definite optimal place (since i see below code does something similar... looks like if _.get isn't available for lodash2? users maybe, it creates it):

if (_.get == null) {
      //...code...
      _.get = get;
}

@eliely
Copy link

eliely commented Jan 15, 2016

Here is solution:

if( typeof _.contains === 'undefined' ) {
    _.contains = _.includes;
    _.prototype.contains = _.includes;
}
if( typeof _.object === 'undefined' ) {
    _.object = _.zipObject;
}

We write it in Main.js file
_.prototype.contains = _.includes; - because ui-gmap-marker used _(someName).contains construction

@armyofda12mnkeys
Copy link
Contributor

ui-gmap-marker used _(someName).contains
Hey eliely,
Why would they create an instance of lodash?
Are you sure its not the googlemapbounds.contains or string.contains methods and definitely lodash's contains?

@nmccready
Copy link
Contributor

@armyofda12mnkeys what file and line please via github urls.

@eliely
Copy link

eliely commented Jan 15, 2016

They use lodash "Chain" methods https://github.com/lodash/lodash/blob/3.10.1/doc/README.md
Here is they code

var doIgnore;
     if (ignores) {
          doIgnore = _(ignores).contains(eventName);
     }

https://github.com/angular-ui/angular-google-maps/blob/master/dist/angular-google-maps.js
line 945

@nmccready
Copy link
Contributor

Yeah its here so it is in events-helper.coffee
https://github.com/angular-ui/angular-google-maps/blob/master/dist/angular-google-maps.js#L947

That chain can easily be removed. Using _.contains(ignores, eventName) or just using array.indexOf

@nmccready
Copy link
Contributor

https://github.com/angular-ui/angular-google-maps/blob/master/src/coffee/directives/api/utils/events-helper.coffee#L19

PR merged changed it to includes, but still the chain is not needed.

@armyofda12mnkeys
Copy link
Contributor

This added to line 99 in lodash.coffee (https://github.com/angular-ui/angular-google-maps/blob/master/src/coffee/extensions/lodash.coffee) might work: but haven't really tested it (I couldn't compile the full coffeescript via grunt ... but it might be something else [wierd phantomJS underscore errors] if you can take a look):

###
        For Lodash 4 compatibility (some aliases are removed)
###

if typeof _.contains == undefined
    _.contains = _.includes;
    _.prototype.contains = _.includes;  
if typeof _.object == undefined
    _.object = _.zipObject;
if typeof _.all == undefined
    _.all = _.every;
if typeof _.any == undefined
    _.any = _.some;

@tucq88
Copy link

tucq88 commented Jan 20, 2016

using temporary solution for now. But I'm looking forward for a proper fix :D Thanks you guys for your hard works.

@nmccready
Copy link
Contributor

This does not fix a lot of things. Many specs are broken on using lodash 4.0 . Still having a tough time figuring out a few of them.

Here is the branch with progress. https://github.com/angular-ui/angular-google-maps/tree/dev/nmccready/tryingToMakeLodash4Wurk

@nmccready
Copy link
Contributor

Anyone feel free to help work on that branch and PR to it. I can only work on it once in a while. I put too much time into it last night anyhow.

@armyofda12mnkeys
Copy link
Contributor

Many specs are broken on using lodash 4.0 .

I can take a look...
Any errors/line#'s in particular (I was able to get a map with that 2.2.x build w/ no errors)?
I wasn't sure if you using some special map options that I wasn't using.

@nmccready
Copy link
Contributor

You should be seeing errors.

Pull down the code and run grunt or grunt buildAll the specs will blow up. Just because and example or the library can be loaded means nothing. The specs are the spoken gospel and they must pass.

@nmccready
Copy link
Contributor

Tests that are broken:

uiGmapMarkers (directive creation)
    object models
      should add markers for each object in model
        ✖ from start
        ✖ from start
    array models
      should add markers for each object in model
        ✔ from start
      dynamic
        ✔ delayed creation
        update an existing marker should modify an existing gObject (gMarker)
          ✖ by reference
          ✖ by position (model)
      can eval key function
        ✔ handles click function in model
      can eval expressions
        ✔ handles click expression to function
      ✔ exists
ModelKey Tests
    ✔ should eval model handle correctly
    ✖ should properly compare models
    ✔ should properly set id key

@nmccready
Copy link
Contributor

#1711 resolves this

nmccready added a commit that referenced this issue Jan 28, 2016
@armyofda12mnkeys
Copy link
Contributor

I couldn't figure it out. Thanks for fixing @nmccready ...
What was the main fix for this?
I thought maybe looking at your updates that it wasn't a lodash fix needed, but possibly the way to handle objects in getArrayAndKeys and maybe thats why those tests were failing?

@nmccready
Copy link
Contributor

Alot of the problem was in _async.coffee and I just removed lodash from that part.

@nmccready
Copy link
Contributor

This part

collection = _.pick collection, (val, propName) ->
collection.hasOwnProperty propName
array = if keys then keys else Object.keys(_.omit(collection, ['length', 'forEach', 'map']))
keys = array

became

if keys
array = keys
else
array = []
for propName, val of collection
if collection.hasOwnProperty(propName) and !_.includes(_ignoreFields, propName)
array.push propName

@nmccready
Copy link
Contributor

More patching

fixLodash = ({missingName, swapName, isProto}) ->
unless _[missingName]?
_[missingName] = _[swapName]
_::[missingName] = _[swapName] if isProto
[
{missingName: 'contains', swapName: 'includes', isProto: true}
{missingName: 'includes', swapName: 'contains', isProto: true}
{missingName: 'object', swapName: 'zipObject'}
{missingName: 'zipObject', swapName: 'object'}
{missingName: 'all', swapName: 'every'}
{missingName: 'every', swapName: 'all'}
{missingName: 'any', swapName: 'some'}
{missingName: 'some', swapName: 'any'}
{missingName: 'first', swapName: 'head'}
{missingName: 'head', swapName: 'first'}
].forEach (toMonkeyPatch) ->
fixLodash(toMonkeyPatch)

@jrencz
Copy link

jrencz commented Mar 7, 2016

@nmccready correct me if I'm wrong (I've never written a line of coffee script) but as long as I can tell lodash.coffee modifies the _ global?
There's https://lodash.com/docs#mixin for that

@nmccready
Copy link
Contributor

Sure, there are several ways to do a lot of things.

@amak07
Copy link

amak07 commented Mar 15, 2016

@nmccready Hey sir, I am having this problem when trying to use angular-ui-map in my MEAN stack application for a course in college. From what I understand, the "aliasing" code bit should fix this problem for me? If so, I am completely unsure of where to add this line of code. I am using the current version of the MEAN.js boilerplate template for this application and have done the bower install --save. Could you please advise on where to implement the code bit to solve this problem, thanks!

@shabana-parveen
Copy link

@nmccready , I am getting below for running appium command from terminal, Please provide solution for same:

$ npm ls lodash
bin@1.0.0 /Users/f2849
└── lodash@4.17.4

npm version : 5.3.0
node version : v8.6.0
appium version : 1.7.0 beta

TypeError: _lodash2.default.toPairs is not a function
at Object. (../../lib/adb.js:74:28)
at Module._compile (module.js:570:32)
at Object.Module._extensions..js (module.js:579:10)
at Module.load (module.js:487:32)
at tryModuleLoad (module.js:446:12)
at Function.Module._load (module.js:438:3)
at Module.require (module.js:497:17)
at require (internal/module.js:20:19)
at Object. (/usr/local/lib/node_modules/appium/node_modules/appium-android-driver/node_modules/appium-adb/build/index.js:11:15)
at Module._compile (module.js:570:32)
at Object.Module._extensions..js (module.js:579:10)
at Module.load (module.js:487:32)
at tryModuleLoad (module.js:446:12)
at Function.Module._load (module.js:438:3)
at Module.require (module.js:497:17)
at require (internal/module.js:20:19)
at Object. (../../lib/android-helpers.js:10:49)
at Module._compile (module.js:570:32)
at Object.Module._extensions..js (module.js:579:10)
at Module.load (module.js:487:32)
at tryModuleLoad (module.js:446:12)
at Function.Module._load (module.js:438:3)
at Module.require (module.js:497:17)
at require (internal/module.js:20:19)
at Object. (../../../lib/commands/general.js:1:23)
at Module._compile (module.js:570:32)
at Object.Module._extensions..js (module.js:579:10)
at Module.load (module.js:487:32)
at tryModuleLoad (module.js:446:12)
at Function.Module._load (module.js:438:3)
at Module.require (module.js:497:17)
at require (internal/module.js:20:19)
at Object. (../../../lib/commands/index.js:1:30)
at Module._compile (module.js:570:32)
at Object.Module._extensions..js (module.js:579:10)
at Module.load (module.js:487:32)
at tryModuleLoad (module.js:446:12)
at Function.Module._load (module.js:438:3)
at Module.require (module.js:497:17)
at require (internal/module.js:20:19)

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

No branches or pull requests

8 participants