List.find hides Array.prototype.find

[gormster]

Bug report

Fiddle

https://jsfiddle.net/gormster/72jkzpy4/5/

Explanation

Expected behaviour

svg.js methods that return List, an extension of the builtin Array, should not hide any existing functions that are declared on Array.

Actual behaviour

find() and its friends that return List hide any methods that share a name with a method in Array.prototype. The one I'm currently being frustrated by is find, but there could be others.

The documentation for SVG.List states:

A list has access to all buildin array methods

This is clearly not the case.

What should be done?

Clearly some kind of resolution needs to occur to avoid breaking changes. While I would say the correct thing to do is not spray new methods all over the prototype for List without checking if they already exist, and perhaps just make it a standard practice to call .each, that's obviously a major change and would have to go in something like a 4.0.

Might I suggest making a property like .array available on List, which simply allows us to call through to the base functions defined by Array?

[Fuzzyma]

Calling each every time would take away the whole point of list and you could just use normal arrays. I agree that it is not optimal that it hides an array function here. That was clearly overlooked. In the next revision this will be solved by using proxies and prefixing all array methods with $ or something along the line (or whatever feels best at that point).

For now your solution seems viable. However array also clashes. Other ideas? native would work 🤔

[gormster]

Hmm. Prefixing the native methods wouldn't be a breaking change, right? As long as it only prefixes when there's a clash. (In fact, you don't even need to worry about that, because it doesn't matter if $map and map are the same function.)

Something like (in List.js):

List.extend = function (methods) {
  methods = methods.reduce((obj, name) => {
    // Don't overwrite own methods
    if (reserved.includes(name)) return obj

    // Don't add private methods
    if (name[0] === '_') return obj

    // Allow access to original Array methods through a prefix
    if (name in Array.prototype) {
      obj['$' + name] = Array.prototype[name]
    }

    // Relay every call to each()
    obj[name] = function (...attrs) {
      return this.each(name, ...attrs)
    }
    return obj
  }, {})

  extend([ List ], methods)
}

native or builtin are unlikely to clash with any plugin methods; nor are $ or _ (I think).

[Fuzzyma]

Doing it like this wouldn't introduce a breaking change. I kinda like it. The question is, if people like to call the native array methods with $. But as an escape hatch, it might just work.

There is one thing though: calling List.extend will add the array methods every time. Its not breaking anything. Just extra work. Its a minor thing so. Afaik, extend is only called once and might be called by a user a second time so its neglictable