EXTEND_PROTOTYPES false by default.

[jdalton]

Currently Ember extends built-in prototypes of Array, Function, & String by default. Although Ember doesn't require these extensions and they can be disabled, because they are enabled by default they tend to stay that way as evident by sites like vine, twitch, nbcnews, & discourse using the default settings.

The problem with adding methods like Array#compact, Array#without, Array#uniq, Function#on, & String#capitalize is that it increases the likelihood that they'll be used and cause problems/roadblocks for future built-in language extensions. For example, if ES7 tried to add Array#without that behaved in a different way than Ember's Array#without newer environments would get the built-in version while older would get Ember's causing an inconsistency and increased possibility of breaking thing. We've seen this recently with MooTools and Array#contains, but it's happened before with Function#bind, String#contains, and even DOM4 Element#remove.

[stefanpenner]

@jdalton as you know, internal ember does not rely on these things. So we have lots of flexibility here.

I am unsure if they should be default on or default off, that is a @wycats and @tomdale question.

But I do think if they are ON, we should always use our user land versions and never fall back to native versions. That way if something does change, our users will still get the consistent original behavior they expected.

[workmanw]

I would also add that there are a lot of blog posts, Stack Overflow answers, JSBins and even Ember guides that currently show code that assumes EXTEND_PROTOTYPES set to true. It's not that changing false to true is hard, but it would effectively invalidate 90% of the examples that exist and could ultimately be another point of confusion for newer users.

[stefanpenner]

An added confusion is we have Enumerable which allows enumeration over non array like objects but with all the array like goodies.

This leaves us with potential differences with [].contains and enumerableThing.contains. Which sucks big time.

This part of ember was inherited from the sprout core days, a time when browser world was stale, prototype extensions where used without fear (wrongfully). Going forward this is in the category of cruft I wish to see improved. As such a strategy and plan needs to be derived.

[stefanpenner]

I think Ember.A() behaving ala lodash might be the way forward. Ultimately, just using a small subset of lodash may be useful, but we are very nervous to explode our gzip'd bite size even more.

[tomdale]

I'd be willing to reconsider it once the way you consume Ember is via ES6 modules, and in a 2.x release, but this change would be way too breaking for the 1.x series.

[stefanpenner]

Also worth noting. In theory currently ember will run slower with prototypes off. And will be an involved but good refactoring to address this

[jdalton]

I think Ember.A() behaving ala lodash might be the way forward.

👍

Also worth noting. In theory currently ember will run slower with prototypes off.

Ah, good to know. Manually bolting on methods reminds me of Prototype's approach for supporting DOM extensions in older browsers (not so awesome).

There's deeper issues as it looks like even with a config of EXTEND_PROTOTYPES as false new built-ins can still leak through to Ember.A(...) which can lead to the same issue described above. Here's a simple example of that.

[stefanpenner]

@jdalton yes, we need to be consistent about this. The current state is the result of much scenario solving but we a more holistic set of solutions.

Currently ember always falls back to existing natives, but clearly we don't actually want this. As it may result in future breakages.

if it wasn't for Ember's Enumerable, I would opt immediately to drop our version of this and move to a minimal lodash.

[totaltrek]

+1

Extending native prototypes is always dangerous and usually avoidable. I haven't yet used Ember, and this is the main reason why. I want my apps to gracefully upgrade as well as they gracefully degrade.

It seems to me that the reason prototypes are extended inside Ember is for syntactic sugar, which, let's be honest, is a fantastic goal. I just worry that it's following such a bad practice that the project itself will suffer in the long term.

As an example, let's look at Function#property -- the sugar that lets us "allow you to treat a function like a property" (quoting the docs). If ECMA version 7 defines a radically different #property on the native Function prototype, all ember apps risk immediate and permanent failure without crisis-mode maintenance. Instead of attaching #property why is the sugar not defined as follows?

App.model = Ember.Object.extend{
    oneAttr: 'a string',
    anotherAttr: [ 'an', 'array' ],
    someAttr: Ember.Property( function () {
        return this.anotherAttr.reverse().concat( this.oneAttr ).reverse()
    }, this )
}
--
> model.someAttr();
'a string an array'

This solves at least three problems: one, interfering with the native prototype; two, replacing sugar with safety and consistency; three, I'm not using Ember yet. The only problem remaining is the dusty example code on stackoverflow, random blogs, codeschool, and the like; most of which is already behind if not broken since it was published.

PLEASE NOTE: I'm not suggesting Ember.Property( function ) as an implementation target here. That is pseudocode provided merely to illustrate a potential solution. Looking at ember-runtime/lib/ext/function.js it seems that Function#property might be fairly easily portable to a new Property class in the Ember namespace.

[OrKoN]

@totaltrek

I think what you suggest is already there (see Ember.computed):

var Person = Ember.Object.extend({
  firstName: 'Betty',
  lastName: 'Jones',

  fullName: Ember.computed('firstName', 'lastName', function(key, value) {
    return this.get('firstName') + ' ' + this.get('lastName');
  })
});

So one can easily avoid using prototype extensions. And projects such as ember-watson can convert your existing code if you wish: https://github.com/abuiles/ember-watson#ember-watsonconvert-prototype-extensions. Also see this emberjs/guides#110

[stefanpenner]

So we will be making an effort during the 2 -> 3x lifespan to mitigate the performance issues caused by totally disabling prototype extensions (which is one of the main blockers). There are several ergonomic things that will need to be improved as well, but nothing very hard just laborious.

I have also thrown together a first pass at updating the guides to prefer non-prototype extension modes for the 2 most common ones. emberjs/guides#110

A future effort will dig into the array extensions (which are the most gnarly, and without them the current internals will suffer performance issues, but this can be mitigated)

[totaltrek]

Thank you @OrKoN - I hadn't yet gotten familiar enough with Ember to track those docs down. Much obliged.

That's a huge help, @stefanpenner - thanks. It would have made reading the docs a much more exciting venture because it's obvious that these methods don't extend native prototypes.

As far as arrays are concerned, I'm not at all surprised at the pushback on removing the Array extensions. However, there's already a historical example for everybody to already use A([ ]) when they need an Ember-extended array, and that's the crash-and-burn of the early versions of Prototype.js, versus its much smarter (at the time at least) competitor, lodash.

True, forcing the A() call makes each of us work harder to do the same thing we'd currently do with a generic array, but wouldn't it make Ember apps more performant to not rely on this antipattern? If we only wrap our arrays when we know they're going to need extended methods, then any generic array can be left as-is, and suffer no performance drags due to extensions.

What parts of the internals will suffer from performance issues should the Array extensions be removed?

[stefanpenner]

What parts of the internals will suffer from performance issues should the Array extensions be removed?

Ya, this isn't entirely obvious but let me explain.

given the following:

{{#each friends as  |friend}}

Ember essentially does (yes a liberal simplification):

1. instantiate a collection view
2. the collection view subscribes to the friends array change notifications
3. ... renders ...

turns out 2. is the offender, as ember assumes friends is a prototype extended array or an ember array like (array controller/proxy etc). The code is something like array.addArrayObserver(...) which forces Ember.A() usage to ensure the prototype extensions.

If EXTEND_PROTOTYPES === true, Ember.A is essentially a no-op, but if it is false Ember.A applies all the array extension methods to each array instance it produces. This part is extremely slow.

I believe the resolution to this issue is simple, although it will require some grunt work. Ember internals, should not rely on addArrayObserver and the rest of the observable related methods existing on the prototype, rather they should take the same approach used for set/get.

To mitigate the need for all objects having get/ set ember internals do the following:

get(obj, 'property');
set(obj, 'property', 'value')

rather then:

obj.get('property');
obj.set('property', value);

This pattern can and likely should by applied to the observable array methods as well.

addArrayObserver(array, ...);
removeArrayObserver(array, ...);
pushObject(array, obj);

Yes this still leaves the other "nice things" and es6/es7 polyfils, but it reduces the cost and internal domain leakage the observable methods on arrays currently cause.

[stefanpenner]

Also, it is worth noting. ember-addons as of recently actually run with prototype extensions disabled in their default dummy app harness. This ensure add-ons written are compatible with both modes, and likely a more sensible default going forward.

Once we unblock ^ (i think i motivated to work on it this weekend), the performance concerns should be mostly mitigated. And we can pave they way for a future of more well behaved ember apps.

[stefanpenner]

i started work on this (just now) #10899

[rwjblue]

Tracking in #10899 for the array scenarios. We will still need to deprecate the usage of the various function prototype extensions, but that should be relatively simple to do (will need to be supported throughout the 2.x timeframe though).

[stefanpenner]

With #10899 it will be easier and of less cost for people to do this opt-in. We can even strongly encourage it.

[locks]

As mentioned, prototype extension is turned off by default in addons. Given this and the effort to not use extensions in Ember's source as well as carefully deprecating some of them, I'm closing this issue. Thanks for the discussion everyone, let's keep working towards a better story!

[runspired]

I haven't shipped an app with them on in forever. It's likely that we can full swap on the default for 3.0

[stefanpenner]

I haven't shipped an app with them on in forever. It's likely that we can full swap on the default for 3.0

Those apps will suffer from perf issues, until the array .replace and didChange hooks stuff is sorted out.

[bartocc]

I am curious about the perf issues you were mentioning @stefanpenner, now that we have Octane, the new reactivity system, ES6 modules, etc… 🤔

Is it still usefull to have EXTEND_PROTOTYPES turned on by default in Ember apps?

[rwjblue]

Is it still usefull to have EXTEND_PROTOTYPES turned on by default in Ember apps?

It is still somewhat important for array in certain contexts. It depends how far down the Octane path really, you could author without ever using Ember.A() (and without using .pushObject and friends). I'm just not sure that most folks actually do author that way.