Skip to content

Commit

Permalink
[FIX] components: do not crash when binding anonymous function
Browse files Browse the repository at this point in the history
It is not really useful, but it is possible to bind an anonymous
function prop. However, with the recent change on how the bind feature
works, it now crashes.

This commit makes sure that we properly apply the `bind` operation to
the function, and not to the last term of the function.
  • Loading branch information
ged-odoo authored and sdegueldre committed Mar 9, 2023
1 parent 532ab7f commit 0024f33
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 7 deletions.
2 changes: 1 addition & 1 deletion src/compiler/code_generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1135,7 +1135,7 @@ export class CodeGenerator {
name = _name;
switch (suffix) {
case "bind":
value = `${value}.bind(this)`;
value = `(${value}).bind(this)`;
break;
case "alike":
break;
Expand Down
30 changes: 27 additions & 3 deletions tests/components/__snapshots__/props.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ exports[`bound functions are considered 'alike' 1`] = `
return function template(ctx, node, key = \\"\\") {
const b2 = text(ctx['state'].val);
const b3 = comp1({fn: ctx['someFunction'].bind(this)}, key + \`__1\`, node, this, null);
const b3 = comp1({fn: (ctx['someFunction']).bind(this)}, key + \`__1\`, node, this, null);
return multi([b2, b3]);
}
}"
Expand All @@ -373,7 +373,7 @@ exports[`bound functions is not referentially equal after update 1`] = `
const comp1 = app.createComponent(\`Child\`, true, false, false, [\\"val\\"]);
return function template(ctx, node, key = \\"\\") {
return comp1({val: ctx['state'].val,fn: ctx['someFunction'].bind(this)}, key + \`__1\`, node, this, null);
return comp1({val: ctx['state'].val,fn: (ctx['someFunction']).bind(this)}, key + \`__1\`, node, this, null);
}
}"
`;
Expand All @@ -396,7 +396,7 @@ exports[`can bind function prop with bind suffix 1`] = `
const comp1 = app.createComponent(\`Child\`, true, false, false, []);
return function template(ctx, node, key = \\"\\") {
return comp1({doSomething: ctx['doSomething'].bind(this)}, key + \`__1\`, node, this, null);
return comp1({doSomething: (ctx['doSomething']).bind(this)}, key + \`__1\`, node, this, null);
}
}"
`;
Expand All @@ -411,3 +411,27 @@ exports[`can bind function prop with bind suffix 2`] = `
}
}"
`;
exports[`do not crash when binding anonymous function prop with bind suffix 1`] = `
"function anonymous(app, bdom, helpers
) {
let { text, createBlock, list, multi, html, toggler, comment } = bdom;
const comp1 = app.createComponent(\`Child\`, true, false, false, []);
return function template(ctx, node, key = \\"\\") {
const v1 = ctx['this'];
return comp1({doSomething: ((_val)=>v1.doSomething(_val)).bind(this)}, key + \`__1\`, node, this, null);
}
}"
`;
exports[`do not crash when binding anonymous function prop with bind suffix 2`] = `
"function anonymous(app, bdom, helpers
) {
let { text, createBlock, list, multi, html, toggler, comment } = bdom;
return function template(ctx, node, key = \\"\\") {
return text(\`child\`);
}
}"
`;
4 changes: 2 additions & 2 deletions tests/components/__snapshots__/slots.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ exports[`slots can define and call slots with bound params 1`] = `

return function template(ctx, node, key = \\"\\") {
const ctx1 = capture(ctx);
return comp1({slots: markRaw({'abc': {__render: slot1.bind(this), __ctx: ctx1, getValue: ctx['getValue'].bind(this)}})}, key + \`__1\`, node, this, null);
return comp1({slots: markRaw({'abc': {__render: slot1.bind(this), __ctx: ctx1, getValue: (ctx['getValue']).bind(this)}})}, key + \`__1\`, node, this, null);
}
}"
`;
Expand Down Expand Up @@ -1516,7 +1516,7 @@ exports[`slots simple default slot with params and bound function 2`] = `
let { callSlot } = helpers;

return function template(ctx, node, key = \\"\\") {
return callSlot(ctx, node, key, 'default', false, {fn: ctx['getValue'].bind(this)});
return callSlot(ctx, node, key, 'default', false, {fn: (ctx['getValue']).bind(this)});
}
}"
`;
Expand Down
2 changes: 1 addition & 1 deletion tests/components/__snapshots__/t_call.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ exports[`t-call t-call-context: ComponentNode is not looked up in the context 2`
let ref1 = (el) => this.__owl__.setRef((\`myRef\`), el);
const b2 = block2([ref1]);
const ctx1 = capture(ctx);
const b6 = comp1({prop: ctx['method'].bind(this),slots: markRaw({'default': {__render: slot1.bind(this), __ctx: ctx1}})}, key + \`__1\`, node, this, null);
const b6 = comp1({prop: (ctx['method']).bind(this),slots: markRaw({'default': {__render: slot1.bind(this), __ctx: ctx1}})}, key + \`__1\`, node, this, null);
return multi([b2, b6]);
}
}"
Expand Down
25 changes: 25 additions & 0 deletions tests/components/props.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,31 @@ test("can bind function prop with bind suffix", async () => {
expect(fixture.innerHTML).toBe("child");
});

test("do not crash when binding anonymous function prop with bind suffix", async () => {
class Child extends Component {
static template = xml`child`;
setup() {
this.props.doSomething(123);
}
}

let boundedThing: any = null;

class Parent extends Component {
static template = xml`<Child doSomething.bind="(val) => this.doSomething(val)"/>`;
static components = { Child };

doSomething(val: number) {
expect(val).toBe(123);
boundedThing = this;
}
}

const parent = await mount(Parent, fixture);
expect(boundedThing).toBe(parent);
expect(fixture.innerHTML).toBe("child");
});

test("bound functions is not referentially equal after update", async () => {
let isEqual = false;
class Child extends Component {
Expand Down

0 comments on commit 0024f33

Please sign in to comment.