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

collections break crypto #196

Open
codebling opened this issue Feb 15, 2018 · 5 comments
Open

collections break crypto #196

codebling opened this issue Feb 15, 2018 · 5 comments
Assignees
Milestone

Comments

@codebling
Copy link

codebling commented Feb 15, 2018

Opening this since most of related issues #36 #70 #94 #95 #116 #139 #145 #162 #165 #169 #178 #182 #185 #197 #215 #220 have been closed and there needs to be some visibility on this.

The shims are still causing issues in collections.

I see that PRs #95 #173 and #189 #212 are still open, the merge of one or both of those should theoretically resolve this issue. (Closed PRs #94 #116 are also about the same issue)

Try this simple test case.

$ node
> require('crypto').getHashes().length
46
>
(To exit, press ^C again or type .exit)
>
$ node
> var Map = require('collections/map')
undefined
> require('crypto').getHashes().length
0
>
@hthetiot
Copy link
Contributor

@codebling Thanks will look into #173 and #189 ASAP.

@hthetiot
Copy link
Contributor

hthetiot commented Feb 16, 2018

Works for me @codebling have you installed [email protected] ?

package.json source:

{
  "name": "collections-test",
  "main": "array.js",
  "dependencies": {
    "collections": "^5.1.2"
  }
}

array.js source:

var Map = require('collections/map');
console.log(require('crypto').getHashes().length);

Results:

node array.js 
46

Some other tests

nvm use v4
Now using node v4.8.6 (npm v2.15.11)
node 
> var Map = require('collections/map')
undefined
> require('crypto').getHashes().length
46
nvm use v8
Now using node v8.9.0 (npm v5.5.1)
node 
> var Map = require('collections/map')
undefined
> require('crypto').getHashes().length
0

Bug is on node v8 not v4.

@hthetiot hthetiot changed the title collections break crypto collections break crypto on node v8 Feb 16, 2018
@hthetiot hthetiot changed the title collections break crypto on node v8 collections break crypto on node v5+ Feb 16, 2018
@hthetiot hthetiot changed the title collections break crypto on node v5+ collections break crypto Feb 16, 2018
@codebling
Copy link
Author

Thanks for looking at this. Did not realise it was not broken on some node versions.

@hthetiot hthetiot added the wip label Mar 9, 2018
@rachel-chocron
Copy link

Any progress with this?
I am experiencing this issue when using http://www.collectionsjs.com/sorted-set with winston file transport. It prevents me from creating a file transport.

@codebling
Copy link
Author

codebling commented Apr 15, 2019

@rachel-chocron Based on comments in all of the issues listed at the top, the root cause of all of the issues is the direct modification of the global array type by Collections. If I recall correctly, this was fixed in the v2 branch, which at the moment is not merged or supported by the montage team.

In this specific issue, hthetiot showed that it does actually work in some legacy versions of node, but later ones all seem to be broken.

Depending on which features of Collections you use, there may be acceptable substitutes available.

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

No branches or pull requests

3 participants