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

Calling super methods doesn't always work, if method is missing in the proto chain. #6446

Closed
trusktr opened this Issue Oct 9, 2017 · 9 comments

Comments

Projects
None yet
5 participants
@trusktr

trusktr commented Oct 9, 2017

Unfortunately, this simple example it works.

But my code base is much more complicated: all the classes are class-factory mixins. If I leave out a method on one of the classes, which may be mixed in any order, then when the call stack reaches a prototype that doesn't not have the called method, I get an error like:

Uncaught TypeError: Cannot read property 'call' of undefined

The line on which this happens is transpiled to look like this:

                (ref = _get(Observable.prototype.__proto__ || _Object$getPrototypeOf(Observable.prototype), 'init', this)).call.apply(ref, [ this ].concat( args ));

The problem is that the _get() function is failing to traverse the prototype to find a more-parent init method.

I am 100% sure that this is the case, but I'm not able to provide a reproduction yet.

If I simply add the missing method to the intermediate class, then it works:

function SomeClassMixin(Base) {
  class SomeClass extends Base {
    init() { super.init() } // this unnecessary place holder makes it work.
  }
}

Let me see if I can post back with a repro...

@babel-bot

This comment has been minimized.

Show comment
Hide comment
@babel-bot

babel-bot Oct 9, 2017

Collaborator

Hey @trusktr! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community that typically always has someone willing to help. You can sign-up here
for an invite.

Collaborator

babel-bot commented Oct 9, 2017

Hey @trusktr! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community that typically always has someone willing to help. You can sign-up here
for an invite.

@loganfsmyth

This comment has been minimized.

Show comment
Hide comment
@loganfsmyth

loganfsmyth Oct 9, 2017

Member

If we don't have a repro case I definitely can't see us keeping this open, so that'd definitely critical to get.

Member

loganfsmyth commented Oct 9, 2017

If we don't have a repro case I definitely can't see us keeping this open, so that'd definitely critical to get.

@babel-bot

This comment has been minimized.

Show comment
Hide comment
@babel-bot

babel-bot Oct 9, 2017

Collaborator

Hi @trusktr! A maintainer of the project has notified me that you're missing
some information we'll need to replicate this issue.

Please understand that we receive a high volume of issues, and there are only a limited number
of volunteers that help maintain this project. The easier it is for us to decipher an issue with the info provided,
the more likely it is that we'll be able to help.

Please make sure you have the following information documented in this ticket:

  1. Your Babel configuration (typically in the form of a .babelrc)
  2. The current (incorrect) behavior you're seeing
  3. The behavior you expect
  4. A short, self-contained example

Please provide either a link to the problem via the repl, or if the repl is insufficient,
a new and minimal repository with instructions on how to build/replicate the issue.

Collaborator

babel-bot commented Oct 9, 2017

Hi @trusktr! A maintainer of the project has notified me that you're missing
some information we'll need to replicate this issue.

Please understand that we receive a high volume of issues, and there are only a limited number
of volunteers that help maintain this project. The easier it is for us to decipher an issue with the info provided,
the more likely it is that we'll be able to help.

Please make sure you have the following information documented in this ticket:

  1. Your Babel configuration (typically in the form of a .babelrc)
  2. The current (incorrect) behavior you're seeing
  3. The behavior you expect
  4. A short, self-contained example

Please provide either a link to the problem via the repl, or if the repl is insufficient,
a new and minimal repository with instructions on how to build/replicate the issue.

@trusktr

This comment has been minimized.

Show comment
Hide comment
@trusktr

trusktr Oct 9, 2017

@loganfsmyth I've got the tab open, will post a repro soon!

trusktr commented Oct 9, 2017

@loganfsmyth I've got the tab open, will post a repro soon!

@jridgewell

This comment has been minimized.

Show comment
Hide comment
@jridgewell

jridgewell Oct 19, 2017

Member

Ping.

Member

jridgewell commented Oct 19, 2017

Ping.

@trusktr

This comment has been minimized.

Show comment
Hide comment
@trusktr

trusktr Oct 23, 2017

Alright, here's a repro. This branch works and this one doesn't.

To run, check out one of the branches, and

npm i
npm run build-global

which outputs global.js in the root of the project (compiled with Babel). Then open test.html in your browser; it should work statically, but you can use optionally http-server or similar.

trusktr commented Oct 23, 2017

Alright, here's a repro. This branch works and this one doesn't.

To run, check out one of the branches, and

npm i
npm run build-global

which outputs global.js in the root of the project (compiled with Babel). Then open test.html in your browser; it should work statically, but you can use optionally http-server or similar.

@trusktr

This comment has been minimized.

Show comment
Hide comment
@trusktr

trusktr Oct 23, 2017

I should mention that the lines changed from working to not-working are in src/html/HTMLNode.js.

trusktr commented Oct 23, 2017

I should mention that the lines changed from working to not-working are in src/html/HTMLNode.js.

@loganfsmyth

This comment has been minimized.

Show comment
Hide comment
@loganfsmyth

loganfsmyth Oct 23, 2017

Member

Potentially the useful link is trusktr/infamous@babel-issue-6446-working...babel-issue-6446-not-working

This doesn't appear to be a Babel issue. Stepping through your code shows that the .construct value is set to undefined by your own code in https://github.com/trusktr/infamous/blob/04a2ac340c14a7edcbeb036ebcbe96b545f15cfe/src/html/DeclarativeBase.js#L336 which does indeed seem to do just that. https://github.com/trusktr/infamous/blob/04a2ac340c14a7edcbeb036ebcbe96b545f15cfe/src/html/DeclarativeBase.js#L390 runs even if there are no getters or setters added onto the object and

var o = {};
Object.defineProperty(o, 'prop', {});

is essentially

o.prop = undefined;
Member

loganfsmyth commented Oct 23, 2017

Potentially the useful link is trusktr/infamous@babel-issue-6446-working...babel-issue-6446-not-working

This doesn't appear to be a Babel issue. Stepping through your code shows that the .construct value is set to undefined by your own code in https://github.com/trusktr/infamous/blob/04a2ac340c14a7edcbeb036ebcbe96b545f15cfe/src/html/DeclarativeBase.js#L336 which does indeed seem to do just that. https://github.com/trusktr/infamous/blob/04a2ac340c14a7edcbeb036ebcbe96b545f15cfe/src/html/DeclarativeBase.js#L390 runs even if there are no getters or setters added onto the object and

var o = {};
Object.defineProperty(o, 'prop', {});

is essentially

o.prop = undefined;
@trusktr

This comment has been minimized.

Show comment
Hide comment
@trusktr

trusktr Oct 26, 2017

@loganfsmyth Ah, thanks, and sorry for wasting your time! I knew that blacklist was going to bite me at some point. I've been meaning to change to an explicit whitelist. I guess now's the time!

trusktr commented Oct 26, 2017

@loganfsmyth Ah, thanks, and sorry for wasting your time! I knew that blacklist was going to bite me at some point. I've been meaning to change to an explicit whitelist. I guess now's the time!

@lock lock bot added the outdated label May 1, 2018

@lock lock bot locked as resolved and limited conversation to collaborators May 1, 2018

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