montagejs / collections

This package contains JavaScript implementations of common data structures with idiomatic interfaces.
http://www.collectionsjs.com
Other
2.09k stars 185 forks source link

shim-array find() doesn't use the same params as the canonical one. #182

Closed Jorenm closed 6 years ago

Jorenm commented 7 years ago

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/find?v=control

Also, shouldn't a shim check if it exists before it replaces it?

This took up maybe 5 hours of debugging till I dug into the right node_module to find its functionality broken because it expected the canonical .find functionality :(

uglow commented 7 years ago

Here's the dependency tree that I had that ended up leading me here:

├─┬ dgeni-packages@0.16.1
│ ├─┬ catharsis@0.8.8
│ │ └─┬ underscore-contrib@0.3.0
│ │   └── underscore@1.6.0
│ ├─┬ change-case@3.0.0
│ │ ├── camel-case@3.0.0
│ │ ├── constant-case@2.0.0
│ │ ├── dot-case@2.1.1
│ │ ├── header-case@1.0.1
│ │ ├── is-lower-case@1.1.3
│ │ ├── is-upper-case@1.1.2
│ │ ├── lower-case@1.1.4
│ │ ├── lower-case-first@1.0.2
│ │ ├── no-case@2.3.1
│ │ ├── param-case@2.1.1
│ │ ├── pascal-case@2.0.1
│ │ ├── path-case@2.1.1
│ │ ├── sentence-case@2.1.1
│ │ ├── snake-case@2.1.0
│ │ ├── swap-case@1.1.2
│ │ ├── title-case@2.1.1
│ │ ├── upper-case@1.1.3
│ │ └── upper-case-first@1.1.2
│ ├── espree@2.2.5
│ ├── estraverse@4.2.0
│ ├─┬ glob@7.1.2
│ │ ├── fs.realpath@1.0.0
│ │ └── inflight@1.0.6
│ ├─┬ htmlparser2@3.9.2
│ │ ├── domelementtype@1.3.0
│ │ ├── domhandler@2.4.1
│ │ ├─┬ domutils@1.6.2
│ │ │ └─┬ dom-serializer@0.1.0
│ │ │   └── domelementtype@1.1.3
│ │ ├── entities@1.1.1
│ │ └─┬ readable-stream@2.2.11
│ │   ├── core-util-is@1.0.2
│ │   ├── process-nextick-args@1.0.7
│ │   ├── safe-buffer@5.0.1
│ │   ├── string_decoder@1.0.2
│ │   └── util-deprecate@1.0.2
│ ├── marked@0.3.6
│ ├─┬ minimatch@3.0.4
│ │ └─┬ brace-expansion@1.1.8
│ │   ├── balanced-match@1.0.0
│ │   └── concat-map@0.0.1
│ ├── node-html-encoder@0.0.2
│ ├── nunjucks@2.5.2
│ ├─┬ q-io@1.13.4
│ │ ├─┬ collections@0.2.2          <----
│ │ │ └── weak-map@1.0.0
│ │ ├── mime@1.3.6
│ │ ├── mimeparse@0.1.4
│ │ ├── qs@6.4.0
│ │ └── url2@0.0.0

Even though this is an old version of collections, @Jorenm 's feedback is valid.

jh3141 commented 6 years ago

+1. Changing existing standardised behavior of core objects is a very bad idea, particularly if the change is documented as not happening. The documentation states:

In version 1, this method is called find, which conflicts with the definition of find provided by ECMAScript 6. In version 2, this method is called findValue to eliminate the conflict.

As I have the most current version available in NPM (5.0.7, according to my package.json file), I would have assumed that I'd get the version 2 behaviour, but apparently it has been reverted to the behaviour of version 1 for some reason.

mattandrews commented 6 years ago

I too have just had several issues due to this library's poorly-named functions – with great power comes great responsibility(!).

hthetiot commented 6 years ago

@mattandrews Can you try npm install collections@2.0.3 and tell me if that fix your issue. I'm going to merge v2 branch in master and release v6 soon.

V3 to v5 was released with legacy shim and find override instead of findValue.

hthetiot commented 6 years ago

Duplicate #185

mattandrews commented 6 years ago

Hi, thanks for the update. Unfortunately my issue was with a library several steps upstream from collections, so even if the issue is fixed here I need to get two further projects to update their dependencies (eg. see the explanation here).

Since commenting here I migrated my code away from the original tools I was trying to use as the effect this was having on my codebase was unmanageable (eg. uses of Array.find() within my app were broken).

codebling commented 5 years ago

See also issues #36 #70 #94 #95 #116 #139 #145 #162 #165 #169 #178 #182 #185 #197 #215 #220 and PRs #94 #95 #116 #173 #189 #212. As mentioned, branch v2 fixes these issues by avoiding global object modification.