From 975ed32f0bb3b3c0e8f2c624e3211d3b22f9fb88 Mon Sep 17 00:00:00 2001 From: Samuel Degueldre Date: Mon, 6 Mar 2023 13:24:39 +0100 Subject: [PATCH] [FIX] runtime, compiler: fix refs getting set or unset incorrectly Previously, refs could get set to null incorrectly, or could stay set when they shouldn't. This was caused by the fact that singleRefSetter and multiRefSetters are created on every render, meaning that any render that did not affect the status of the ref (mounted or not), would overwrite the previous ref setter, including its closure, causing the captured `_el` or `count` to be lost. This means that on a subsequent render that did affect the status of the ref, the singleRefSetter would consider that it did not set the ref, and so shouldn't unset it, causing it to incorrectly stay keep the element, while in multiRefSetter, the opposite problem occured: the count would be 0 on removal, causing it to believe that it was the first call in the corresponding patch that affected the ref, when in fact, the closure is already stale, because it was created from the previous render, and the fresh multiRefSetter from the latest render may already have been called by an element with that ref that came earlier in the template. This commit changes the ref setting strategy to work around the issue of stale closures: we define a setRef method on ComponentNode that is always called, this method ignores calls with `null`, meaning that the refs object on the ComponentNode never reverts to a null value for any key. Instead, the useRef hook will check whether the element that the ref points to is still mounted, and return null if not. --- src/compiler/code_generator.ts | 51 ++++------- src/runtime/component_node.ts | 13 +++ src/runtime/hooks.ts | 3 +- src/runtime/template_helpers.ts | 45 ++++------ tests/app/app.test.ts | 2 +- .../compiler/__snapshots__/misc.test.ts.snap | 7 +- .../compiler/__snapshots__/t_ref.test.ts.snap | 31 ++----- tests/compiler/t_ref.test.ts | 85 +++++++++++-------- .../__snapshots__/hooks.test.ts.snap | 20 ++--- .../__snapshots__/refs.test.ts.snap | 52 +++++++----- .../__snapshots__/slots.test.ts.snap | 5 +- .../__snapshots__/t_call.test.ts.snap | 8 +- tests/components/refs.test.ts | 26 +++++- 13 files changed, 177 insertions(+), 171 deletions(-) diff --git a/src/compiler/code_generator.ts b/src/compiler/code_generator.ts index a40b3a02..626b8e26 100644 --- a/src/compiler/code_generator.ts +++ b/src/compiler/code_generator.ts @@ -192,11 +192,9 @@ class CodeTarget { code: string[] = []; hasRoot = false; hasCache = false; - hasRef: boolean = false; - // maps ref name to [id, expr] - refInfo: { [name: string]: [string, string] } = {}; shouldProtectScope: boolean = false; on: EventHandlers | null; + hasRefWrapper: boolean = false; constructor(name: string, on?: EventHandlers | null) { this.name = name; @@ -215,17 +213,13 @@ class CodeTarget { generateCode(): string { let result: string[] = []; result.push(`function ${this.name}(ctx, node, key = "") {`); - if (this.hasRef) { - result.push(` const refs = this.__owl__.refs;`); - for (let name in this.refInfo) { - const [id, expr] = this.refInfo[name]; - result.push(` const ${id} = ${expr};`); - } - } if (this.shouldProtectScope) { result.push(` ctx = Object.create(ctx);`); result.push(` ctx[isBoundary] = 1`); } + if (this.hasRefWrapper) { + result.push(` let refWrapper = makeRefWrapper(this.__owl__);`); + } if (this.hasCache) { result.push(` let cache = ctx.cache || {};`); result.push(` let nextCache = ctx.cache = {};`); @@ -704,30 +698,21 @@ export class CodeGenerator { // t-ref if (ast.ref) { - this.target.hasRef = true; - const isDynamic = INTERP_REGEXP.test(ast.ref); - if (isDynamic) { - this.helpers.add("singleRefSetter"); - const str = replaceDynamicParts(ast.ref, (expr) => this.captureExpression(expr, true)); - const idx = block!.insertData(`singleRefSetter(refs, ${str})`, "ref"); - attrs["block-ref"] = String(idx); - } else { - let name = ast.ref; - if (name in this.target.refInfo) { - // ref has already been defined - this.helpers.add("multiRefSetter"); - const info = this.target.refInfo[name]; - const index = block!.data.push(info[0]) - 1; - attrs["block-ref"] = String(index); - info[1] = `multiRefSetter(refs, \`${name}\`)`; - } else { - let id = generateId("ref"); - this.helpers.add("singleRefSetter"); - this.target.refInfo[name] = [id, `singleRefSetter(refs, \`${name}\`)`]; - const index = block!.data.push(id) - 1; - attrs["block-ref"] = String(index); - } + if (this.dev) { + this.helpers.add("makeRefWrapper"); + this.target.hasRefWrapper = true; } + const isDynamic = INTERP_REGEXP.test(ast.ref); + let name = `\`${ast.ref}\``; + if (isDynamic) { + name = replaceDynamicParts(ast.ref, (expr) => this.captureExpression(expr, true)); + } + let setRefStr = `(el) => this.__owl__.setRef((${name}), el)`; + if (this.dev) { + setRefStr = `refWrapper(${name}, ${setRefStr})`; + } + const idx = block!.insertData(setRefStr, "ref"); + attrs["block-ref"] = String(idx); } const dom = xmlDoc.createElement(ast.tag); diff --git a/src/runtime/component_node.ts b/src/runtime/component_node.ts index d94452ad..4a3adf02 100644 --- a/src/runtime/component_node.ts +++ b/src/runtime/component_node.ts @@ -281,6 +281,19 @@ export class ComponentNode

implements VNode(name: string): { el: const refs = node.refs; return { get el(): T | null { - return refs[name] || null; + const el = refs[name]; + return el?.ownerDocument.contains(el) ? el : null; }, }; } diff --git a/src/runtime/template_helpers.ts b/src/runtime/template_helpers.ts index 0085d7de..f472a6d5 100644 --- a/src/runtime/template_helpers.ts +++ b/src/runtime/template_helpers.ts @@ -5,6 +5,7 @@ import { isOptional, validateSchema } from "./validation"; import type { ComponentConstructor } from "./component"; import { markRaw } from "./reactivity"; import { OwlError } from "./error_handling"; +import type { ComponentNode } from "./component_node"; const ObjectCreate = Object.create; /** @@ -184,34 +185,6 @@ function bind(component: any, fn: Function): Function { return boundFn; } -type RefMap = { [key: string]: HTMLElement | null }; -type RefSetter = (el: HTMLElement | null) => void; - -function multiRefSetter(refs: RefMap, name: string): RefSetter { - let count = 0; - return (el) => { - if (el) { - count++; - if (count > 1) { - throw new OwlError("Cannot have 2 elements with same ref name at the same time"); - } - } - if (count === 0 || el) { - refs[name] = el; - } - }; -} - -function singleRefSetter(refs: RefMap, name: string): RefSetter { - let _el: HTMLElement | null = null; - return (el) => { - if (el || refs[name] === _el) { - refs[name] = el; - _el = el; - } - }; -} - /** * Validate the component props (or next props) against the (static) props * description. This is potentially an expensive operation: it may needs to @@ -260,6 +233,19 @@ export function validateProps

(name: string | ComponentConstructor

, props: } } +function makeRefWrapper(node: ComponentNode) { + let refNames: Set = new Set(); + return (name: string, fn: Function) => { + if (refNames.has(name)) { + throw new OwlError( + `Cannot set the same ref more than once in the same component, ref "${name}" was set multiple times in ${node.name}` + ); + } + refNames.add(name); + return fn; + }; +} + export const helpers = { withDefault, zero: Symbol("zero"), @@ -269,8 +255,6 @@ export const helpers = { withKey, prepareList, setContextValue, - multiRefSetter, - singleRefSetter, shallowEqual, toNumber, validateProps, @@ -280,4 +264,5 @@ export const helpers = { createCatcher, markRaw, OwlError, + makeRefWrapper, }; diff --git a/tests/app/app.test.ts b/tests/app/app.test.ts index 37699a70..ec2f5eee 100644 --- a/tests/app/app.test.ts +++ b/tests/app/app.test.ts @@ -69,7 +69,7 @@ describe("app", () => { static template = xml`

`; } - await mount(Root, fixture, { dev: true, props: { messge: "hey" }, warnIfNoStaticProps: true }); + await mount(Root, fixture, { test: true, props: { messge: "hey" }, warnIfNoStaticProps: true }); console.warn = originalconsoleWarn; expect(mockConsoleWarn).toBeCalledWith( diff --git a/tests/compiler/__snapshots__/misc.test.ts.snap b/tests/compiler/__snapshots__/misc.test.ts.snap index fa3a47a3..2d3e8f81 100644 --- a/tests/compiler/__snapshots__/misc.test.ts.snap +++ b/tests/compiler/__snapshots__/misc.test.ts.snap @@ -182,7 +182,7 @@ exports[`misc other complex template 1`] = ` "function anonymous(app, bdom, helpers ) { let { text, createBlock, list, multi, html, toggler, comment } = bdom; - let { prepareList, withKey, singleRefSetter } = helpers; + let { prepareList, withKey } = helpers; const callTemplate_1 = app.getTemplate(\`LOAD_INFOS_TEMPLATE\`); const comp1 = app.createComponent(\`BundlesList\`, true, false, false, false); const comp2 = app.createComponent(\`BundlesList\`, true, false, false, false); @@ -217,9 +217,6 @@ exports[`misc other complex template 1`] = ` let block25 = createBlock(\`
\`); return function template(ctx, node, key = \\"\\") { - const refs = this.__owl__.refs; - const ref1 = singleRefSetter(refs, \`search_input\`); - const ref2 = singleRefSetter(refs, \`settings_menu\`); let b2,b4,b14,b17,b22,b23,b24,b25; let attr1 = \`/runbot/\${ctx['project'].slug}\`; let txt1 = ctx['project'].name; @@ -280,7 +277,9 @@ exports[`misc other complex template 1`] = ` let prop2 = new String((ctx['search'].value) === 0 ? 0 : ((ctx['search'].value) || \\"\\")); let hdlr4 = [ctx['updateFilter'], ctx]; let hdlr5 = [ctx['updateFilter'], ctx]; + let ref1 = (el) => this.__owl__.setRef((\`search_input\`), el); let hdlr6 = [ctx['clearSearch'], ctx]; + let ref2 = (el) => this.__owl__.setRef((\`settings_menu\`), el); if (ctx['triggers']) { ctx = Object.create(ctx); const [k_block18, v_block18, l_block18, c_block18] = prepareList(ctx['triggers']);; diff --git a/tests/compiler/__snapshots__/t_ref.test.ts.snap b/tests/compiler/__snapshots__/t_ref.test.ts.snap index 22ece76f..a9d4d1e0 100644 --- a/tests/compiler/__snapshots__/t_ref.test.ts.snap +++ b/tests/compiler/__snapshots__/t_ref.test.ts.snap @@ -4,14 +4,12 @@ exports[`t-ref can get a dynamic ref on a node 1`] = ` "function anonymous(app, bdom, helpers ) { let { text, createBlock, list, multi, html, toggler, comment } = bdom; - let { singleRefSetter } = helpers; let block1 = createBlock(\`
\`); return function template(ctx, node, key = \\"\\") { - const refs = this.__owl__.refs; const v1 = ctx['id']; - let ref1 = singleRefSetter(refs, \`myspan\${v1}\`); + let ref1 = (el) => this.__owl__.setRef((\`myspan\${v1}\`), el); return block1([ref1]); } }" @@ -21,14 +19,12 @@ exports[`t-ref can get a dynamic ref on a node, alternate syntax 1`] = ` "function anonymous(app, bdom, helpers ) { let { text, createBlock, list, multi, html, toggler, comment } = bdom; - let { singleRefSetter } = helpers; let block1 = createBlock(\`
\`); return function template(ctx, node, key = \\"\\") { - const refs = this.__owl__.refs; const v1 = ctx['id']; - let ref1 = singleRefSetter(refs, \`myspan\${v1}\`); + let ref1 = (el) => this.__owl__.setRef((\`myspan\${v1}\`), el); return block1([ref1]); } }" @@ -38,13 +34,11 @@ exports[`t-ref can get a ref on a node 1`] = ` "function anonymous(app, bdom, helpers ) { let { text, createBlock, list, multi, html, toggler, comment } = bdom; - let { singleRefSetter } = helpers; let block1 = createBlock(\`
\`); return function template(ctx, node, key = \\"\\") { - const refs = this.__owl__.refs; - const ref1 = singleRefSetter(refs, \`myspan\`); + let ref1 = (el) => this.__owl__.setRef((\`myspan\`), el); return block1([ref1]); } }" @@ -69,13 +63,11 @@ exports[`t-ref ref in a t-call 2`] = ` "function anonymous(app, bdom, helpers ) { let { text, createBlock, list, multi, html, toggler, comment } = bdom; - let { singleRefSetter } = helpers; let block1 = createBlock(\`
12
\`); return function template(ctx, node, key = \\"\\") { - const refs = this.__owl__.refs; - const ref1 = singleRefSetter(refs, \`name\`); + let ref1 = (el) => this.__owl__.setRef((\`name\`), el); return block1([ref1]); } }" @@ -85,16 +77,14 @@ exports[`t-ref ref in a t-if 1`] = ` "function anonymous(app, bdom, helpers ) { let { text, createBlock, list, multi, html, toggler, comment } = bdom; - let { singleRefSetter } = helpers; let block1 = createBlock(\`
\`); let block2 = createBlock(\`\`); return function template(ctx, node, key = \\"\\") { - const refs = this.__owl__.refs; - const ref1 = singleRefSetter(refs, \`name\`); let b2; if (ctx['condition']) { + let ref1 = (el) => this.__owl__.setRef((\`name\`), el); b2 = block2([ref1]); } return block1([], [b2]); @@ -106,13 +96,12 @@ exports[`t-ref refs in a loop 1`] = ` "function anonymous(app, bdom, helpers ) { let { text, createBlock, list, multi, html, toggler, comment } = bdom; - let { prepareList, singleRefSetter, withKey } = helpers; + let { prepareList, withKey } = helpers; let block1 = createBlock(\`
\`); let block3 = createBlock(\`
\`); return function template(ctx, node, key = \\"\\") { - const refs = this.__owl__.refs; ctx = Object.create(ctx); const [k_block2, v_block2, l_block2, c_block2] = prepareList(ctx['items']);; for (let i1 = 0; i1 < l_block2; i1++) { @@ -120,7 +109,7 @@ exports[`t-ref refs in a loop 1`] = ` const key1 = ctx['item']; const tKey_1 = ctx['item']; const v1 = ctx['item']; - let ref1 = singleRefSetter(refs, (v1)); + let ref1 = (el) => this.__owl__.setRef(((v1)), el); let txt1 = ctx['item']; c_block2[i1] = withKey(block3([ref1, txt1]), tKey_1 + key1); } @@ -134,19 +123,17 @@ exports[`t-ref two refs, one in a t-if 1`] = ` "function anonymous(app, bdom, helpers ) { let { text, createBlock, list, multi, html, toggler, comment } = bdom; - let { singleRefSetter } = helpers; let block1 = createBlock(\`

\`); let block2 = createBlock(\`\`); return function template(ctx, node, key = \\"\\") { - const refs = this.__owl__.refs; - const ref1 = singleRefSetter(refs, \`name\`); - const ref2 = singleRefSetter(refs, \`p\`); let b2; if (ctx['condition']) { + let ref1 = (el) => this.__owl__.setRef((\`name\`), el); b2 = block2([ref1]); } + let ref2 = (el) => this.__owl__.setRef((\`p\`), el); return block1([ref2], [b2]); } }" diff --git a/tests/compiler/t_ref.test.ts b/tests/compiler/t_ref.test.ts index 8bef41e1..a8384b45 100644 --- a/tests/compiler/t_ref.test.ts +++ b/tests/compiler/t_ref.test.ts @@ -2,6 +2,17 @@ import { TemplateSet } from "../../src/runtime/template_set"; import { mount } from "../../src/runtime/blockdom"; import { renderToBdom, snapshotEverything } from "../helpers"; +function mockNode() { + return { + refs: {} as { [key: string]: HTMLElement | null }, + setRef(name: string, value: HTMLElement | null) { + if (value) { + this.refs[name] = value; + } + }, + }; +} + snapshotEverything(); // ----------------------------------------------------------------------------- @@ -11,29 +22,29 @@ snapshotEverything(); describe("t-ref", () => { test("can get a ref on a node", () => { const template = `
`; - const refs: any = {}; - const bdom = renderToBdom(template, { __owl__: { refs } }); - expect(refs).toEqual({}); + const node = mockNode(); + const bdom = renderToBdom(template, { __owl__: node }); + expect(node.refs).toEqual({}); mount(bdom, document.createElement("div")); - expect(refs.myspan.tagName).toBe("SPAN"); + expect(node.refs.myspan!.tagName).toBe("SPAN"); }); test("can get a dynamic ref on a node", () => { const template = `
`; - const refs: any = {}; - const bdom = renderToBdom(template, { id: 3, __owl__: { refs } }); - expect(refs).toEqual({}); + const node = mockNode(); + const bdom = renderToBdom(template, { id: 3, __owl__: node }); + expect(node.refs).toEqual({}); mount(bdom, document.createElement("div")); - expect(refs.myspan3.tagName).toBe("SPAN"); + expect(node.refs.myspan3!.tagName).toBe("SPAN"); }); test("can get a dynamic ref on a node, alternate syntax", () => { const template = `
`; - const refs: any = {}; - const bdom = renderToBdom(template, { id: 3, __owl__: { refs } }); - expect(refs).toEqual({}); + const node = mockNode(); + const bdom = renderToBdom(template, { id: 3, __owl__: node }); + expect(node.refs).toEqual({}); mount(bdom, document.createElement("div")); - expect(refs.myspan3.tagName).toBe("SPAN"); + expect(node.refs.myspan3!.tagName).toBe("SPAN"); }); test("refs in a loop", () => { @@ -42,11 +53,11 @@ describe("t-ref", () => {
`; - const refs: any = {}; - const bdom = renderToBdom(template, { items: [1, 2, 3], __owl__: { refs } }); - expect(refs).toEqual({}); + const node = mockNode(); + const bdom = renderToBdom(template, { items: [1, 2, 3], __owl__: node }); + expect(node.refs).toEqual({}); mount(bdom, document.createElement("div")); - expect(Object.keys(refs)).toEqual(["1", "2", "3"]); + expect(Object.keys(node.refs)).toEqual(["1", "2", "3"]); }); test("ref in a t-if", () => { @@ -57,22 +68,23 @@ describe("t-ref", () => { `; - const refs: any = {}; + const node = mockNode(); // false - const bdom = renderToBdom(template, { condition: false, __owl__: { refs } }); - expect(refs).toEqual({}); + const bdom = renderToBdom(template, { condition: false, __owl__: node }); + expect(node.refs).toEqual({}); mount(bdom, document.createElement("div")); - expect(refs).toEqual({}); + expect(node.refs).toEqual({}); // true now - const bdom2 = renderToBdom(template, { condition: true, __owl__: { refs } }); + const bdom2 = renderToBdom(template, { condition: true, __owl__: node }); bdom.patch(bdom2, true); - expect(refs.name.tagName).toBe("SPAN"); + expect(node.refs.name!.tagName).toBe("SPAN"); // false again - const bdom3 = renderToBdom(template, { condition: false, __owl__: { refs } }); + const bdom3 = renderToBdom(template, { condition: false, __owl__: node }); bdom.patch(bdom3, true); - expect(refs).toEqual({ name: null }); + const span = node.refs.name; + expect(span!.ownerDocument.contains(span)).toBe(false); }); test("two refs, one in a t-if", () => { @@ -84,40 +96,41 @@ describe("t-ref", () => {

`; - const refs: any = {}; + const node = mockNode(); // false - const bdom = renderToBdom(template, { condition: false, __owl__: { refs } }); - expect(Object.keys(refs)).toEqual([]); + const bdom = renderToBdom(template, { condition: false, __owl__: node }); + expect(Object.keys(node.refs)).toEqual([]); mount(bdom, document.createElement("div")); - expect(Object.keys(refs)).toEqual(["p"]); + expect(Object.keys(node.refs)).toEqual(["p"]); // true now - const bdom2 = renderToBdom(template, { condition: true, __owl__: { refs } }); + const bdom2 = renderToBdom(template, { condition: true, __owl__: node }); bdom.patch(bdom2, true); - expect(Object.keys(refs)).toEqual(["p", "name"]); + expect(Object.keys(node.refs)).toEqual(["p", "name"]); // false again - const bdom3 = renderToBdom(template, { condition: false, __owl__: { refs } }); + const bdom3 = renderToBdom(template, { condition: false, __owl__: node }); bdom.patch(bdom3, true); - expect(Object.keys(refs)).toEqual(["p", "name"]); - expect(refs.name).toBeNull(); + expect(Object.keys(node.refs)).toEqual(["p", "name"]); + const span = node.refs.name; + expect(span!.ownerDocument.contains(span)).toBe(false); }); test("ref in a t-call", () => { const main = `

`; const sub = `
12
`; - const refs: any = {}; + const node = mockNode(); const app = new TemplateSet(); app.addTemplate("main", main); app.addTemplate("sub", sub); - const comp = { __owl__: { refs } }; + const comp = { __owl__: node }; const bdom = app.getTemplate("main").call(comp, comp, {}); mount(bdom, document.createElement("div")); - expect(refs.name.tagName).toBe("SPAN"); + expect(node.refs.name!.tagName).toBe("SPAN"); }); }); diff --git a/tests/components/__snapshots__/hooks.test.ts.snap b/tests/components/__snapshots__/hooks.test.ts.snap index e7c427f7..2dc2e9c6 100644 --- a/tests/components/__snapshots__/hooks.test.ts.snap +++ b/tests/components/__snapshots__/hooks.test.ts.snap @@ -4,17 +4,15 @@ exports[`hooks autofocus hook input in a t-if 1`] = ` "function anonymous(app, bdom, helpers ) { let { text, createBlock, list, multi, html, toggler, comment } = bdom; - let { singleRefSetter } = helpers; let block1 = createBlock(\`
\`); let block2 = createBlock(\`\`); return function template(ctx, node, key = \\"\\") { - const refs = this.__owl__.refs; - const ref1 = singleRefSetter(refs, \`input1\`); - const ref2 = singleRefSetter(refs, \`input2\`); let b2; + let ref1 = (el) => this.__owl__.setRef((\`input1\`), el); if (ctx['state'].flag) { + let ref2 = (el) => this.__owl__.setRef((\`input2\`), el); b2 = block2([ref2]); } return block1([ref1], [b2]); @@ -26,14 +24,12 @@ exports[`hooks autofocus hook simple input 1`] = ` "function anonymous(app, bdom, helpers ) { let { text, createBlock, list, multi, html, toggler, comment } = bdom; - let { singleRefSetter } = helpers; let block1 = createBlock(\`
\`); return function template(ctx, node, key = \\"\\") { - const refs = this.__owl__.refs; - const ref1 = singleRefSetter(refs, \`input1\`); - const ref2 = singleRefSetter(refs, \`input2\`); + let ref1 = (el) => this.__owl__.setRef((\`input1\`), el); + let ref2 = (el) => this.__owl__.setRef((\`input2\`), el); return block1([ref1, ref2]); } }" @@ -266,15 +262,13 @@ exports[`hooks useEffect hook effect can depend on stuff in dom 1`] = ` "function anonymous(app, bdom, helpers ) { let { text, createBlock, list, multi, html, toggler, comment } = bdom; - let { singleRefSetter } = helpers; let block2 = createBlock(\`
\`); return function template(ctx, node, key = \\"\\") { - const refs = this.__owl__.refs; - const ref1 = singleRefSetter(refs, \`div\`); let b2; if (ctx['state'].value) { + let ref1 = (el) => this.__owl__.setRef((\`div\`), el); b2 = block2([ref1]); } return multi([b2]); @@ -356,13 +350,11 @@ exports[`hooks useRef hook: basic use 1`] = ` "function anonymous(app, bdom, helpers ) { let { text, createBlock, list, multi, html, toggler, comment } = bdom; - let { singleRefSetter } = helpers; let block1 = createBlock(\`
\`); return function template(ctx, node, key = \\"\\") { - const refs = this.__owl__.refs; - const ref1 = singleRefSetter(refs, \`button\`); + let ref1 = (el) => this.__owl__.setRef((\`button\`), el); let txt1 = ctx['value']; return block1([ref1, txt1]); } diff --git a/tests/components/__snapshots__/refs.test.ts.snap b/tests/components/__snapshots__/refs.test.ts.snap index 88b0284b..af83f838 100644 --- a/tests/components/__snapshots__/refs.test.ts.snap +++ b/tests/components/__snapshots__/refs.test.ts.snap @@ -4,13 +4,11 @@ exports[`refs basic use 1`] = ` "function anonymous(app, bdom, helpers ) { let { text, createBlock, list, multi, html, toggler, comment } = bdom; - let { singleRefSetter } = helpers; let block1 = createBlock(\`
\`); return function template(ctx, node, key = \\"\\") { - const refs = this.__owl__.refs; - const ref1 = singleRefSetter(refs, \`div\`); + let ref1 = (el) => this.__owl__.setRef((\`div\`), el); return block1([ref1]); } }" @@ -20,38 +18,54 @@ exports[`refs can use 2 refs with same name in a t-if/t-else situation 1`] = ` "function anonymous(app, bdom, helpers ) { let { text, createBlock, list, multi, html, toggler, comment } = bdom; - let { singleRefSetter, multiRefSetter } = helpers; let block2 = createBlock(\`
\`); let block3 = createBlock(\`\`); return function template(ctx, node, key = \\"\\") { - const refs = this.__owl__.refs; - const ref1 = multiRefSetter(refs, \`coucou\`); let b2,b3; if (ctx['state'].value) { + let ref1 = (el) => this.__owl__.setRef((\`coucou\`), el); b2 = block2([ref1]); } else { - b3 = block3([ref1]); + let ref2 = (el) => this.__owl__.setRef((\`coucou\`), el); + b3 = block3([ref2]); } return multi([b2, b3]); } }" `; +exports[`refs ref is unset when t-if goes to false after unrelated render 1`] = ` +"function anonymous(app, bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, comment } = bdom; + + let block2 = createBlock(\`
\`); + + return function template(ctx, node, key = \\"\\") { + let b2; + if (ctx['state'].show) { + let attr1 = ctx['state'].class; + let ref1 = (el) => this.__owl__.setRef((\`coucou\`), el); + b2 = block2([attr1, ref1]); + } + return multi([b2]); + } +}" +`; + exports[`refs refs and recursive templates 1`] = ` "function anonymous(app, bdom, helpers ) { let { text, createBlock, list, multi, html, toggler, comment } = bdom; - let { singleRefSetter } = helpers; const comp1 = app.createComponent(\`Test\`, true, false, false, false); let block1 = createBlock(\`

\`); return function template(ctx, node, key = \\"\\") { - const refs = this.__owl__.refs; - const ref1 = singleRefSetter(refs, \`root\`); let b2; + let ref1 = (el) => this.__owl__.setRef((\`root\`), el); let txt1 = ctx['props'].tree.value; if (ctx['props'].tree.child) { b2 = comp1({tree: ctx['props'].tree.child}, key + \`__1\`, node, this, null); @@ -65,18 +79,16 @@ exports[`refs refs and t-key 1`] = ` "function anonymous(app, bdom, helpers ) { let { text, createBlock, list, multi, html, toggler, comment } = bdom; - let { singleRefSetter } = helpers; let block2 = createBlock(\`\`); function slot1(ctx, node, key = \\"\\") { - const refs = this.__owl__.refs; - const ref1 = singleRefSetter(refs, \`myButton\`); let hdlr1 = [ctx['doSomething'], ctx]; + let ref1 = (el) => this.__owl__.setRef((\`myButton\`), el); return block2([hdlr1, ref1]); } @@ -128,16 +139,17 @@ exports[`refs throws if there are 2 same refs at the same time 1`] = ` "function anonymous(app, bdom, helpers ) { let { text, createBlock, list, multi, html, toggler, comment } = bdom; - let { singleRefSetter, multiRefSetter } = helpers; + let { makeRefWrapper } = helpers; let block2 = createBlock(\`
\`); let block3 = createBlock(\`\`); return function template(ctx, node, key = \\"\\") { - const refs = this.__owl__.refs; - const ref1 = multiRefSetter(refs, \`coucou\`); + let refWrapper = makeRefWrapper(this.__owl__); + let ref1 = refWrapper(\`coucou\`, (el) => this.__owl__.setRef((\`coucou\`), el)); const b2 = block2([ref1]); - const b3 = block3([ref1]); + let ref2 = refWrapper(\`coucou\`, (el) => this.__owl__.setRef((\`coucou\`), el)); + const b3 = block3([ref2]); return multi([b2, b3]); } }" diff --git a/tests/components/__snapshots__/slots.test.ts.snap b/tests/components/__snapshots__/slots.test.ts.snap index 2a3521ea..b8fe1a97 100644 --- a/tests/components/__snapshots__/slots.test.ts.snap +++ b/tests/components/__snapshots__/slots.test.ts.snap @@ -158,15 +158,14 @@ exports[`slots can render node with t-ref and Component in same slot 1`] = ` "function anonymous(app, bdom, helpers ) { let { text, createBlock, list, multi, html, toggler, comment } = bdom; - let { singleRefSetter, markRaw } = helpers; + let { markRaw } = helpers; const comp1 = app.createComponent(\`Child\`, true, false, false, true); const comp2 = app.createComponent(\`Child\`, true, true, false, true); let block2 = createBlock(\`
\`); function slot1(ctx, node, key = \\"\\") { - const refs = this.__owl__.refs; - const ref1 = singleRefSetter(refs, \`div\`); + let ref1 = (el) => this.__owl__.setRef((\`div\`), el); const b2 = block2([ref1]); const b3 = comp1({}, key + \`__1\`, node, this, null); return multi([b2, b3]); diff --git a/tests/components/__snapshots__/t_call.test.ts.snap b/tests/components/__snapshots__/t_call.test.ts.snap index ee11586e..375665ae 100644 --- a/tests/components/__snapshots__/t_call.test.ts.snap +++ b/tests/components/__snapshots__/t_call.test.ts.snap @@ -534,7 +534,7 @@ exports[`t-call t-call-context: ComponentNode is not looked up in the context 2` "function anonymous(app, bdom, helpers ) { let { text, createBlock, list, multi, html, toggler, comment } = bdom; - let { singleRefSetter, bind, capture, isBoundary, withDefault, setContextValue, markRaw } = helpers; + let { bind, capture, isBoundary, withDefault, setContextValue, markRaw } = helpers; const comp1 = app.createComponent(\`Child\`, true, true, false, false); let block2 = createBlock(\`
outside slot
\`); @@ -542,10 +542,9 @@ exports[`t-call t-call-context: ComponentNode is not looked up in the context 2` let block5 = createBlock(\`
\`); function slot1(ctx, node, key = \\"\\") { - const refs = this.__owl__.refs; - const ref2 = singleRefSetter(refs, \`myRef2\`); ctx = Object.create(ctx); ctx[isBoundary] = 1 + let ref2 = (el) => this.__owl__.setRef((\`myRef2\`), el); const b4 = block4([ref2]); setContextValue(ctx, \\"test\\", 3); let txt1 = ctx['test']; @@ -554,8 +553,7 @@ exports[`t-call t-call-context: ComponentNode is not looked up in the context 2` } return function template(ctx, node, key = \\"\\") { - const refs = this.__owl__.refs; - const ref1 = singleRefSetter(refs, \`myRef\`); + let ref1 = (el) => this.__owl__.setRef((\`myRef\`), el); const b2 = block2([ref1]); const ctx1 = capture(ctx); const b6 = comp1({prop: bind(this, ctx['method']),slots: markRaw({'default': {__render: slot1.bind(this), __ctx: ctx1}})}, key + \`__1\`, node, this, null); diff --git a/tests/components/refs.test.ts b/tests/components/refs.test.ts index 4e2cf07e..b6cf1fb7 100644 --- a/tests/components/refs.test.ts +++ b/tests/components/refs.test.ts @@ -90,6 +90,28 @@ describe("refs", () => { expect(test.ref.el!.tagName).toBe("DIV"); }); + test("ref is unset when t-if goes to false after unrelated render", async () => { + class Comp extends Component { + static template = xml`
`; + state = useState({ show: true, class: "test" }); + ref = useRef("coucou"); + } + + const comp = await mount(Comp, fixture); + expect(comp.ref.el).not.toBeNull(); + + comp.state.class = "test2"; + await nextTick(); + + comp.state.show = false; + await nextTick(); + expect(comp!.ref.el).toBeNull(); + + comp.state.show = true; + await nextTick(); + expect(comp!.ref.el).not.toBeNull(); + }); + test("throws if there are 2 same refs at the same time", async () => { const consoleWarn = console.warn; console.warn = jest.fn(); @@ -104,10 +126,10 @@ describe("refs", () => { const app = new App(Test, { test: true }); const mountProm = expect(app.mount(fixture)).rejects.toThrowError( - "Cannot have 2 elements with same ref name at the same time" + 'Cannot set the same ref more than once in the same component, ref "coucou" was set multiple times in Test' ); await expect(nextAppError(app)).resolves.toThrow( - "Cannot have 2 elements with same ref name at the same time" + 'Cannot set the same ref more than once in the same component, ref "coucou" was set multiple times in Test' ); await mountProm; expect(console.warn).toBeCalledTimes(1);