Skip to content
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

Surplus & this #71

Open
philipahlberg opened this issue Aug 8, 2018 · 4 comments
Open

Surplus & this #71

philipahlberg opened this issue Aug 8, 2018 · 4 comments

Comments

@philipahlberg
Copy link

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
Copy link

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
Copy link
Author

philipahlberg commented Aug 9, 2018

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
Copy link
Owner

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants