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

Lodash broken, won't fix #32

Closed
ekkis opened this issue Mar 28, 2019 · 6 comments
Closed

Lodash broken, won't fix #32

ekkis opened this issue Mar 28, 2019 · 6 comments

Comments

@ekkis
Copy link

ekkis commented Mar 28, 2019

your module depends on lodash, which has a problem described here: https://github.com/lodash/lodash/issues/4247

as is evidenced by the thread, the lodash team is completely uncooperative, which means I can't use your module. the fix is small and simple. it is possible for your team to post-patch their module, fork it and fix it, or implement some other kind of solution?

@christroutner
Copy link
Contributor

@ekkis can you expand on how the bug manifests itself in SLP-SDK? What is the problem that it causes, exactly?

@ekkis
Copy link
Author

ekkis commented Mar 29, 2019

the lodash owner has agreed to apply a patch, so this should be ok. will post more when done

@ekkis
Copy link
Author

ekkis commented May 2, 2019

after a long and painful path, I couldn't figure out how to fix that pile of doodoo so I need to revert back to you guys. here's an example of how this breaks:

const jsp = require('js-prototype-lib');
jsp.install()

const SLP = require('slp-sdk');
var slp = new SLP({restURL: 'https://rest.bitcoin.com/v2/'});
var ret = slp.Mnemonic.toSeed('').toString('hex').substr(0, 16);
console.log(ret);

if you run the above (using the latest 3.3.0 version of SLP), you'll get something like:

/Users/ekkis/dev/now/node_modules/lodash/lodash.js:17045
names.push({ 'name': methodName, 'func': lodashFunc });
^

TypeError: names.push is not a function
at /Users/ekkis/dev/now/node_modules/lodash/lodash.js:17045:15
at /Users/ekkis/dev/now/node_modules/lodash/lodash.js:4911:15
at baseForOwn (/Users/ekkis/dev/now/node_modules/lodash/lodash.js:2996:24)
at runInContext (/Users/ekkis/dev/now/node_modules/lodash/lodash.js:17039:5)
at Object. (/Users/ekkis/dev/now/node_modules/lodash/lodash.js:17080:11)
at Object. (/Users/ekkis/dev/now/node_modules/lodash/lodash.js:17107:3)
at Module._compile (internal/modules/cjs/loader.js:721:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:732:10)
at Module.load (internal/modules/cjs/loader.js:620:32)
at tryModuleLoad (internal/modules/cjs/loader.js:560:12)

which can be solved by applying the following patch:

~/dev/now $ cat patches/lodash+4.17.11.patch 
diff --git a/node_modules/lodash/lodash.js b/node_modules/lodash/lodash.js
index cb139dd..9730f52 100644
--- a/node_modules/lodash/lodash.js
+++ b/node_modules/lodash/lodash.js
@@ -17039,8 +17039,11 @@
     baseForOwn(LazyWrapper.prototype, function(func, methodName) {
       var lodashFunc = lodash[methodName];
       if (lodashFunc) {
-        var key = (lodashFunc.name + ''),
-            names = realNames[key] || (realNames[key] = []);
+        var key = (lodashFunc.name + ''), names;
+		if (realNames.hasOwnProperty(key))
+			names = realNames[key];
+		else
+            names = realNames[key] = [];
 
         names.push({ 'name': methodName, 'func': lodashFunc });
       }

please help?

@christroutner
Copy link
Contributor

christroutner commented May 2, 2019

I tried recreating the bug. Thank you for the concise code example.

This is the code I tried first and everything ran perfectly:

// Used for debugging and iterrogating JS objects.
const util = require("util");
util.inspect.defaultOptions = {depth: 1};

const SLP = require('slp-sdk')
var slp = new SLP()

let val

// Creates a Buffer
val = slp.Mnemonic.toSeed('')

// Converts the Buffer to a string
val = val.toString('hex')

console.log(`val: ${util.inspect(val)}`)
// val: '4ed8d4b17698ddeaa1f1559f152f87b5d472f725ca86d341bd0276f1b61197e21dd5a391f9f5ed7340ff4d4513aab9cce44f9497a5e7ed85fd818876b6eb402e'

let ret = val.substr(0,16)

console.log(`ret: ${util.inspect(ret)}`)
// ret: '4ed8d4b17698ddea'

It was only when I added this code at the top, that the bug you're reporting manifested itself:

const jsp = require('js-prototype-lib');
jsp.install()

I checked out the npm entry for js-prototype-lib and it says right at the top:

The argument could be made that its use is unadvisable within browsers as your mileage may vary due to the instability of these implementations...

I've never used the js-prototype-lib and I haven't heard of any reports of any other users using this library or having an issue with lodash.

The obvious solution to me, seems to be avoiding the use of this library.

@christroutner
Copy link
Contributor

I ran npm list lodash. Lodash is not a direct dependency of slp-sdk. It's a dependency of a dependency. There isn't anything we can do within the slp-sdk repo to fix this issue.

@ekkis
Copy link
Author

ekkis commented May 10, 2019

I've submitted a patch to lodash that fixes this problem and it has been accepted: lodash/lodash#4290

I've also requested of the Babel team to upgrade to the fixed version of lodash: babel/babel#9966

when that team upgrades I'll need you guys to upgrade

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

No branches or pull requests

2 participants