observing of props in useEffect is inconsistent with component rendering

[seb-odoo]

The following example prints 1 then 2 in the console, but never 3.
The template does render 1 then 2 then 3 however.

We expect all read of this.props.o.val to be reactive in the same way, but it is not the case.

I understand the canonical way to proceed here would be to pass the signal as props instead of its return value.
But the example is still valid, and the inconsistency of props in terms of effect and rendering is highly problematic.

Because of this, onWillUpdateProps cannot be simply converted to equivalent useEffect, because callers need to provide a signal.

import { Component, mount, xml, props, types, useEffect, proxy, signal } from "@odoo/owl";

class Test extends Component {
    static template = xml`<span t-out="this.props.o.val"/>`;
    props = props(["o"]);
    setup() {
        useEffect(() => console.log(this.props.o.val));
    }
}

class Root extends Component {
    static template = xml`<div>Hello Owl!<Test o="this.obj()"/></div>`;
    static components = { Test };

    obj = signal(proxy({ val: 1 }));

    setup() {
        setTimeout(() => this.obj().val = 2, 100);
        setTimeout(() => this.obj.set(proxy({ val: 3 })), 200);
    }
}

mount(Root, document.body, { templates: TEMPLATES, dev: true });

Side remark, but I don't understand why props.signal="..." is not the default if the given value is not a signal. It seems extremely confusing and error prone to allow non-reactive values as props, but with just a little bit of magic to still trigger re-rendering. This is even more strange as .alike already exists for case where we would want to disable the auto-wrapping behavior.

[seb-odoo]

It seems the current OWL 3 props system is a hybrid between OWL 2 and OWL 3, I believe we should fully migrate it to new concepts instead.

If we allow magic: props automatically wrapped into signals like .signal does.
If we don't allow magic: remove comparison code, adapt all call sites to pass signals. But I understand how problematic that would be in terms of code migration...

[ged-odoo]

mostly agree

[seb-odoo]

Just to keep some notes of our discussion. None of the above suggestions are acceptable.

Converting manually (or automatically wrapping) all props into signal doesn't even fix the issue, as the parent could substitute one signal by another.
Code using props.val() still needs to know if val changes from one signal to another.
Passing a signal from parent to child is only to optimize the parent: delegate the observing to the child to avoid extra renders in the parent. It will never resolve props changing issues.

The only solution to resolve this issue is to make sure props itself is/returns a reactive object (at least proxy, or optimized signal collection).

Props change should be nothing more than updating a key/val pair on the reactive props object.

Render should observe the reactive props object just like it observes all other reactives, and just like any other effect would also observe the props reactive.

This fixes the inconsistency while keeping a similar API as we have now, requiring no specific migration.

[ged-odoo]

agreed on this. only issue is with this part: Props change should be nothing more than updating a key/val pair on the reactive props object. This will probably be a significative breaking change, so it may be difficult to do it in practice. My rough plan is this:

1. make props reactive object
2. rename prop to props.static (mostly cosmetic, to make it clearer)
3. in odoo: remove all onwillupdateprops
4. in owl: remove onwillupdateprops
5. only then: try to make the change mentioned above: props change only triggers signal updates, and sub components update themselves through reactivity