odoo / owl

OWL: A web framework for structured, dynamic and maintainable applications
https://odoo.github.io/owl/
Other
1.1k stars 332 forks source link

[perf] props (reactive object) should not be observed outside of rendering #1601

Closed seb-odoo closed 3 months ago

seb-odoo commented 3 months ago

What is observed? Reactive props are always observed, even if they are used outside of the render.

Why is it bad? This leads to extra renders when the value changes, which can be problematic for performance if the render is costly or happening many times.

Solution? When the value of a props is used outside of rendering, it should not be observed and its change should not trigger a re-render.

Example playground

Prints: 1, then 2 Expected: only 1

import { Component, useState, mount, onWillRender } from "@odoo/owl";

class Greeter extends Component {
    static template = "Greeter";

    setup() {
        this.renderCount = 0;
        onWillRender(() => { this.renderCount++ }); // to notice there is an extra render (it will print 2)
        setTimeout(() => this.props.obj.name, 150); // accessing value outside of render
    }
}

// Main root component
class Root extends Component {
    static components = { Greeter };
    static template = "Root"

    setup() {
        this.state = useState({ obj: { name: 'Hi'} });
        setTimeout(() => this.state.obj.name = "Hello", 300); // changing value, after it was accessed outside of render in child
    }

}

mount(Root, document.body, { templates: TEMPLATES, dev: true });
<templates>
  <div t-name="Greeter">
    <t t-esc="renderCount"/>
  </div>

  <t t-name="Root">
    <Greeter obj="state.obj"/>
  </t>
</templates>
ged-odoo commented 3 months ago

yep, we know, and i think that we agree.

seb-odoo commented 3 months ago

duplicate https://github.com/odoo/owl/issues/1574 ?

ged-odoo commented 3 months ago

good catch. yes, exactly. closing this then. Thanks for caring enough to make an issue