MatAtBread / fast-async

605 stars 21 forks source link

[bug] infinite recursion (stack overflow) with $super$N helper #54

Closed trusktr closed 6 years ago

trusktr commented 6 years ago

I've got some transpiled ode that looks like this:

        disconnectedCallback() {
            return new Promise(function ($return, $error) {
                if (this.$super$1('disconnectedCallback')) this.$super$1('disconnectedCallback').call(this);

and it looks like $super$1('disconnectedCallback').call(this) calls the exact same disconnectedCallback method in an infinite loop.

I'm not sure if this is a fast-async problem, I think probably not, because $super$1 just calls native super. But it is fast-async output, so thought I'd post just in case.

Gonna investigate...

trusktr commented 6 years ago

Ah, I see the problem: fast-async places two $super$1 methods in my class's prototype chain, so it looks like the following contrived example:

let p1 = {
  method() {
    // transpiled-like code:
    if ( this.$super$1( 'method' ) ) this.$super$1( 'method' ).call(this)
  },
  $super$1(prop) { return super[prop] },
}

let p2 = {
  __proto__: p1,
  $super$1(prop) { return super[prop] },
}

let o = {
  __proto__: p2,
}

o.method() // infinite loop

Here's a fiddle showing Maximum call stack size exceeded error: https://jsfiddle.net/vhwv5tk1/4/

trusktr commented 6 years ago

To prevent this, I think probably fast-async should probably use the parent constructor.prototype directly, or similar.

trusktr commented 6 years ago

The problem it likely to be caused because I'm using lowclass to create one or more of the classes in the middle of the class hierarchy. Because of this, probably something in fast-async's analyses is breaking, and not realizing that there's a single class hierarchy in my case (I'm guessing it thinks there's two hierarchies, one for each part of the prototype chain on either side of my non-class class).

I'm willing to bet that this problem will occur when using any of these 42 libraries in place of lowclass; basically any time a class in the hierarchy is made in a custom manner as with those libraries.

trusktr commented 6 years ago

One workaround (which is what I was going to do anyways) is to replace every class in my hierarchy with lowclass-made version, which allows me to use my custom Super helper, and thus avoid fast-async placing $super$N anywhere in the class hierarchy prototype chain.

trusktr commented 6 years ago

As a quick workaround so I can get to dev'ing before I convert my whole hierarchy, your webpack-async-await plugin is helping me avoid the async syntax error in Webpack 3. Thanks! :)

matAtWork commented 6 years ago

Thanks for the very detailed narrative. To be honest, I'm not sure I followed all of it - did you manage to produce a minimal case I can try and fix?

matAtWork commented 6 years ago

I doubt it's related to lowclass or any other library (as long as they are valid JS), as the $super$ is specifically providing a reference for the super ES6 keyword within class definitions which may be referenced after an await and therefore in a Promise callback (and out of scope for the super).

class X {
   async f() { 
     await Promise.resolve(false) ;
     const b = super.a() ; // This will end up in a callback, so a reference to `super` must be captured
   }
}
MatAtBread commented 6 years ago

Did this get resolved? I've still not been able to produce a simple test case

matAtWork commented 6 years ago

I'll close this for now, but if you can you provide me with an example of JS that exhibits this behaviour before it its transpiled I'd be happy to re-open and take a look.