From 600f1e35d4354a8a5ed50b1eeb7c17d143e7173f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucas=20Lef=C3=A8vre?= Date: Wed, 8 Sep 2021 08:55:30 +0200 Subject: [PATCH] [IMP] package.json: update jest and rollup If we want to upgrade Typescript to version >4 (in this repo or another using owl), we also need jest >25. Jest <26 do not support Typescript >4. However, jest >26 has a few breaking changes that completely breaks owl in tests. This commit updates jest to the current latest version (27) and adapts the code accordingly. A few words on what changed --------------------------- 1. the default test environment is no longer `jsdom`. It is now manually configured to restore the previous env. 2. the jsdom version has been upgraded. This brings a few breaking changes, detailed later. 3. there's a bug in jsdom >16.4. Manually created `` (by qweb compilation) are recognized as "T" and not "t". Qweb thinks it's a component (since the first letter is capitalized), but it's not a component: boom, everything breaks. A fix has been proposed here jsdom/jsdom#3240. But it's not likely to land in jest in the short term (jest would need to update its dependency to the next major jsdom version). This commit works around the problem for now by creating `` slighty differently such that they are in an XML document from the start (and not HTML document). 4. the xml parser implementation changed to increase the strictness and correctness of XML parsing, according to specifications. A few tests needed to be adapted. https://github.com/jsdom/jsdom/commit/c96decf837ece54bdc550dfb7dca7e5d6c97bc2d 5. jest matcher `toHaveProperty` now check inherited properties. This breaks a few tests. Since the breaking assertions didn't bring a lot of value from a behavior point of view (it was more "white box" technical tests), they are removed in this commit https://github.com/facebook/jest/commit/1256f76a5a83034b51c7524142b60b099f69a7ab 6. jsdom now implements the behavior of links (``). One test was `click`ing on such a link, with the right click. Jsdom now tries to navigate to the pointed URl...but crashes because navigation is not implemented :( It turns out the test is probably not a valid/realistic scenario. On every tested browser (chrome - chromium - brave - edge -firefox - safari), a right click with the mouse triggers a `contextmenu` event and no `click` event. https://github.com/jsdom/jsdom/commit/cc95abc576f596ff7f3eaf8245f376e1f21aa485 --- package.json | 13 +++++++------ src/qweb/qweb.ts | 7 +++++-- tests/hooks.test.ts | 6 ------ tests/qweb/qweb.test.ts | 6 ++---- tests/router/link.test.ts | 2 +- 5 files changed, 15 insertions(+), 19 deletions(-) diff --git a/package.json b/package.json index f0753647..7e31123f 100644 --- a/package.json +++ b/package.json @@ -37,28 +37,29 @@ }, "homepage": "https://github.com/odoo/owl#readme", "devDependencies": { - "@types/jest": "^23.3.14", + "@types/jest": "^27.0.1", "@types/node": "^14.11.8", "chalk": "^3.0.0", "cpx": "^1.5.0", "current-git-branch": "^1.1.0", "git-rev-sync": "^1.12.0", "github-api": "^3.3.0", - "jest": "^23.6.0", - "jest-environment-jsdom": "^24.7.1", + "jest": "^27.1.0", + "jest-environment-jsdom": "^27.1.0", "live-server": "^1.2.1", "npm-run-all": "^4.1.5", "prettier": "^2.0.4", - "rollup": "^1.6.0", + "rollup": "^2.56.3", "rollup-plugin-terser": "^7.0.2", - "rollup-plugin-typescript2": "^0.27.3", + "rollup-plugin-typescript2": "^0.30.0", "sass": "^1.16.1", "source-map-support": "^0.5.10", - "ts-jest": "^23.10.5", + "ts-jest": "^27.0.5", "typescript": "^3.7.2", "uglify-es": "^3.3.9" }, "jest": { + "testEnvironment": "jsdom", "roots": [ "/src", "/tests" diff --git a/src/qweb/qweb.ts b/src/qweb/qweb.ts index b23d7d2d..c2bb445e 100644 --- a/src/qweb/qweb.ts +++ b/src/qweb/qweb.ts @@ -329,6 +329,9 @@ export class QWeb extends EventBus { * template, with the name given by the t-name attribute. */ addTemplates(xmlstr: string | Document) { + if (!xmlstr) { + return; + } const doc = typeof xmlstr === "string" ? parseXML(xmlstr) : xmlstr; const templates = doc.getElementsByTagName("templates")[0]; if (!templates) { @@ -557,7 +560,7 @@ export class QWeb extends EventBus { } if (node.tagName !== "t" && node.hasAttribute("t-call")) { - const tCallNode = document.createElement("t"); + const tCallNode = document.implementation.createDocument("http://www.w3.org/1999/xhtml", "t", null).documentElement; tCallNode.setAttribute("t-call", node.getAttribute("t-call")!); node.removeAttribute("t-call"); node.prepend(tCallNode); @@ -593,7 +596,7 @@ export class QWeb extends EventBus { throw new Error(`Unknown QWeb directive: '${attrName}'`); } if (node.tagName !== "t" && (attrName === "t-esc" || attrName === "t-raw")) { - const tNode = document.createElement("t"); + const tNode = document.implementation.createDocument("http://www.w3.org/1999/xhtml", "t", null).documentElement; tNode.setAttribute(attrName, node.getAttribute(attrName)!); for (let child of Array.from(node.childNodes)) { tNode.appendChild(child); diff --git a/tests/hooks.test.ts b/tests/hooks.test.ts index da91f655..52456e11 100644 --- a/tests/hooks.test.ts +++ b/tests/hooks.test.ts @@ -75,8 +75,6 @@ describe("hooks", () => { } const component = new MyComponent(); await component.mount(fixture); - expect(component).not.toHaveProperty("mounted"); - expect(component).not.toHaveProperty("willUnmount"); expect(fixture.innerHTML).toBe("
hey
"); expect(steps).toEqual(["mounted"]); component.unmount(); @@ -383,8 +381,6 @@ describe("hooks", () => { const component = new MyComponent(); await component.mount(fixture); - expect(component).not.toHaveProperty("patched"); - expect(component).not.toHaveProperty("willPatch"); expect(steps).toEqual([]); expect(fixture.innerHTML).toBe("
hey
"); @@ -630,8 +626,6 @@ describe("hooks", () => { const app = new App(); await app.mount(fixture); - expect(app).not.toHaveProperty("willStart"); - expect(app).not.toHaveProperty("willUpdateProps"); expect(fixture.innerHTML).toBe("
1
"); // NOTE: 'on2ndStart' appears first in the list even though diff --git a/tests/qweb/qweb.test.ts b/tests/qweb/qweb.test.ts index 2615e711..b87b9ec7 100644 --- a/tests/qweb/qweb.test.ts +++ b/tests/qweb/qweb.test.ts @@ -1862,8 +1862,7 @@ describe("t-ref", () => { describe("loading templates", () => { test("can initialize qweb with a string", () => { - const templates = ` - + const templates = `
jupiler
`; @@ -1872,8 +1871,7 @@ describe("loading templates", () => { }); test("can load a few templates from a xml string", () => { - const data = ` - + const data = `
  • ok
  • foo
    •  diff --git a/tests/router/link.test.ts b/tests/router/link.test.ts index c1f8f9e2..f325ea2b 100644 --- a/tests/router/link.test.ts +++ b/tests/router/link.test.ts @@ -80,7 +80,7 @@ describe("Link component", () => { await app.mount(fixture); expect(window.location.pathname).toBe("/users"); - var evt = new MouseEvent("click", { + var evt = new MouseEvent("contextmenu", { button: 1, });