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

extends null #543

Closed
zloirock opened this Issue Apr 14, 2016 · 6 comments

Comments

Projects
None yet
5 participants
@zloirock

zloirock commented Apr 14, 2016

It does not allow me to sleep for a year. It was broken after the last class reform. See the last sentence. For usage this should be called super, but we can't call null. This issue was closed. Now added many serious changes for ES6, so maybe makes sense fix extends null like it was proposed in #22? Usage this in extended from null constructors without super or a special case for super in those constructors should not break any current code, but should make extends null useful instead of current useless garbage in the spec.

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Apr 14, 2016

Member

@zloirock not sleeping is definitely a bug. We should fix this. I think @allenwb's suggestion in #22 is sound (modulo early error restrictions, but I'm not seeing any).

Member

bterlson commented Apr 14, 2016

@zloirock not sleeping is definitely a bug. We should fix this. I think @allenwb's suggestion in #22 is sound (modulo early error restrictions, but I'm not seeing any).

@zloirock

This comment has been minimized.

Show comment
Hide comment
@zloirock

zloirock Apr 14, 2016

@bterlson special case for usage super in those constructors should remove early error and default constructor issues:

var this = Parent !== null ? Reflect.construct(Parent, args, new.target) : Object.create(new.target.prototype);

zloirock commented Apr 14, 2016

@bterlson special case for usage super in those constructors should remove early error and default constructor issues:

var this = Parent !== null ? Reflect.construct(Parent, args, new.target) : Object.create(new.target.prototype);
@allenwb

This comment has been minimized.

Show comment
Hide comment
@allenwb

allenwb Apr 14, 2016

Member

I agree. I think it is a specification bug that extend null classes were marked as "derived"

Member

allenwb commented Apr 14, 2016

I agree. I think it is a specification bug that extend null classes were marked as "derived"

@loganfsmyth

This comment has been minimized.

Show comment
Hide comment
@loganfsmyth

loganfsmyth Apr 19, 2016

Contributor

@bterlson Assuming I'm following this right, this still means the existing spec behavior of

class Foo extends null {
    constructor(){
        super();
    }
}

will throw because super() can't be used in an extends null class, correct? If so, it seems like step 10 should also be updated, since this will now initialize this by default, but still default to calling super() when no constructor is given.

I'll admit, auto-initializing this seems unfortunate to me though, it seems much more consistent to always require super() to initialize this even in cases where it is null, since "use super() when you have extends" is a more straightforward thing to understand from an end-user stand-point.

Contributor

loganfsmyth commented Apr 19, 2016

@bterlson Assuming I'm following this right, this still means the existing spec behavior of

class Foo extends null {
    constructor(){
        super();
    }
}

will throw because super() can't be used in an extends null class, correct? If so, it seems like step 10 should also be updated, since this will now initialize this by default, but still default to calling super() when no constructor is given.

I'll admit, auto-initializing this seems unfortunate to me though, it seems much more consistent to always require super() to initialize this even in cases where it is null, since "use super() when you have extends" is a more straightforward thing to understand from an end-user stand-point.

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Apr 19, 2016

Member

Yep, good catch, overlooked it (and fixed via a35f0f3). I don't think super makes sense here since there is no super constructor.

Member

bterlson commented Apr 19, 2016

Yep, good catch, overlooked it (and fixed via a35f0f3). I don't think super makes sense here since there is no super constructor.

jugglinmike added a commit to bocoup/test262 that referenced this issue May 30, 2016

Update tests concerning null-extending classes
The latest revision of ECMA262 makes special provisions for classes
which extend the `null` value [1]. Update the relevant tests
accordingly.

[1] tc39/ecma262#543

jugglinmike added a commit to bocoup/test262 that referenced this issue May 30, 2016

Update tests concerning null-extending classes
The latest revision of ECMA262 makes special provisions for classes
which extend the `null` value [1]. Update the relevant tests
accordingly.

[1] tc39/ecma262#543

jugglinmike added a commit to bocoup/test262 that referenced this issue May 30, 2016

Update tests concerning null-extending classes
The latest revision of ECMA262 makes special provisions for classes
which extend the `null` value [1]. Update the relevant tests
accordingly.

[1] tc39/ecma262#543

leobalter added a commit to tc39/test262 that referenced this issue Jun 10, 2016

Update tests concerning null-extending classes (#658)
The latest revision of ECMA262 makes special provisions for classes
which extend the `null` value [1]. Update the relevant tests
accordingly.

[1] tc39/ecma262#543
@littledan

This comment has been minimized.

Show comment
Hide comment
@littledan

littledan Jun 16, 2016

Member

The change here makes sense to me, but I missed this thread until the test262 test went in and V8 failed it. I'm wondering, for normative changes like this, would it make sense to summarize them at TC39 (or some other place which really highlights it), and get some implementation feedback (or documentation that an implementation already does this) before committing?

Member

littledan commented Jun 16, 2016

The change here makes sense to me, but I missed this thread until the test262 test went in and V8 failed it. I'm wondering, for normative changes like this, would it make sense to summarize them at TC39 (or some other place which really highlights it), and get some implementation feedback (or documentation that an implementation already does this) before committing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment