adamhaile / surplus

High performance JSX web views for S.js applications
636 stars 26 forks source link

Surplus & this #71

Open philipahlberg opened 5 years ago

philipahlberg commented 5 years ago

I'm trying to create a web component using Surplus. The web component implements a render() method that returns a DOM node created with Surplus. However, I am unable to access this in certain situations because Surplus generates a bunch of IIFEs inside the render call.

Consider the following:

<button onClick={this.onClick}>Click me!</button>

The output of the code above is this:

(function () {
    var __;
    __ = createElement("button", null, null);
    __.onclick = this.onClick;
    __.textContent = "Click me!";
    return __;
})()

Because of the surrounding anonymous function, this is no longer the same as it was before, and thus I get an error (Cannot read property 'onClick' of undefined).

Is it possible for the compiler to emit arrow functions instead of regular functions? Could that fix the issue?

derekhawker commented 5 years ago

Is this after rebinding your onclick function in your class constructor? Like this example from the React docs? https://reactjs.org/docs/handling-events.html https://codepen.io/gaearon/pen/xEmzGg?editors=0010

Do you have a fuller example? I haven't really used class components with Surplus.

philipahlberg commented 5 years ago

It is not quite that; it's the reference to the method that is wrong because of this, not the context inside the method.

The example I'm trying to get to work is this:

import * as Surplus from 'surplus'; Surplus;
import S from 's-js';

class Component extends HTMLElement {
  constructor() {
    super();
    this.attachShadow({ mode: 'open' });
  }

  connectedCallback() {
    S.root(dispose => {
      this.dispose = dispose;
      this.shadowRoot.appendChild(this.render(this));
    });
  }

  disconnectedCallback() {
    this.dispose();
  }
}

class SElement extends Component {
  constructor() {
    super();
    this.onClick = this.onClick.bind(this);
    this.count = S.value(0);
  }

  onClick() {
    this.count(this.count() + 1);
  }

  render() {
    return <button onclick={this.onClick}>{this.count()}</button>
  }
}

customElements.define('s-element', SElement);

If I were to delete the onclick binding, I would have a similar problem with this.count().

adamhaile commented 5 years ago

Hi Philip -- Hmm, yeah, I follow what you're saying. I'm surprised this hasn't been reported already. I tend not to use this much in my coding style, so hadn't encountered it myself, but it should definitely be fixed.

You probably already knew this, but if this is a blocker (pun, hah), you can stash a reference to this outside the JSX expression:

  render() {
    const _this = this;
    return <button onclick={_this.onClick}>{_this.count()}</button>
  }

I'll look at solutions. Arrow functions would do it, yes, but I want to make sure first that they're now sufficiently performant on some of the older browsers out there. Last time I looked, about two years ago, they were a good bit slower on older browsers. If that's still true, I may use .bind(this), and only on the expressions that reference this.

Thanks for the report. I'm working on some internal perf changes right now but will hit this as soon as that's done. -- Adam

philipahlberg commented 5 years ago

Another possibility could be a simple block with a variable binding outside like this:

let __;
{
    __ = createElement("button", null, null);
    __.onclick = this.onClick;
    __.textContent = "Click me!";
}

Though in this case it's completely superfluous, but I imagine blocks could even outperform IIFEs.