New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for extending builtins #7020

Merged
merged 10 commits into from Dec 20, 2017
Next

Add support for extending builtins

  • Loading branch information...
nicolo-ribaudo committed Dec 12, 2017
commit b7697f426943b4fa03d0c45bb7bf63506a298573
@@ -425,6 +425,46 @@ helpers.inheritsLoose = defineHelper(`
}
`);
// Based on https://github.com/WebReflection/babel-plugin-transform-builtin-classes
helpers.wrapNativeSuper = defineHelper(`
var _gPO = Object.getPrototypeOf || function _gPO(o) { return o.__proto__ };

This comment has been minimized.

@Kovensky

Kovensky Dec 14, 2017

Member

This breaks the workaround in #3527 but there's no hope of extending builtins working if the workaround is necessary anyway 🤷‍♀️

@Kovensky

Kovensky Dec 14, 2017

Member

This breaks the workaround in #3527 but there's no hope of extending builtins working if the workaround is necessary anyway 🤷‍♀️

This comment has been minimized.

@WebReflection

WebReflection Dec 14, 2017

this patch is explicitly incompatible with IE <= 10 where you should not extend native builtins

@WebReflection

WebReflection Dec 14, 2017

this patch is explicitly incompatible with IE <= 10 where you should not extend native builtins

This comment has been minimized.

@trusktr

trusktr Jan 31, 2018

But, if we're writing new code that extends builtins...?

@trusktr

trusktr Jan 31, 2018

But, if we're writing new code that extends builtins...?

This comment has been minimized.

@WebReflection

WebReflection Jan 31, 2018

then you are responsible to make such code work as expected otherwise you should stop using JavaScript since even Object could be overwritten 😄

@WebReflection

WebReflection Jan 31, 2018

then you are responsible to make such code work as expected otherwise you should stop using JavaScript since even Object could be overwritten 😄

This comment has been minimized.

@trusktr

trusktr Jan 31, 2018

Gotcha, let me stop using JS on my next web app!

@trusktr

trusktr Jan 31, 2018

Gotcha, let me stop using JS on my next web app!

var _sPO = Object.setPrototypeOf || function _sPO(o, p) { o.__proto__ = p };

This comment has been minimized.

@jridgewell

jridgewell Dec 20, 2017

Member

Bug! This fallback does not return o.

@jridgewell

jridgewell Dec 20, 2017

Member

Bug! This fallback does not return o.

This comment has been minimized.

@nicolo-ribaudo

nicolo-ribaudo Dec 20, 2017

Member

Whoops, good catch!

@nicolo-ribaudo

nicolo-ribaudo Dec 20, 2017

Member

Whoops, good catch!

var _construct = Reflect.construct || function _construct(Parent, args, Class) {

This comment has been minimized.

@xtuc

xtuc Dec 13, 2017

Member

Shouldn't we check for Reflect first?

@xtuc

xtuc Dec 13, 2017

Member

Shouldn't we check for Reflect first?

This comment has been minimized.

@WebReflection

WebReflection Dec 13, 2017

This comment has been minimized.

@nicolo-ribaudo

nicolo-ribaudo Dec 13, 2017

Member

I'll use (typeof Reflect === "object" && Reflect.construct) || function ... instead of the ternary (which is used in the original plugin) since someone might only polyfill some Reflect methods, so Reflect.constructor could be not defined.

@nicolo-ribaudo

nicolo-ribaudo Dec 13, 2017

Member

I'll use (typeof Reflect === "object" && Reflect.construct) || function ... instead of the ternary (which is used in the original plugin) since someone might only polyfill some Reflect methods, so Reflect.constructor could be not defined.

var Constructor, a = [null];
a.push.apply(a, args);
Constructor = Parent.bind.apply(Parent, a);
return _sPO(new Constructor, Class.prototype);
};
var _cache = typeof WeakMap === "function" && new WeakMap();
export default function _wrapNativeSuper(Class) {
if (_cache) {

This comment has been minimized.

@xtuc

xtuc Dec 13, 2017

Member

Could this check be more explicit? typeof x === 'undefined'

@xtuc

xtuc Dec 13, 2017

Member

Could this check be more explicit? typeof x === 'undefined'

if (_cache.has(Class)) return _cache.get(Class);
_cache.set(Class, Wrapper);
}
function Wrapper() {}
Wrapper.prototype = Object.create(Class.prototype, {
constructor: {
value: Wrapper,
enumerable: false,
writeable: true,
configurable: true,
}
});
return _sPO(
Wrapper,
_sPO(
function Super() {
return _construct(Class, arguments, _gPO(this).constructor);
},
Class
)
);
}
`);
helpers.instanceof = defineHelper(`
export default function _instanceof(left, right) {
if (right != null && typeof Symbol !== "undefined" && right[Symbol.hasInstance]) {
@@ -119,3 +119,37 @@ class Bar extends Foo {
When `Bar.prototype.foo` is defined it triggers the setter on `Foo`. This is a
case that is very unlikely to appear in production code however it's something
to keep in mind.
### `builtins`
`Array<string>`, defaults to `[]`.
When extending a native constructor (e.g., `class extends Array {}`), it needs
to be wrapped.

This comment has been minimized.

@justinfagnani

justinfagnani Dec 13, 2017

Contributor

A couple of changes I'd reccomend:

  • Change "constructor" to "class" (since the key distinction here is constructors that can't be called with Function.apply, which are class constructors) and mention a little about w
  • Clarify what needs to be wrapped: the subclass or the native class
  • Add just a little hint on why the wrapper is necessary

"When extending a native class (e.g., class extends Array {}), the native class needs to be wrapped in order to support Babel's emulation of super() calls."

@justinfagnani

justinfagnani Dec 13, 2017

Contributor

A couple of changes I'd reccomend:

  • Change "constructor" to "class" (since the key distinction here is constructors that can't be called with Function.apply, which are class constructors) and mention a little about w
  • Clarify what needs to be wrapped: the subclass or the native class
  • Add just a little hint on why the wrapper is necessary

"When extending a native class (e.g., class extends Array {}), the native class needs to be wrapped in order to support Babel's emulation of super() calls."

Babel only recognizes built-in JavaScript functions defined in the ECMAScript

This comment has been minimized.

@justinfagnani

justinfagnani Dec 13, 2017

Contributor

Does this option override or augment the default list? If it augments, I'd consider making that more clear in the option name, like additionalBuiltins.

@justinfagnani

justinfagnani Dec 13, 2017

Contributor

Does this option override or augment the default list? If it augments, I'd consider making that more clear in the option name, like additionalBuiltins.

specification. If you need to extend other classes, you can define them using
this option.
#### Compatibility
This transformer works on IE11 and every other browser with `Object.setPrototypeOf` or `__proto__` as fallback.
There is **NO IE <= 10 support**. If you need IE <= 10 don't it's recommended to not don't extend natives.

This comment has been minimized.

@WebReflection

WebReflection Dec 12, 2017

wow, I made it super clear there 😂

If you need IE <= 10 don't it's recommended to not don't extend natives.

@WebReflection

WebReflection Dec 12, 2017

wow, I made it super clear there 😂

If you need IE <= 10 don't it's recommended to not don't extend natives.

This comment has been minimized.

@nicolo-ribaudo

nicolo-ribaudo Dec 12, 2017

Member

I removed the don't use this plugin bit and screwed it up 😆

@nicolo-ribaudo

nicolo-ribaudo Dec 12, 2017

Member

I removed the don't use this plugin bit and screwed it up 😆

This comment has been minimized.

@xtuc

xtuc Dec 13, 2017

Member

Also note that IE is not official supported by Babel (according to https://github.com/babel/babel/blob/master/doc/design/compiler-environment-support.md)

@xtuc

xtuc Dec 13, 2017

Member

Also note that IE is not official supported by Babel (according to https://github.com/babel/babel/blob/master/doc/design/compiler-environment-support.md)

This comment has been minimized.

@mgol

mgol Dec 20, 2017

@xtuc You mean that the compiler doesn't work IE, don't you? I surely hope Babel itself still supports IE...

@mgol

mgol Dec 20, 2017

@xtuc You mean that the compiler doesn't work IE, don't you? I surely hope Babel itself still supports IE...

This comment has been minimized.

@Andarist

Andarist Dec 12, 2017

Member

grammar "to not don't"

@Andarist

Andarist Dec 12, 2017

Member

grammar "to not don't"

This comment has been minimized.

@xtuc

xtuc Dec 13, 2017

Member

It sounds strange to me, could we reword it to something like:

We don't recommend to extend native classes under IE (version <= 10) because it can cause undefined behaviors.

@xtuc

xtuc Dec 13, 2017

Member

It sounds strange to me, could we reword it to something like:

We don't recommend to extend native classes under IE (version <= 10) because it can cause undefined behaviors.

#### Example
```json
{
"plugins": [
["@babel/transform-classes", {
"builtins": [ "HTMLElement" ]
}]
]
}
```
```js
class MyCustomElement extends HTMLElement {
// ...
}
```
@@ -0,0 +1,35 @@
export default new Set([
"Array",
"ArrayBuffer",
"Boolean",
"DataView",
"Date",
"Error",
"EvalError",
"Float32Array",
"Float64Array",
"Function",

This comment has been minimized.

@WebReflection

WebReflection Dec 13, 2017

I'm pretty sure having at least HTMLElement in this list by default would make many people happy

@WebReflection

WebReflection Dec 13, 2017

I'm pretty sure having at least HTMLElement in this list by default would make many people happy

This comment has been minimized.

@nicolo-ribaudo

nicolo-ribaudo Dec 13, 2017

Member

I'm okay with it, but I'd like to wait for other opinions (since currently Babel is only about ECMAScript, and not about DOM extensions)

@nicolo-ribaudo

nicolo-ribaudo Dec 13, 2017

Member

I'm okay with it, but I'd like to wait for other opinions (since currently Babel is only about ECMAScript, and not about DOM extensions)

This comment has been minimized.

@WebReflection

WebReflection Dec 13, 2017

I think it's a reasonable exception to the rule, specially considering Babel is used the most for the Web.

@WebReflection

WebReflection Dec 13, 2017

I think it's a reasonable exception to the rule, specially considering Babel is used the most for the Web.

This comment has been minimized.

@justinfagnani

justinfagnani Dec 13, 2017

Contributor

I would also add Event and CustomEvent. Considering that these classes aren't wrapped until used, this doesn't seem to add more cost to the transform. I think this transform will be most popular with web components users, so having a useful default seems better to me.

@justinfagnani

justinfagnani Dec 13, 2017

Contributor

I would also add Event and CustomEvent. Considering that these classes aren't wrapped until used, this doesn't seem to add more cost to the transform. I think this transform will be most popular with web components users, so having a useful default seems better to me.

"In16Array",

This comment has been minimized.

@TimothyGu

TimothyGu Dec 12, 2017

Contributor

typo

@TimothyGu

TimothyGu Dec 12, 2017

Contributor

typo

"Int32Array",
"Int8Array",
"Map",
"Number",
"Object",
"Promise",
"Proxy",

This comment has been minimized.

@TimothyGu

TimothyGu Dec 12, 2017

Contributor

Is it worth it to include this here when Proxy can't get subclassed anyway?

@TimothyGu

TimothyGu Dec 12, 2017

Contributor

Is it worth it to include this here when Proxy can't get subclassed anyway?

This comment has been minimized.

@WebReflection

WebReflection Dec 12, 2017

I don't think it makes any sense to maintain this list manually.

This is what I've used to include them all, something you can produce on any browser via:

JSON.stringify(
  Object.getOwnPropertyNames(window)
    .filter(name => /^[A-Z]/.test(name))
    .sort(),
  null,
  '  '
)

so that from time to time one can run that again and update the list.

@WebReflection

WebReflection Dec 12, 2017

I don't think it makes any sense to maintain this list manually.

This is what I've used to include them all, something you can produce on any browser via:

JSON.stringify(
  Object.getOwnPropertyNames(window)
    .filter(name => /^[A-Z]/.test(name))
    .sort(),
  null,
  '  '
)

so that from time to time one can run that again and update the list.

This comment has been minimized.

@nicolo-ribaudo

nicolo-ribaudo Dec 12, 2017

Member

I initially did something like this, but then it would include things like Math, Infinity...
I will ask to include this list in the Globals package.

@nicolo-ribaudo

nicolo-ribaudo Dec 12, 2017

Member

I initially did something like this, but then it would include things like Math, Infinity...
I will ask to include this list in the Globals package.

This comment has been minimized.

@WebReflection

WebReflection Dec 12, 2017

Well, that filter might include typeof “function” too

@WebReflection

WebReflection Dec 12, 2017

Well, that filter might include typeof “function” too

"RangeError",
"ReferenceError",
"RegExp",
"Set",
"SharedArrayBuffer",
"String",
"SyntaxError",
"TypeError",
"Uint16Array",
"Uint32Array",
"Uint8Array",
"Uint8ClampedArray",
"URIError",
"WeakMap",
"WeakSet",
]);
@@ -5,9 +5,22 @@ import nameFunction from "@babel/helper-function-name";
import { types as t } from "@babel/core";
export default function(api, options) {
const { loose } = options;
const { loose, builtins } = options;
const Constructor = loose ? LooseTransformer : VanillaTransformer;
let customBuiltins;
if (builtins) {

This comment has been minimized.

@Kovensky

Kovensky Dec 14, 2017

Member

I'd check for builtins !== undefined, but maybe it's intended that you can pass builtins: false?

(is it also intended that you can pass 0 or ""?)

@Kovensky

Kovensky Dec 14, 2017

Member

I'd check for builtins !== undefined, but maybe it's intended that you can pass builtins: false?

(is it also intended that you can pass 0 or ""?)

This comment has been minimized.

@nicolo-ribaudo

nicolo-ribaudo Dec 14, 2017

Member

It should be !== undefined

@nicolo-ribaudo

nicolo-ribaudo Dec 14, 2017

Member

It should be !== undefined

if (!Array.isArray(builtins)) {
throw new Error(".builtins must be an array.");
}
if (builtins.some(s => typeof s !== "string")) {
throw new Error(".builtins must be an array of strings.");
}
customBuiltins = new Set(builtins);
} else {
customBuiltins = new Set();
}
// todo: investigate traversal requeueing
const VISITED = Symbol();
@@ -54,7 +67,9 @@ export default function(api, options) {
node[VISITED] = true;
path.replaceWith(new Constructor(path, state.file).run());
path.replaceWith(
new Constructor(path, state.file, customBuiltins).run(),
);
if (path.isCallExpression()) {
annotateAsPure(path);
@@ -4,6 +4,8 @@ import optimiseCall from "@babel/helper-optimise-call-expression";
import * as defineMap from "@babel/helper-define-map";
import { traverse, template, types as t } from "@babel/core";
import builtinClasses from "./builtin-classes";
const noMethodVisitor = {
"FunctionExpression|FunctionDeclaration"(path) {
path.skip();
@@ -61,7 +63,7 @@ const findThisesVisitor = traverse.visitors.merge([
]);
export default class ClassTransformer {
constructor(path: NodePath, file) {
constructor(path: NodePath, file, customBuiltins: Set<string>) {

This comment has been minimized.

@Kovensky

Kovensky Dec 14, 2017

Member

Does flow have a ReadonlySet type? I'd prefer to use that if possible since it'd guard against unintentional modification (Set minus all methods that cause mutation)

@Kovensky

Kovensky Dec 14, 2017

Member

Does flow have a ReadonlySet type? I'd prefer to use that if possible since it'd guard against unintentional modification (Set minus all methods that cause mutation)

This comment has been minimized.

@nicolo-ribaudo

nicolo-ribaudo Dec 14, 2017

Member

Unfortunately the only read-only type is ReadonlyAray

@nicolo-ribaudo

nicolo-ribaudo Dec 14, 2017

Member

Unfortunately the only read-only type is ReadonlyAray

this.parent = path.parent;
this.scope = path.scope;
this.node = path.node;
@@ -93,6 +95,12 @@ export default class ClassTransformer {
this.superName = this.node.superClass || t.identifier("Function");
this.isDerived = !!this.node.superClass;
const { name } = this.superName;
this.extendsNative =
this.isDerived &&
(builtinClasses.has(name) || customBuiltins.has(name)) &&

This comment has been minimized.

@justinfagnani

justinfagnani Dec 13, 2017

Contributor

What happens when the extension of the native is indirect due to a mixin application? ie:

class MyElement extends PolymerElement(HTMLElement) {}

Can we have a way to force the wrapping of a particular set of builtins? It'd be nice to have a test for the mixin case too, considering how popular the pattern is in the Web Components ecosystem.

@justinfagnani

justinfagnani Dec 13, 2017

Contributor

What happens when the extension of the native is indirect due to a mixin application? ie:

class MyElement extends PolymerElement(HTMLElement) {}

Can we have a way to force the wrapping of a particular set of builtins? It'd be nice to have a test for the mixin case too, considering how popular the pattern is in the Web Components ecosystem.

This comment has been minimized.

@WebReflection

WebReflection Dec 13, 2017

an easy work around could be to:

class MyElement extends PolymerElement(class extends HTMLElement {}) {}

I don't think it's that simple to handle every time a constructor is passed around and in a meaningful way.

mixins also can be passed around as regular invoke.

I mean ... this is valid code too, right?

const mixed = (E => () => PolymerElement(E))(window['HTMLElement']);
class MyElement extends mixed() {}

We need a compromise for common cases and IMO these are not necessarily based on Polymer.

@WebReflection

WebReflection Dec 13, 2017

an easy work around could be to:

class MyElement extends PolymerElement(class extends HTMLElement {}) {}

I don't think it's that simple to handle every time a constructor is passed around and in a meaningful way.

mixins also can be passed around as regular invoke.

I mean ... this is valid code too, right?

const mixed = (E => () => PolymerElement(E))(window['HTMLElement']);
class MyElement extends mixed() {}

We need a compromise for common cases and IMO these are not necessarily based on Polymer.

This comment has been minimized.

@justinfagnani

justinfagnani Dec 13, 2017

Contributor

The most common case in the wild is that HTMLElement is extended via mixin application. Without support for that, this transform will be much less used than it could be.

This is why I suggest a way to force wrapping, possibly like:

{
  "plugins": [
    ["@babel/transform-classes", {
      "builtins": [ "HTMLElement" ],
      "forceWrap": [ "HTMLElement" ],
    }]
  ]
}
@justinfagnani

justinfagnani Dec 13, 2017

Contributor

The most common case in the wild is that HTMLElement is extended via mixin application. Without support for that, this transform will be much less used than it could be.

This is why I suggest a way to force wrapping, possibly like:

{
  "plugins": [
    ["@babel/transform-classes", {
      "builtins": [ "HTMLElement" ],
      "forceWrap": [ "HTMLElement" ],
    }]
  ]
}

This comment has been minimized.

@WebReflection

WebReflection Dec 13, 2017

The most common case in the wild is that HTMLElement is extended via mixin application.

I'd like to see data about this. I use CE since ever and never used mixins there. It's also worth noticing that Babel supposed to be about ES, not DOM, as previously mentioned.

Every extended builtin constructor is broken right now so this is already quite a big step forward.

This is why I suggest a way to force wrapping, possibly like:

I'm a bit confused by the fact you don't want to double wrap polyfills for Custom Elements but you want to force wrap HTMLElement which basically means the transpiled code will wrap already that class ...

how does it work? You either force wrap, or you avoid runtime, when happens, the double wrap.

Unless you are thinking about having the polyfill before the transpiled code that uses that constructor ... is that the case?

@WebReflection

WebReflection Dec 13, 2017

The most common case in the wild is that HTMLElement is extended via mixin application.

I'd like to see data about this. I use CE since ever and never used mixins there. It's also worth noticing that Babel supposed to be about ES, not DOM, as previously mentioned.

Every extended builtin constructor is broken right now so this is already quite a big step forward.

This is why I suggest a way to force wrapping, possibly like:

I'm a bit confused by the fact you don't want to double wrap polyfills for Custom Elements but you want to force wrap HTMLElement which basically means the transpiled code will wrap already that class ...

how does it work? You either force wrap, or you avoid runtime, when happens, the double wrap.

Unless you are thinking about having the polyfill before the transpiled code that uses that constructor ... is that the case?

This comment has been minimized.

@justinfagnani

justinfagnani Dec 14, 2017

Contributor

The polyfill would load before the compiled code, and so would wrap native HTMLElement to have a working constructor in the first place, otherwise without the polyfill both new HTMLElement() and HTMLElement.apply fail. So when the polyfill is loaded, the wrapping doesn't need to happen.

Optimally, code would be compiled to "force" wrapping of HTMLElement, even if it can't statically see a direct subclass, but the wrapping would bail if the constructor is callable.

@justinfagnani

justinfagnani Dec 14, 2017

Contributor

The polyfill would load before the compiled code, and so would wrap native HTMLElement to have a working constructor in the first place, otherwise without the polyfill both new HTMLElement() and HTMLElement.apply fail. So when the polyfill is loaded, the wrapping doesn't need to happen.

Optimally, code would be compiled to "force" wrapping of HTMLElement, even if it can't statically see a direct subclass, but the wrapping would bail if the constructor is callable.

!this.scope.hasBinding(name, /* noGlobals */ true);
}
run() {
@@ -112,7 +120,13 @@ export default class ClassTransformer {
//
if (this.isDerived) {
closureArgs.push(superName);
if (this.extendsNative) {
closureArgs.push(
t.callExpression(this.file.addHelper("wrapNativeSuper"), [superName]),
);
} else {
closureArgs.push(superName);
}
superName = this.scope.generateUidIdentifierBasedOnNode(superName);
closureParams.push(superName);
@@ -0,0 +1,12 @@
let Foo =
/*#__PURE__*/
function (_Bar) {
babelHelpers.inherits(Foo, _Bar);
function Foo() {
babelHelpers.classCallCheck(this, Foo);
return babelHelpers.possibleConstructorReturn(this, (Foo.__proto__ || Object.getPrototypeOf(Foo)).apply(this, arguments));
}
return Foo;
}(babelHelpers.wrapNativeSuper(Bar));
@@ -0,0 +1,6 @@
{
"plugins": [
["transform-classes", { "builtins": [ "Bar" ] }],
"external-helpers"
]
}
@@ -0,0 +1,34 @@
// Imported from
// https://github.com/WebReflection/babel-plugin-transform-builtin-classes/blob/85efe1374e1c59a8323c7eddd4326f6c93d9f64f/test/test.js
class List extends Array {
constructor(value) {
super().push(value);
}
push(value) {
super.push(value);
return this;
}
}
assert.ok(new List(1) instanceof List, 'new List is an instanceof List');
assert.ok(new List(2) instanceof Array, 'new List is an instanceof Array');
var l = new List(3);
assert.ok(l.length === 1 && l[0] === 3, 'constructor pushes an entry');
assert.ok(l.push(4) === l && l.length === 2 && l.join() === '3,4', 'method override works');
class SecondLevel extends List {
method() {
return this;
}
}
assert.ok(new SecondLevel(1) instanceof SecondLevel, 'new SecondLevel is an instanceof SecondLevel');
assert.ok(new SecondLevel(2) instanceof List, 'new SecondLevel is an instanceof List');
assert.ok(new SecondLevel(3) instanceof Array, 'new SecondLevel is an instanceof Array');
var s = new SecondLevel(4);
assert.ok(s.length === 1 && s[0] === 4, 'constructor pushes an entry');
assert.ok(s.push(5) === s && s.length === 2 && s.join() === '4,5', 'inherited override works');
assert.ok(s.method() === s, 'new method works');
@@ -0,0 +1,3 @@
{
"plugins": ["transform-classes"]
}
@@ -0,0 +1 @@
class List extends Array {}
@@ -0,0 +1,4 @@
class List extends Array {}
assert.ok(new List instanceof List);
assert.ok(new List instanceof Array);
@@ -0,0 +1,23 @@
function _inheritsLoose(subClass, superClass) { subClass.prototype = Object.create(superClass.prototype); subClass.prototype.constructor = subClass; subClass.__proto__ = superClass; }
var _gPO = Object.getPrototypeOf || function _gPO(o) { return o.__proto__; };
var _sPO = Object.setPrototypeOf || function _sPO(o, p) { o.__proto__ = p; };
var _construct = Reflect.construct || function _construct(Parent, args, Class) { var Constructor, a = [null]; a.push.apply(a, args); Constructor = Parent.bind.apply(Parent, a); return _sPO(new Constructor(), Class.prototype); };
var _cache = typeof WeakMap === "function" && new WeakMap();
function _wrapNativeSuper(Class) { if (_cache) { if (_cache.has(Class)) return _cache.get(Class); _cache.set(Class, Wrapper); } function Wrapper() {} Wrapper.prototype = Object.create(Class.prototype, { constructor: { value: Wrapper, enumerable: false, writeable: true, configurable: true } }); return _sPO(Wrapper, _sPO(function Super() { return _construct(Class, arguments, _gPO(this).constructor); }, Class)); }

This comment has been minimized.

@Kovensky

Kovensky Dec 13, 2017

Member

Don't need enumerate: false (default, and if omitted, core-js/es5-sham can avoid being forced to throw on ES3 engines)

@Kovensky

Kovensky Dec 13, 2017

Member

Don't need enumerate: false (default, and if omitted, core-js/es5-sham can avoid being forced to throw on ES3 engines)

This comment has been minimized.

@WebReflection

WebReflection Dec 13, 2017

true that it's false by default but also true that ES3 engines should throw if you extend builtins because ES3 engines are not supported. IE <= 10 is discouraged and that's already mostly ES5

@WebReflection

WebReflection Dec 13, 2017

true that it's false by default but also true that ES3 engines should throw if you extend builtins because ES3 engines are not supported. IE <= 10 is discouraged and that's already mostly ES5

This comment has been minimized.

@nicolo-ribaudo

nicolo-ribaudo Dec 13, 2017

Member

The enumerable: false is also used by the _inherits helper.
Also, ES3 engines don't support setPrototypeOf/__proto__, so classes won't work in any case.

@nicolo-ribaudo

nicolo-ribaudo Dec 13, 2017

Member

The enumerable: false is also used by the _inherits helper.
Also, ES3 engines don't support setPrototypeOf/__proto__, so classes won't work in any case.

This comment has been minimized.

@Kovensky

Kovensky Dec 14, 2017

Member

The use of __proto__ was actually added to the helpers specifically so it would work on ES3 engines that were polyfilled.

The idea was that, even if the engine doesn't support it, as long as we read from __proto__ first, we're going to get a reference to the relevant constructor, which is all that we need for calling super. static property inheritance will still, of course, be broken.

#3527

@Kovensky

Kovensky Dec 14, 2017

Member

The use of __proto__ was actually added to the helpers specifically so it would work on ES3 engines that were polyfilled.

The idea was that, even if the engine doesn't support it, as long as we read from __proto__ first, we're going to get a reference to the relevant constructor, which is all that we need for calling super. static property inheritance will still, of course, be broken.

#3527

let List =
/*#__PURE__*/
function (_Array) {
_inheritsLoose(List, _Array);
function List() {
return _Array.apply(this, arguments) || this;
}
return List;
}(_wrapNativeSuper(Array));
@@ -0,0 +1,3 @@
{
"plugins": [["transform-classes", { "loose": true }]]
}
@@ -0,0 +1,3 @@
class Array {}
class List extends Array {}
@@ -0,0 +1,16 @@
let Array = function Array() {
babelHelpers.classCallCheck(this, Array);
};
let List =
/*#__PURE__*/
function (_Array) {
babelHelpers.inherits(List, _Array);
function List() {
babelHelpers.classCallCheck(this, List);
return babelHelpers.possibleConstructorReturn(this, (List.__proto__ || Object.getPrototypeOf(List)).apply(this, arguments));
}
return List;
}(Array);
@@ -0,0 +1,3 @@
{
"plugins": ["transform-classes", "external-helpers"]
}
@@ -0,0 +1 @@
class List extends Array {}
@@ -0,0 +1,4 @@
class List extends Array {}
assert.ok(new List instanceof List);
assert.ok(new List instanceof Array);
@@ -0,0 +1,29 @@
function _classCallCheck(instance, Constructor) { if (!(instance instanceof Constructor)) { throw new TypeError("Cannot call a class as a function"); } }
function _possibleConstructorReturn(self, call) { if (call && (typeof call === "object" || typeof call === "function")) { return call; } if (self === void 0) { throw new ReferenceError("this hasn't been initialised - super() hasn't been called"); } return self; }
function _inherits(subClass, superClass) { if (typeof superClass !== "function" && superClass !== null) { throw new TypeError("Super expression must either be null or a function"); } subClass.prototype = Object.create(superClass && superClass.prototype, { constructor: { value: subClass, enumerable: false, writable: true, configurable: true } }); if (superClass) Object.setPrototypeOf ? Object.setPrototypeOf(subClass, superClass) : subClass.__proto__ = superClass; }
var _gPO = Object.getPrototypeOf || function _gPO(o) { return o.__proto__; };
var _sPO = Object.setPrototypeOf || function _sPO(o, p) { o.__proto__ = p; };
var _construct = Reflect.construct || function _construct(Parent, args, Class) { var Constructor, a = [null]; a.push.apply(a, args); Constructor = Parent.bind.apply(Parent, a); return _sPO(new Constructor(), Class.prototype); };
var _cache = typeof WeakMap === "function" && new WeakMap();
function _wrapNativeSuper(Class) { if (_cache) { if (_cache.has(Class)) return _cache.get(Class); _cache.set(Class, Wrapper); } function Wrapper() {} Wrapper.prototype = Object.create(Class.prototype, { constructor: { value: Wrapper, enumerable: false, writeable: true, configurable: true } }); return _sPO(Wrapper, _sPO(function Super() { return _construct(Class, arguments, _gPO(this).constructor); }, Class)); }
let List =
/*#__PURE__*/
function (_Array) {
_inherits(List, _Array);
function List() {
_classCallCheck(this, List);
return _possibleConstructorReturn(this, (List.__proto__ || Object.getPrototypeOf(List)).apply(this, arguments));
}
return List;
}(_wrapNativeSuper(Array));
@@ -0,0 +1,3 @@
{
"plugins": ["transform-classes"]
}
ProTip! Use n and p to navigate between commits in a pull request.