odoo / owl

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

useState not rendering when also having useStore (useContextWithCB) #799

Closed seb-odoo closed 3 years ago

seb-odoo commented 3 years ago

I'm not sure if the error always happens yet, but here are the steps to reproduce:

This is due to this condition preventing the render:

            if (ctx.rev > mapping[id]) {
                // in this case, the context has been updated since we were rendering
                // last, and we do not need to render here with the observer. A
                // rendering is coming anyway, with the correct props.
                return;
            }

Except that in the case I am in, this render is actually needed.

I think this might be due to the component sharing the same observer for the useStore and useState, and this logic is only supposed to apply to useStore, so useState does not work correctly anymore.

I will try to reproduce on playground and keep posting here, but if you read this and the current report is enough let me know so I don't spend too much time getting a simple example working.

seb-odoo commented 3 years ago

Here is how to reproduce on playground.

The expected result is to have "test, world" on the button on click on "run", but due to the commented await in the code below, the useState is ignored.

const { Component, useState } = owl;
const { useStore } = owl.hooks;

const store = new owl.Store({ state: { rev: 0 }});

let state;
class Greeter extends Component {
    constructor() {
        super(...arguments);
        this.state = useState({ word: 'Hello' });
        state = this.state;
        useStore(props => {
            console.log('usestore 1');
            return 1;
        });
    }

    shouldUpdate() {
        // to prevent the parent rendering from rendering the children automatically
        // needed to actually notice the effect (or lack of) from useState
        return false;
    }
}

// Main root component
class App extends Component {
    constructor() {
        super(...arguments);
        this.state = useState({ name: 'World'});
        useStore(props => store.state.rev);
    }

    async willPatch() {

    }
}
(async function () {
    App.components = { Greeter };
    App.env.store = store;
    // Application setup
    const app = new App();
    await app.mount(document.body);
    store.state.rev++;
    // this is the key to the bug, it makes the App be in "render" state but not
    // yed rendered while the change of state happens
    await Promise.resolve();
    state.word = 'test';
})();
<templates>
  <div t-name="Greeter" class="greeter">
    <t t-esc="state.word"/>, <t t-esc="props.name"/>
  </div>

  <div t-name="App">
    <Greeter name="state.name"/>
    <p t-esc="env.store.state.rev"/>
  </div>
</templates>
seb-odoo commented 3 years ago

Hello @ged-odoo, I suppose you already get a notification when a new issue is created, but just in case I want to let you know this issue is currently blocking me for a fix in v 14.

This is related to what we discussed about "slow render" that forced me start using shouldUpdate, which made me notice this issue. It was previously hidden due to automatic update from parent -> children that made observer on children totally irrelevant (and so any issue was hidden), but now it becomes important.