Support ES6 method-shorthand constructors in extend (#4213)

[BigBalli]

Fixes #4213.

Problem

ES6 method shorthand (constructor() {}) produces a non-constructable
function with no prototype property. Passing such a constructor to
extend previously caused new Subclass() to throw
Subclass is not a constructor:

const MyModel = Backbone.Model.extend({
  constructor() {
    Backbone.Model.apply(this, arguments);
  }
});
new MyModel(); // TypeError: MyModel is not a constructor

This forces users on modern JS to write the awkward mixed form
(constructor: function() {} next to method shorthand) or to give up on
extend altogether.

Fix

Detect the shorthand case by checking for the absence of prototype on
the supplied constructor, and in that case wrap it in a thin function so
new continues to work. The traditional constructor: function() {}
form is unchanged and still uses the supplied function directly, so the
edge case noted in the issue (Model === proto.constructor) remains
true for ES5 users.

Test

Added a regression test in test/model.js that defines the
constructor via ES6 method shorthand (constructed via eval so the
test file still parses on engines without shorthand support) and
verifies that new Model({foo: 'bar'}) returns a usable instance.

npm run lint passes.

[jgonggrijp]

I can imagine making this check a bit more precise, in one of the following ways:

Suggested change

	if (supplied.prototype) {
	if (_.has(supplied, 'prototype')) {

Suggested change

	if (supplied.prototype) {
	if (typeof supplied.prototype === 'object') {

(combination of both)

Suggested change

	if (supplied.prototype) {
	if (_.has(supplied, 'prototype') && typeof supplied.prototype === 'object') {

Can we think of situations where the distinction might be important enough that one of the more precise versions might be preferred? I can't quite put my finger on it, but I have a feeling that it might be important that the prototype is an own property of constructor, which would warrant the _.has check.

CC @GammaGames @Yahasana

[BigBalli]

Good catch. Would be good to switch to _.has(supplied, 'prototype').

Walking the realistic inputs (classic functions, classes, shorthand, arrow/async/generator), the truthy check and _.has behave identically. Own-vs-inherited only diverges if someone does Object.setPrototypeOf on a shorthand method, which isn't a real user path. But _.has expresses the intent directly ("supplied a function with its own prototype slot") instead of leaning on truthiness as a proxy, so it's the better line at the same cost.

We could skip the typeof === 'object' variant. The only way to fail it with a truthy prototype is manual assignment like Fn.prototype = 42, which is sabotage either way.