mirror of
https://github.com/odoo/owl.git
synced 2025-10-06 19:59:41 +07:00
[FIX] runtime: correctly throw an error for duplicate object keys
Currently when checking for duplicate keys, we insert the value of the key as is in a set then check for unicity against those. When the key is an object, we check for duplicates based on object identity, whereas the keys are used by owl as strings, and so using objects can cause duplicate key errors that do not throw correctly but crash in the owl internals. This commit fixes that by making the duplicate checking code serialize the key to string before insertion and when comparing against existing keys.
This commit is contained in:
committed by
Bruno Boi
parent
a1f22829c1
commit
d546244fc3
@@ -877,9 +877,9 @@ export class CodeGenerator {
|
||||
// Throw error on duplicate keys in dev mode
|
||||
this.helpers.add("OwlError");
|
||||
this.addLine(
|
||||
`if (keys${block.id}.has(key${this.target.loopLevel})) { throw new OwlError(\`Got duplicate key in t-foreach: \${key${this.target.loopLevel}}\`)}`
|
||||
`if (keys${block.id}.has(String(key${this.target.loopLevel}))) { throw new OwlError(\`Got duplicate key in t-foreach: \${key${this.target.loopLevel}}\`)}`
|
||||
);
|
||||
this.addLine(`keys${block.id}.add(key${this.target.loopLevel});`);
|
||||
this.addLine(`keys${block.id}.add(String(key${this.target.loopLevel}));`);
|
||||
}
|
||||
let id: string;
|
||||
if (ast.memo) {
|
||||
|
||||
@@ -53,8 +53,8 @@ exports[`list of components crash on duplicate key in dev mode 1`] = `
|
||||
for (let i1 = 0; i1 < l_block1; i1++) {
|
||||
ctx[\`item\`] = v_block1[i1];
|
||||
const key1 = 'child';
|
||||
if (keys1.has(key1)) { throw new OwlError(\`Got duplicate key in t-foreach: \${key1}\`)}
|
||||
keys1.add(key1);
|
||||
if (keys1.has(String(key1))) { throw new OwlError(\`Got duplicate key in t-foreach: \${key1}\`)}
|
||||
keys1.add(String(key1));
|
||||
const props1 = {};
|
||||
helpers.validateProps(\`Child\`, props1, this);
|
||||
c_block1[i1] = withKey(comp1(props1, key + \`__1__\${key1}\`, node, this, null), key1);
|
||||
@@ -75,6 +75,42 @@ exports[`list of components crash on duplicate key in dev mode 2`] = `
|
||||
}"
|
||||
`;
|
||||
|
||||
exports[`list of components crash when using object as keys that serialize to the same string 1`] = `
|
||||
"function anonymous(app, bdom, helpers
|
||||
) {
|
||||
let { text, createBlock, list, multi, html, toggler, comment } = bdom;
|
||||
let { prepareList, OwlError, withKey } = helpers;
|
||||
const comp1 = app.createComponent(\`Child\`, true, false, false, true);
|
||||
|
||||
return function template(ctx, node, key = \\"\\") {
|
||||
ctx = Object.create(ctx);
|
||||
const [k_block1, v_block1, l_block1, c_block1] = prepareList([{},{}]);;
|
||||
const keys1 = new Set();
|
||||
for (let i1 = 0; i1 < l_block1; i1++) {
|
||||
ctx[\`item\`] = v_block1[i1];
|
||||
const key1 = ctx['item'];
|
||||
if (keys1.has(String(key1))) { throw new OwlError(\`Got duplicate key in t-foreach: \${key1}\`)}
|
||||
keys1.add(String(key1));
|
||||
const props1 = {};
|
||||
helpers.validateProps(\`Child\`, props1, this);
|
||||
c_block1[i1] = withKey(comp1(props1, key + \`__1__\${key1}\`, node, this, null), key1);
|
||||
}
|
||||
return list(c_block1);
|
||||
}
|
||||
}"
|
||||
`;
|
||||
|
||||
exports[`list of components crash when using object as keys that serialize to the same string 2`] = `
|
||||
"function anonymous(app, bdom, helpers
|
||||
) {
|
||||
let { text, createBlock, list, multi, html, toggler, comment } = bdom;
|
||||
|
||||
return function template(ctx, node, key = \\"\\") {
|
||||
return text(\`\`);
|
||||
}
|
||||
}"
|
||||
`;
|
||||
|
||||
exports[`list of components list of sub components inside other nodes 1`] = `
|
||||
"function anonymous(app, bdom, helpers
|
||||
) {
|
||||
|
||||
@@ -331,4 +331,32 @@ describe("list of components", () => {
|
||||
console.info = consoleInfo;
|
||||
expect(mockConsoleWarn).toBeCalledTimes(1);
|
||||
});
|
||||
|
||||
test("crash when using object as keys that serialize to the same string", async () => {
|
||||
const consoleInfo = console.info;
|
||||
console.info = jest.fn();
|
||||
class Child extends Component {
|
||||
static template = xml``;
|
||||
}
|
||||
|
||||
class Parent extends Component {
|
||||
static template = xml`
|
||||
<t t-foreach="[{}, {}]" t-as="item" t-key="item">
|
||||
<Child/>
|
||||
</t>
|
||||
`;
|
||||
static components = { Child };
|
||||
}
|
||||
|
||||
const app = new App(Parent, { test: true });
|
||||
const mountProm = expect(app.mount(fixture)).rejects.toThrow(
|
||||
"Got duplicate key in t-foreach: [object Object]"
|
||||
);
|
||||
await expect(nextAppError(app)).resolves.toThrow(
|
||||
"Got duplicate key in t-foreach: [object Object]"
|
||||
);
|
||||
await mountProm;
|
||||
console.info = consoleInfo;
|
||||
expect(mockConsoleWarn).toBeCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user