ionic-team / stencil

A toolchain for building scalable, enterprise-ready component systems on top of TypeScript and Web Component standards. Stencil components can be distributed natively to React, Angular, Vue, and traditional web developers from a single, framework-agnostic codebase.
https://stenciljs.com
Other
12.41k stars 777 forks source link

Arrow functions inside JSX props #2667

Closed Incomming336 closed 3 years ago

Incomming336 commented 3 years ago

Stencil version:

 @stencil/core@1.17.3

I'm submitting a: [x] bug report

Current behavior: https://stenciljs.com/docs/templating-jsx The stencil documentation recommends using arrow functions in JSX props but does that not slow down performance?

A bind call or arrow function in a JSX prop will create a brand new function on every single render. This is bad for performance, as it may cause unnecessary re-renders if a brand new function is passed as a prop to a component that uses reference equality check on the prop to determine if it should update.

Source: https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/jsx-no-bind.md

Or does this problem only exist in React projects?

Expected behavior: Update the stencil documentation with a more performance friendly solution.

dmartinjs commented 3 years ago

This problem is in JSX, not specific to React. So the Stencil documentation should be updated.

Can you close this issue and reopen it in the good repo here => https://github.com/ionic-team/stencil-site

I'll open a PR to update the examples.

markcellus commented 3 years ago

Sure. Also please drop link to that new issue here so interested parties can follow along. Thanks!

olliejm commented 3 years ago

What about when the function being called from JSX need to access this of the component class?

e.g.

@State() myValue: string = 'Test';

render() {
  return <div onClick={this.doSomething}>{this.myValue}</div>
}

private doSomething() {
  this.myValue = 'Test 2';
}

In this case I get the error that I cannot read the property myValue of undefined because it's not bound to this. The following however, does work:

return <div onClick={() => this.doSomething()}>{this.myValue}</div>

The stencil site documentation on events shows the use of arrow functions in JSX: https://stenciljs.com/docs/events#using-events-in-jsx

Currently I'm unsure how to use stencil events in the way that article describes without breaking the react/jsx-no-bind eslint rule

dmartinjs commented 3 years ago

@olliejm to bind this here, you should use arrow function in your private method, like that

@State() myValue: string = 'Test';

render() {
  return <div onClick={this.doSomething}>{this.myValue}</div>;
}

private doSomething = () => {
  this.myValue = 'Test 2';
};
gerardo-rodriguez commented 2 years ago

@dmartinjs Does using the arrow function for the method lead to any performance issues?

As far as I can tell, using an arrow function for a method means the function gets moved into the constructor() which means the function is now unique to each instance. I believe if I use a non-arrow method function, then the function is moved to the MyComponent.prototype which means if there are many instances the function is shared via the prototype and the browser can be more efficient, which means it'll be more performant.

I found this article but I don't know if it applies to Stencil JS component classes or not?

It seems to suggest arrow functions perform the worst. 🤔

Screen Shot 2021-11-29 at 2 52 25 PM