Update Array.prototype.flatten to match TC39 proposal #2797

callmevlad · 2018-03-07T06:07:28Z

Array.prototype.flatten and Array.prototype.flatMap are already on Stage 3 (Candidate) of the TC39 standards process and are starting to land in browsers.

There's currently a discussion happening here around avoiding the flatten name due to the conflict with Mootools core, which was a response to this Bugzilla report.

Since flatten is the commonly accepted name for this type of operation, it seems unreasonable for the name to change for the entire spec to something more esoteric just because of the Mootools conflict.

Can this library be modified to match the upcoming TC39 standard for Array.prototype.flatten? 🙏

The text was updated successfully, but these errors were encountered:

cpojer · 2018-03-07T06:58:02Z

It could be modified but MooTools isn't installed through npm (it didn't exist when MooTools was popular) and websites that still use it today aren't likely to update. Updating the code in this repo will not resolve your issue with Array.prototype.flatten unless you also find a way to update every (unmaintained) legacy website that still uses MooTools.

gkatsanos · 2018-03-07T07:50:07Z

It's a start?

cpojer · 2018-03-07T08:09:37Z

Sure, feel free to send a PR with a fix. However, please also consider this tweet for more perspective: https://twitter.com/tomdale/status/971288054966247430

I am going to venture a guess that most of the sites that will break are not using MooTools 1.3 to 1.6 but rather 1.1 or 1.2. These sites haven't been updated in more than half a decade and it's unlikely that they will even if you make a change to the code in this repository.

timwienk · 2018-03-07T08:13:55Z

That being said, MooTools' aim has always been to help improve the JavaScript landscape, and I'd like to think we really did help do that.

Apart from the prototype extension wars, I believe the fact that MooTools libraries are not (and should not) be used for new projects anymore (and therefore only need maintenance - of which it doesn't need a lot) is because the libraries are not needed anymore: most of what MooTools wanted to bring to JavaScript has been implemented natively.

In my opinion, the worst thing to do is to settle for a bad name for a JavaScript API, because a JavaScript library decided to use it first.

API should be decided on because of how intuitive it is, which means it should at least be logical, easy to understand and if possible in line with established names in other languages.

Anyway -

I'm sure we can look into updating this API on the MooTools end, but like @cpojer said, it won't really change what happens to old projects.

It should work in such a way - but I'll have to double check if that goes for all versions (do you know off the top of your head @cpojer ?) - that old versions of MooTools Core just overwrite a native method if it exists. For a new version, it should then only place the method (with new API) if it doesn't exist.

This way old projects continue to work fine (since their code will expect MooTools Core's flatten and will get it), new projects (with an updated MooTools Core) will have the new method.

I believe the only difference with MooTools Core's flatten is the depth argument. MooTools Core flattens recursively, while the TC39 proposal only goes up to depth, which defaults to 1. Is this API final?

cpojer · 2018-03-07T08:17:46Z

@timwienk it's not final yet, it could still be changed. depth=1 is a sensible default, but in light of this TC39 may want to reconsider setting it to Infinite.

I think the best fix would be to delete Array.prototype.flatten at the top of Array.js in MooTools.

timwienk · 2018-03-07T08:19:58Z

I think the best fix would be to delete Array.prototype.flatten at the top of Array.js in MooTools.

I don't think we even have to. Old versions should overwrite (still have to check). For new versions we'd want to use the final API, I guess. (Like we did with all the ES5 methods, years ago.)

appsforartists · 2018-03-07T08:41:29Z

Any gauge of how big the window is (if any) where MooTools would check for flatten and use the built-in if it exists?

bakkot · 2018-03-07T16:56:09Z

@timwienk

old versions of MooTools Core just overwrite a native method if it exists. For a new version, it should then only place the method (with new API) if it doesn't exist.

That's actually backwards: new versions overwrite unconditionally, whereas old versions check if the method exists first.

But this also appears not to be the issue. Rather, the issue is probably in how MooTools is setting up methods on Elements.prototype, including up to the present. See here. As a consequence, if MooTools adds any method on Array.prototype, then it will break any MooTools users if TC39 adds a method of the same name even if it behaves identically to how MooTools did it (since we will not add an enumerable method to Array.prototype). The fact that .flatten does not behave identically here was actually a red herring, though that difference would probably also break some sites.

That really sucks. There's probably nothing to be done about it - MooTools is too widely deployed, now - but I think it would still be good to change how MooTools sets up at least Elements.prototype so that this doesn't happen.

kentaromiura · 2018-03-12T14:33:15Z

@timwienk that is exactly how @arian solved String#contains back in 1.2.6:
7057039
@bakkot it's a known issue that IIRC goes way back before enumerability was a thing in JS; core has a force method to work around that.
If no one else wants to do it I can send a PR for 1.2.7; a compat+1.6.x branch can be done once the behavior is finalized.

mathiasbynens · 2018-03-19T09:43:14Z

IIUC, the problem is not the difference in implementation; MooTools overrides Array.prototype.flatten if it’s natively available.

The problem is that, when a non-enumerable Array.prototype.flatten is available (e.g. because a browser ships it natively), then MooTools assigning to it (i.e. Array.prototype.flatten = mooToolsImplementation) doesn’t magically make it enumerable, meaning MooTools won’t copy it over to Element.prototype.flatten. Code relying on Element.prototype.flatten now breaks.

Update: Ah, @bakkot already said this.

ljharb and MatthiasPortzel reacted with thumbs up emoji

ljharb · 2018-03-19T09:45:11Z

The better solution for MooTools (if this is still the approach in the current version) might be to hardcode its list of methods (on Array and Elements .prototype) rather than to infer the list from the objects at runtime?

svieira · 2018-03-19T17:06:40Z

As a slightly-different-form-of-horrible what if setting these methods without a descriptor made them enumerable by default? (E. g. monkey-patching of built-in prototypes as a signal to the JS-engine to revert to ES3 semantics). We probably don't want to do this for all objects, just the built-in ones.

Especially since Mootools isn't checking & using, but assigning to a "known-good" implementation always (horray for doing the sensible thing), that would solve these cases (flatten, contains, etc.).

Jessidhia · 2018-03-20T01:14:22Z

That would also mean that polyfills would have to either replace the prototype with a Proxy, or would have to check and replace every single entry in the prototype with a getter/setter pair. 😱

callmevlad mentioned this issue Mar 7, 2018

rename flatten to smoosh tc39/proposal-flatMap#56

Closed

js-choi mentioned this issue Mar 7, 2018

flatten shallow by default tc39/proposal-flatMap#9

Closed

willheslam mentioned this issue Jan 11, 2019

Enhance _isFunction To Handle AsyncFunction (+ more) ramda/ramda#2513

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Update Array.prototype.flatten to match TC39 proposal #2797

Update Array.prototype.flatten to match TC39 proposal #2797

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!