diff --git a/doc/v3/owl/reference/props.md b/doc/v3/owl/reference/props.md index 0195ec06a..3251f20cc 100644 --- a/doc/v3/owl/reference/props.md +++ b/doc/v3/owl/reference/props.md @@ -377,6 +377,29 @@ wrong to use `.alike`. ``` +### Alike props and effects + +Props coming from `useProps` are reactive, so reading `this.props.someProp` in an +[effect](effects.md) or a [computed value](computed_values.md) subscribes to it. +Alike props are left out of that too, and do not notify when the parent renders +again with a new function. + +```js +class Child extends Component { + props = useProps(["value", "onClick"]); + setup() { + // runs again when `value` changes, but not because the parent rebuilt `onClick` + useEffect(() => { + console.log(this.props.value, this.props.onClick); + }); + } +} +``` + +An anonymous function still notifies if one of the values it captures changes, +since that is also what makes Owl re-render the child. A `.alike` or `.bind` prop +captures nothing, so it never notifies. + ## Binding function props It is common to have the need to pass a callback as a prop. Since Owl components diff --git a/packages/owl-compiler/src/code_generator.ts b/packages/owl-compiler/src/code_generator.ts index 560c20d7b..162d017c0 100644 --- a/packages/owl-compiler/src/code_generator.ts +++ b/packages/owl-compiler/src/code_generator.ts @@ -1164,6 +1164,9 @@ export class CodeGenerator { const hasSlotsProp = "slots" in (ast.props || {}); const props: string[] = []; const propList: string[] = []; + // the props left out of propList because they are always equivalent. The + // runtime needs that list too, to know which prop signals to leave alone. + const alikeProps: string[] = []; for (let p in ast.props || {}) { let [name, suffix] = p.split("."); @@ -1178,8 +1181,11 @@ export class CodeGenerator { } if (suffix) { - // .alike, .bind, .translate — delegate to formatProp, no propList entry props.push(this.formatProp(p, ast.props![p], ast.propsTranslationCtx, ctx.translationCtx)); + // a .translate prop is a constant string, so its signal never really changes + if (suffix !== "translate") { + alikeProps.push(`"${name}"`); + } continue; } @@ -1189,6 +1195,7 @@ export class CodeGenerator { props.push(`${propName}: ${compiledValue || undefined}`); if (freeVariables) { + alikeProps.push(`"${name}"`); for (const varName of freeVariables) { const syntheticKey = `\x01${name}.${varName}`; propList.push(`"${syntheticKey}"`); @@ -1268,11 +1275,12 @@ export class CodeGenerator { } let id = generateId("comp"); this.helpers.add("createComponent"); + const alikeArg = alikeProps.length ? `, [${alikeProps}]` : ""; this.staticDefs.push({ id, expr: `createComponent(app, ${ ast.isDynamic ? null : expr - }, ${!ast.isDynamic}, ${!!ast.slots}, ${!!ast.dynamicProps}, [${propList}])`, + }, ${!ast.isDynamic}, ${!!ast.slots}, ${!!ast.dynamicProps}, [${propList}]${alikeArg})`, }); if (ast.isDynamic) { diff --git a/packages/owl-runtime/src/component_node.ts b/packages/owl-runtime/src/component_node.ts index 090a6bddd..c45eb6c9d 100644 --- a/packages/owl-runtime/src/component_node.ts +++ b/packages/owl-runtime/src/component_node.ts @@ -33,6 +33,9 @@ export class ComponentNode extends Scope implements VNode { parentKey: string | null; props: Record; defaultProps: Record | null = null; + // set by createComponent, read by useProps. Only here to carry the list from + // the template down to the hook. + alikeProps: Set | null; renderFn!: Function; parent: ComponentNode | null; children: { [key: string]: ComponentNode } = Object.create(null); @@ -58,11 +61,13 @@ export class ComponentNode extends Scope implements VNode { props: Record, app: App, parent: ComponentNode | null, - parentKey: string | null + parentKey: string | null, + alikeProps: Set | null = null ) { super(app); this.parent = parent; this.parentKey = parentKey; + this.alikeProps = alikeProps; this.pluginManager = parent ? parent.pluginManager : app.pluginManager; this.componentName = C.name; this.signalComputation = createComputation( diff --git a/packages/owl-runtime/src/props.ts b/packages/owl-runtime/src/props.ts index a102aba5b..5f7d1e23c 100644 --- a/packages/owl-runtime/src/props.ts +++ b/packages/owl-runtime/src/props.ts @@ -44,6 +44,24 @@ export interface PropsFunction { static: typeof staticProp; } +function collectCaptureKeys(props: Record): Map | null { + let byProp: Map | null = null; + for (const key in props) { + if (key.charCodeAt(0) !== 1) { + continue; + } + byProp ||= new Map(); + const name = key.slice(1, key.indexOf(".")); + const keys = byProp.get(name); + if (keys) { + keys.push(key); + } else { + byProp.set(name, [key]); + } + } + return byProp; +} + function makeProps(type?: any): Props<{}> { const node = getComponentScope(); const { app, componentName } = node; @@ -71,6 +89,26 @@ function makeProps(type?: any): Props<{}> { return props[key]; } + const alikeProps = node.alikeProps; + const captureKeys = alikeProps ? collectCaptureKeys(node.props) : null; + let lastProps = node.props; + + // an alike prop is a new function on every render, but it does the same thing + // unless one of the values it captured changed. `.alike` and `.bind` capture + // nothing, so they never change. + function capturesChanged(key: string, props: Record) { + const keys = captureKeys?.get(key); + if (!keys) { + return false; + } + for (const capture of keys) { + if (lastProps[capture] !== props[capture]) { + return true; + } + } + return false; + } + const signals: Record> = Object.create(null); const result = Object.create(null); function defineProp(key: string) { @@ -89,9 +127,14 @@ function makeProps(type?: any): Props<{}> { } function updateSignals(keys: string[]) { + const props = node.props; for (const key of keys) { - signals[key].set(resolveValue(node.props, key)); + if (alikeProps !== null && alikeProps.has(key) && !capturesChanged(key, props)) { + continue; + } + signals[key].set(resolveValue(props, key)); } + lastProps = props; } if (type) { diff --git a/packages/owl-runtime/src/rendering/template_helpers.ts b/packages/owl-runtime/src/rendering/template_helpers.ts index 0fe4c82c5..a75da98fe 100644 --- a/packages/owl-runtime/src/rendering/template_helpers.ts +++ b/packages/owl-runtime/src/rendering/template_helpers.ts @@ -240,9 +240,13 @@ function createComponent

>( isStatic: boolean, hasSlotsProp: boolean, hasDynamicPropList: boolean, - propList: string[] + propList: string[], + alikeProps?: string[] ) { const isDynamic = !isStatic; + // undefined for templates compiled before this list existed, and empty for + // components that have no such prop + const alikePropSet = alikeProps?.length ? new Set(alikeProps) : null; let arePropsDifferent: (p1: P, p2: P) => boolean; const hasNoProp = propList.length === 0; if (hasSlotsProp) { @@ -351,7 +355,7 @@ function createComponent

>( ); } } - node = new ComponentNode(C, props, app, ctx, key); + node = new ComponentNode(C, props, app, ctx, key, alikePropSet); children[key] = node; const fiber = new Fiber(node, parentFiber); if (node.willStart.length) { diff --git a/packages/owl-runtime/tests/components/__snapshots__/error_handling.test.ts.snap b/packages/owl-runtime/tests/components/__snapshots__/error_handling.test.ts.snap index 2f1a208f0..abf65f9db 100644 --- a/packages/owl-runtime/tests/components/__snapshots__/error_handling.test.ts.snap +++ b/packages/owl-runtime/tests/components/__snapshots__/error_handling.test.ts.snap @@ -213,7 +213,7 @@ exports[`basics > render from above on error -- handler is not a Root or MountFi ) { let { text, createBlock, list, multi, html, toggler } = bdom; let { createComponent } = helpers; - const comp1 = createComponent(app, \`Boom\`, true, false, false, []); + const comp1 = createComponent(app, \`Boom\`, true, false, false, [], ["onError"]); let block1 = createBlock(\`

\`); @@ -1156,7 +1156,7 @@ exports[`can catch errors > catching error, rethrow, render parent -- a main co let { text, createBlock, list, multi, html, toggler } = bdom; let { prepareList, createComponent, markRaw, withKey } = helpers; const comp1 = createComponent(app, null, false, false, false, []); - const comp2 = createComponent(app, \`ErrorHandler\`, true, true, false, ["onError.cp"]); + const comp2 = createComponent(app, \`ErrorHandler\`, true, true, false, ["onError.cp"], ["onError"]); function slot1(ctx, node, key = "") { const Comp1 = ctx['cp'].Comp; @@ -1234,7 +1234,7 @@ exports[`can catch errors > catching in child makes parent render 1`] = ` let { text, createBlock, list, multi, html, toggler } = bdom; let { prepareList, createComponent, markRaw, withKey } = helpers; const comp1 = createComponent(app, null, false, false, false, ["id"]); - const comp2 = createComponent(app, \`Catch\`, true, true, false, ["onError.elem"]); + const comp2 = createComponent(app, \`Catch\`, true, true, false, ["onError.elem"], ["onError"]); function slot1(ctx, node, key = "") { const Comp1 = ctx['elem'][1]; diff --git a/packages/owl-runtime/tests/components/__snapshots__/props.test.ts.snap b/packages/owl-runtime/tests/components/__snapshots__/props.test.ts.snap index 086718ace..61c564c35 100644 --- a/packages/owl-runtime/tests/components/__snapshots__/props.test.ts.snap +++ b/packages/owl-runtime/tests/components/__snapshots__/props.test.ts.snap @@ -5,7 +5,7 @@ exports[`.alike suffix in a list 1`] = ` ) { let { text, createBlock, list, multi, html, toggler } = bdom; let { prepareList, createComponent, withKey } = helpers; - const comp1 = createComponent(app, \`Todo\`, true, false, false, ["todo"]); + const comp1 = createComponent(app, \`Todo\`, true, false, false, ["todo"], ["toggle"]); return function template(ctx, node, key = "") { const ctx1 = ctx; @@ -47,7 +47,7 @@ exports[`.alike suffix in a simple case 1`] = ` ) { let { text, createBlock, list, multi, html, toggler } = bdom; let { safeOutput, createComponent } = helpers; - const comp1 = createComponent(app, \`Child\`, true, false, false, []); + const comp1 = createComponent(app, \`Child\`, true, false, false, [], ["fn"]); return function template(ctx, node, key = "") { const b2 = safeOutput(ctx['this'].state.counter); @@ -284,12 +284,139 @@ exports[`.translate props are translated 2`] = ` }" `; +exports[`alike props and reactivity > a .bind prop does not rerun effects when another prop changes 1`] = ` +"function anonymous(app, bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler } = bdom; + let { createComponent } = helpers; + const comp1 = createComponent(app, \`Child\`, true, false, false, ["val"], ["fn"]); + + return function template(ctx, node, key = "") { + return comp1({val: ctx['this'].state.val,fn: (ctx['this'].someFunction).bind(this)}, key + \`__1\`, node, this, null); + } +}" +`; + +exports[`alike props and reactivity > a .bind prop does not rerun effects when another prop changes 2`] = ` +"function anonymous(app, bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler } = bdom; + let { safeOutput } = helpers; + + return function template(ctx, node, key = "") { + return safeOutput(ctx['this'].props.val); + } +}" +`; + +exports[`alike props and reactivity > a plain function prop still reruns effects when it is reassigned 1`] = ` +"function anonymous(app, bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler } = bdom; + let { createComponent } = helpers; + const comp1 = createComponent(app, \`Child\`, true, false, false, ["val","fn"]); + + return function template(ctx, node, key = "") { + return comp1({val: ctx['this'].state.val,fn: ctx['this'].state.fn}, key + \`__1\`, node, this, null); + } +}" +`; + +exports[`alike props and reactivity > a plain function prop still reruns effects when it is reassigned 2`] = ` +"function anonymous(app, bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler } = bdom; + let { safeOutput } = helpers; + + return function template(ctx, node, key = "") { + return safeOutput(ctx['this'].props.val); + } +}" +`; + +exports[`alike props and reactivity > an .alike prop does not rerun effects either 1`] = ` +"function anonymous(app, bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler } = bdom; + let { createComponent } = helpers; + const comp1 = createComponent(app, \`Child\`, true, false, false, ["val"], ["fn"]); + + return function template(ctx, node, key = "") { + return comp1({val: ctx['this'].state.val,fn: ()=>ctx['this'].state.val}, key + \`__1\`, node, this, null); + } +}" +`; + +exports[`alike props and reactivity > an .alike prop does not rerun effects either 2`] = ` +"function anonymous(app, bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler } = bdom; + let { safeOutput } = helpers; + + return function template(ctx, node, key = "") { + return safeOutput(ctx['this'].props.val); + } +}" +`; + +exports[`alike props and reactivity > an arrow function prop reruns effects when a captured variable changes 1`] = ` +"function anonymous(app, bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler } = bdom; + let { createComponent } = helpers; + const comp1 = createComponent(app, \`Child\`, true, false, false, ["val","fn.factor"], ["fn"]); + + return function template(ctx, node, key = "") { + ctx = Object.create(ctx); + ctx["factor"] = ctx['this'].state.val; + return comp1({val: ctx['this'].state.val,fn: ()=>ctx['factor']*10,"fn.factor": ctx['factor']}, key + \`__1\`, node, this, null); + } +}" +`; + +exports[`alike props and reactivity > an arrow function prop reruns effects when a captured variable changes 2`] = ` +"function anonymous(app, bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler } = bdom; + let { safeOutput } = helpers; + + return function template(ctx, node, key = "") { + return safeOutput(ctx['this'].props.val); + } +}" +`; + +exports[`alike props and reactivity > an effect reading an arrow function prop does not rerun on unrelated updates 1`] = ` +"function anonymous(app, bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler } = bdom; + let { createComponent } = helpers; + const comp1 = createComponent(app, \`Child\`, true, false, false, ["val"], ["fn"]); + + return function template(ctx, node, key = "") { + return comp1({val: ctx['this'].state.val,fn: ()=>ctx['this'].someFunction()}, key + \`__1\`, node, this, null); + } +}" +`; + +exports[`alike props and reactivity > an effect reading an arrow function prop does not rerun on unrelated updates 2`] = ` +"function anonymous(app, bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler } = bdom; + let { safeOutput } = helpers; + + return function template(ctx, node, key = "") { + return safeOutput(ctx['this'].props.val); + } +}" +`; + exports[`arrow function props auto-skip re-render when captured variables don't change 1`] = ` "function anonymous(app, bdom, helpers ) { let { text, createBlock, list, multi, html, toggler } = bdom; let { safeOutput, createComponent } = helpers; - const comp1 = createComponent(app, \`Child\`, true, false, false, []); + const comp1 = createComponent(app, \`Child\`, true, false, false, [], ["fn"]); return function template(ctx, node, key = "") { const b2 = safeOutput(ctx['this'].state.counter); @@ -316,7 +443,7 @@ exports[`arrow function props re-render when captured variable changes 1`] = ` ) { let { text, createBlock, list, multi, html, toggler } = bdom; let { prepareList, createComponent, withKey } = helpers; - const comp1 = createComponent(app, \`Todo\`, true, false, false, ["todo","toggle.elem"]); + const comp1 = createComponent(app, \`Todo\`, true, false, false, ["todo","toggle.elem"], ["toggle"]); return function template(ctx, node, key = "") { const ctx1 = ctx; @@ -389,7 +516,7 @@ exports[`basics > arrow function props do not leak synthetic keys into props() 1 ) { let { text, createBlock, list, multi, html, toggler } = bdom; let { prepareList, createComponent, withKey } = helpers; - const comp1 = createComponent(app, \`Child\`, true, false, false, ["onClick.item"]); + const comp1 = createComponent(app, \`Child\`, true, false, false, ["onClick.item"], ["onClick"]); return function template(ctx, node, key = "") { const ctx1 = ctx; @@ -425,7 +552,7 @@ exports[`basics > arrow functions as prop correctly capture their scope 1`] = ` ) { let { text, createBlock, list, multi, html, toggler } = bdom; let { prepareList, createComponent, withKey } = helpers; - const comp1 = createComponent(app, \`Child\`, true, false, false, ["onClick.item"]); + const comp1 = createComponent(app, \`Child\`, true, false, false, ["onClick.item"], ["onClick"]); return function template(ctx, node, key = "") { const ctx1 = ctx; @@ -678,7 +805,7 @@ exports[`bound functions are considered 'alike' 1`] = ` ) { let { text, createBlock, list, multi, html, toggler } = bdom; let { safeOutput, createComponent } = helpers; - const comp1 = createComponent(app, \`Child\`, true, false, false, []); + const comp1 = createComponent(app, \`Child\`, true, false, false, [], ["fn"]); return function template(ctx, node, key = "") { const b2 = safeOutput(ctx['this'].state.val); @@ -704,7 +831,7 @@ exports[`bound functions is not referentially equal after update 1`] = ` ) { let { text, createBlock, list, multi, html, toggler } = bdom; let { createComponent } = helpers; - const comp1 = createComponent(app, \`Child\`, true, false, false, ["val"]); + const comp1 = createComponent(app, \`Child\`, true, false, false, ["val"], ["fn"]); return function template(ctx, node, key = "") { return comp1({val: ctx['this'].state.val,fn: (ctx['this'].someFunction).bind(this)}, key + \`__1\`, node, this, null); @@ -729,7 +856,7 @@ exports[`can bind function prop with bind suffix 1`] = ` ) { let { text, createBlock, list, multi, html, toggler } = bdom; let { createComponent } = helpers; - const comp1 = createComponent(app, \`Child\`, true, false, false, []); + const comp1 = createComponent(app, \`Child\`, true, false, false, [], ["doSomething"]); return function template(ctx, node, key = "") { return comp1({doSomething: (ctx['this'].doSomething).bind(this)}, key + \`__1\`, node, this, null); @@ -810,7 +937,7 @@ exports[`do not crash when binding anonymous function prop with bind suffix 1`] ) { let { text, createBlock, list, multi, html, toggler } = bdom; let { createComponent } = helpers; - const comp1 = createComponent(app, \`Child\`, true, false, false, []); + const comp1 = createComponent(app, \`Child\`, true, false, false, [], ["doSomething"]); return function template(ctx, node, key = "") { return comp1({doSomething: ((_val)=>ctx['this'].doSomething(_val)).bind(this)}, key + \`__1\`, node, this, null); diff --git a/packages/owl-runtime/tests/components/__snapshots__/t_call.test.ts.snap b/packages/owl-runtime/tests/components/__snapshots__/t_call.test.ts.snap index e733cbc29..65f45c8e1 100644 --- a/packages/owl-runtime/tests/components/__snapshots__/t_call.test.ts.snap +++ b/packages/owl-runtime/tests/components/__snapshots__/t_call.test.ts.snap @@ -635,7 +635,7 @@ exports[`t-call > t-call-context: ComponentNode is not looked up in the context ) { let { text, createBlock, list, multi, html, toggler } = bdom; let { createRef, safeOutput, markRaw, createComponent } = helpers; - const comp1 = createComponent(app, \`Child\`, true, true, false, []); + const comp1 = createComponent(app, \`Child\`, true, true, false, [], ["prop"]); let block2 = createBlock(\`
outside slot
\`); let block4 = createBlock(\`
I'm the default slot
\`); diff --git a/packages/owl-runtime/tests/components/props.test.ts b/packages/owl-runtime/tests/components/props.test.ts index b86a4d870..d7b340ac0 100644 --- a/packages/owl-runtime/tests/components/props.test.ts +++ b/packages/owl-runtime/tests/components/props.test.ts @@ -7,6 +7,7 @@ import { proxy, signal, types as t, + useEffect, xml, } from "../../src"; import { @@ -607,6 +608,154 @@ test("arrow function props re-render when captured variable changes", async () = `); }); +describe("alike props and reactivity", () => { + test("an effect reading an arrow function prop does not rerun on unrelated updates", async () => { + let count = 0; + class Child extends Component { + static template = xml``; + props = props(); + setup() { + useEffect(() => { + this.props.fn; + count++; + }); + } + } + + class Parent extends Component { + static template = xml``; + static components = { Child }; + state = proxy({ val: 1 }); + someFunction() {} + } + + const parent = await mount(Parent, fixture); + expect(count).toBe(1); + + parent.state.val = 2; + await nextTick(); + // the child did re-render (val changed), but fn is the same closure + expect(fixture.innerHTML).toBe("2"); + expect(count).toBe(1); + }); + + test("a .bind prop does not rerun effects when another prop changes", async () => { + let count = 0; + class Child extends Component { + static template = xml``; + props = props(); + setup() { + useEffect(() => { + this.props.fn; + count++; + }); + } + } + + class Parent extends Component { + static template = xml``; + static components = { Child }; + state = proxy({ val: 1 }); + someFunction() {} + } + + const parent = await mount(Parent, fixture); + expect(count).toBe(1); + + parent.state.val = 2; + await nextTick(); + expect(fixture.innerHTML).toBe("2"); + expect(count).toBe(1); + }); + + test("an .alike prop does not rerun effects either", async () => { + let count = 0; + class Child extends Component { + static template = xml``; + props = props(); + setup() { + useEffect(() => { + this.props.fn; + count++; + }); + } + } + + class Parent extends Component { + static template = xml``; + static components = { Child }; + state = proxy({ val: 1 }); + } + + const parent = await mount(Parent, fixture); + expect(count).toBe(1); + + parent.state.val = 2; + await nextTick(); + expect(fixture.innerHTML).toBe("2"); + expect(count).toBe(1); + }); + + test("an arrow function prop reruns effects when a captured variable changes", async () => { + let count = 0; + let result = 0; + class Child extends Component { + static template = xml``; + props = props(); + setup() { + useEffect(() => { + result = this.props.fn(); + count++; + }); + } + } + + class Parent extends Component { + static template = xml` + + `; + static components = { Child }; + state = proxy({ val: 1 }); + } + + const parent = await mount(Parent, fixture); + expect(count).toBe(1); + expect(result).toBe(10); + + parent.state.val = 2; + await nextTick(); + expect(count).toBe(2); + expect(result).toBe(20); + }); + + test("a plain function prop still reruns effects when it is reassigned", async () => { + let count = 0; + class Child extends Component { + static template = xml``; + props = props(); + setup() { + useEffect(() => { + this.props.fn; + count++; + }); + } + } + + class Parent extends Component { + static template = xml``; + static components = { Child }; + state = proxy<{ val: number; fn: Function }>({ val: 1, fn: () => 1 }); + } + + const parent = await mount(Parent, fixture); + expect(count).toBe(1); + + parent.state.fn = () => 2; + await nextTick(); + expect(count).toBe(2); + }); +}); + test("schema defaults and signal-driven props", async () => { class Child extends Component { static template = xml` / `;