From 8603d13d1961e0146df19aad65936c84eb6071ac Mon Sep 17 00:00:00 2001 From: Samuel Degueldre Date: Fri, 29 Apr 2022 08:33:23 +0000 Subject: [PATCH] [FIX] web, test_assetsbundle: make useAssets work in owl 2 Previously, the useAssets hook would load the templates from the given bundles into the qweb instance that was in the environment. In owl 2, qweb is no longer in the environment, and templates need to be loaded in the app. This commit fixes that by exporting a function to load templates in the main app from assets.js, when the main app is created, it should be written on the function in a similar vein to renderToString, so that all further calls to the function will load the templates in that app. This commit also adds an actual lazy-loading test in the test_assetsbundle module, as we don't want to pollute the manifest of web with bundles that only exist for one specific test. closes odoo/odoo#90122 Related: odoo/enterprise#26776 Signed-off-by: Aaron Bohy (aab) --- addons/web/static/src/core/assets.js | 207 +++++------------- .../web/static/src/core/errors/error_utils.js | 6 +- .../src/legacy/js/public/public_root.js | 4 +- addons/web/static/src/start.js | 3 +- .../static/src/views/graph/graph_renderer.js | 6 +- .../web/static/src/views/onboarding_banner.js | 27 +-- .../static/tests/core/utils/assets_tests.js | 18 +- odoo/addons/test_assetsbundle/__manifest__.py | 6 + .../lazy_test_component.js | 14 ++ .../lazy_test_component.scss | 3 + .../lazy_test_component.xml | 8 + .../static/tests/lazyloading_test.js | 47 ++++ 12 files changed, 157 insertions(+), 192 deletions(-) create mode 100644 odoo/addons/test_assetsbundle/static/tests/lazy_test_component/lazy_test_component.js create mode 100644 odoo/addons/test_assetsbundle/static/tests/lazy_test_component/lazy_test_component.scss create mode 100644 odoo/addons/test_assetsbundle/static/tests/lazy_test_component/lazy_test_component.xml create mode 100644 odoo/addons/test_assetsbundle/static/tests/lazyloading_test.js diff --git a/addons/web/static/src/core/assets.js b/addons/web/static/src/core/assets.js index f872aa0327b..f84799eb2d8 100644 --- a/addons/web/static/src/core/assets.js +++ b/addons/web/static/src/core/assets.js @@ -2,42 +2,10 @@ import { memoize } from "./utils/functions"; import { browser } from "./browser/browser"; - -const { onWillStart, useEnv } = owl; +import { registry } from "./registry"; class AssetsLoadingError extends Error {} -//------------------------------------------------------------------------------ -// Types -//------------------------------------------------------------------------------ - -/** - * An object describing a bundle to load - * @typedef {Object} Bundle - * @property {boolean} [templates] whether to load the qweb templates - * @property {boolean} [js] whether to load the js from the bundle. - * @property {boolean} [css] whether to load the css from the bundle. - */ - -/** - * An object describing a bundle to load. The keys are bundle names. - * @typedef {Object} Bundles - */ - -/** - * An object describing a loaded bundle - * @typedef {Object} LoadedBundle - * @property {XMLDocument} [templates] an XML document containing the owl - * templates defined in that bundle - * @property {true[]} [js] a promise or the resolution of that promise that signifies that the js has been loaded - * @property {true[]} [css] a promise or the resolution of that promise that signifies that the css has been loaded - */ - -/** - * An object describing a loaded bundle. The keys are bundle names. - * @typedef {Object} LoadedBundles - */ - /** * An object describing a bundle to load * @typedef {Object} BundleInfo @@ -45,9 +13,6 @@ class AssetsLoadingError extends Error {} * @property {string} [src] the url of the file for this bundle, for this type of file * @example `[{"type": "script", "src": "/web/assets/266-d34b0b4/documents_spreadsheet.o_spreadsheet.min.js"}]` */ -//------------------------------------------------------------------------------ -// Helpers -//------------------------------------------------------------------------------ /** * Loads the given url inside a script tag. @@ -80,7 +45,7 @@ export const loadJS = memoize(function loadJS(url) { * @param {string} url the url of the stylesheet * @returns {Promise} resolved when the stylesheet has been loaded */ -const loadCSS = memoize(function loadCSS(url) { +export const loadCSS = memoize(function loadCSS(url) { if (document.querySelector(`link[href="${url}"]`)) { // Already in the DOM and wasn't loaded through this function // Unfortunately there is no way to check whether a link has loaded @@ -102,21 +67,26 @@ const loadCSS = memoize(function loadCSS(url) { }); /** * Loads the qweb templates from a given bundle name. + * TODO: merge this into loadBundleDefinition? * - * @param {string} name the name of the bundle as declared in the manifest. - * @returns {Promise} A Promise of an XML document containing the - * owl templates. + * @param {string} bundle the name of the bundle as declared in the manifest. + * @returns {Promise} A Promise of an XML document containing + * the owl templates or an empty string if the bundle has none. */ -export const loadBundleTemplates = memoize(async function loadBundleTemplates(name) { - // TODO: quid of the "unique" in the URL? We can"t have one cache_hash - // for each and every bundle I"m guessing. - const bundleURL = `/web/webclient/qweb/${Date.now()}?bundle=${name}`; +export const fetchAndProcessTemplates = memoize(async function fetchAndProcessTemplates(bundle) { + // TODO: quid of the "unique" in the URL? We can't have one cache_hash + // for each and every bundle I'm guessing. + const bundleURL = `/web/webclient/qweb/${Date.now()}?bundle=${bundle}`; const templates = await (await browser.fetch(bundleURL)).text(); + if (!templates) { + return ""; + } return processTemplates(templates); }); /** - * Loads the content definition of a bundle + * Loads the content definition of a bundle. + * * @param {string} name the bundleName of the bundle as declared in the manifest. * @returns {Promise} A promise of the content definition of the bundle */ @@ -131,49 +101,30 @@ const bundlesCache = {}; * Loads a bundle. * * @param {string} name the name of the bundle to load - * @param {Bundle} options parts of the bundle to load (see Bundle typedef) - * @returns {Promise} + * @param {owl.App} [app] the app in which the bundle's templates should be + * loaded. Defaults to the app that's written on the function itself, this + * is considered the main app, and should be written on the function by the + * code that bootstraps the app. In most cases, this will be the webclient, + * and is set in start.js + * @returns {Promise} a promise that is resolved after the bundle has been + * loaded. */ -export async function loadBundle(name, options) { - const { templates, css, js } = options; +export async function loadBundle(name, app = loadBundle.app) { if (!bundlesCache[name]) { - bundlesCache[name] = { name }; - } - if (templates && !bundlesCache[name].templates) { - bundlesCache[name].templates = loadBundleTemplates(name); + bundlesCache[name] = Promise.all([ + fetchAndProcessTemplates(name).then((templates) => app.addTemplates(templates, app)), + loadBundleDefinition(name).then((bundleInfo) => + Promise.all([ + ...bundleInfo.filter((i) => i.type === "script").map((i) => loadJS(i.src)), + ...bundleInfo.filter((i) => i.type === "link").map((i) => loadCSS(i.src)), + ]) + ), + ]).then(() => {}); } - if ((css && !bundlesCache[name].css) || (js && !bundlesCache[name].js)) { - // we only load the bundle info (that is an RPC) if JS or CSS is requested on the bundle, - // and if it has not yet been requested. - /** @type BundleInfo[] */ - const bundleInfo = await loadBundleDefinition(name); - - if (options.js) { - bundlesCache[name].js = Promise.all( - bundleInfo.filter((i) => i.type === "script").map((i) => loadJS(i.src)) - ); - } - - if (options.css) { - bundlesCache[name].css = Promise.all( - bundleInfo.filter((i) => i.type === "link").map((i) => loadCSS(i.src)) - ); - } - } - - // Wait only for the requested keys - const entries = await Promise.all( - Object.keys(options).map(async (key) => [key, await bundlesCache[name][key]]) - ); - - return Object.fromEntries(entries); + return bundlesCache[name]; } -//------------------------------------------------------------------------------ -// Exports -//------------------------------------------------------------------------------ - /** * Process the qweb templates to obtain only the owl templates. This function * does NOT register the templates into Owl. @@ -187,7 +138,7 @@ export function processTemplates(templates) { // flagged with attribute `owl="1"`. The following lines removes the "owl" // attribute from the templates, so that it doesn't appear in the DOM. We // also remove the non-owl templates, as those shouldn't be loaded in the - // owl environment's QWeb, and will be loaded separately. + // owl application, and will be loaded separately. for (const template of [...doc.querySelector("templates").children]) { if (template.hasAttribute("owl")) { template.removeAttribute("owl"); @@ -207,86 +158,32 @@ export function processTemplates(templates) { * @deprecated * @param {string} xmlid The xmlid of the template that defines the public asset * @param {ORM} orm An ORM object capable of calling methods on models - * @returns {Promise} Resolved when the contents of the asset is loaded + * @returns {Promise} Resolved when the contents of the asset is loaded */ export const loadPublicAsset = memoize(async function loadPublicAsset(xmlid, orm) { const xml = await orm.call("ir.ui.view", "render_public_asset", [xmlid]); const doc = new DOMParser().parseFromString(`${xml}`, "text/xml"); - return loadAssets({ - cssLibs: [...doc.querySelectorAll("link[href]")].map((node) => node.getAttribute("href")), - jsLibs: [...doc.querySelectorAll("script[src]")].map((node) => node.getAttribute("src")), - }); + return Promise.all([ + ...[...doc.querySelectorAll("link[href]")].map((el) => loadCSS(el.getAttribute("href"))), + ...[...doc.querySelectorAll("script[src]")].map((el) => loadJS(el.getAttribute("src"))), + ]); }); +const { Component, xml, onWillStart } = owl; /** - * Loads the given assets. Currently, when passing bundles, only the templates - * key is supported, as loading a bundle's JS/CSS asynchronously requires some - * groundwork. - * - * @param {Object} assets - * @param {Bundles} [assets.bundles] an object describing the bundles to load. - * The keys are bundle names, and the values describe whether to load the - * templates, js and/or css from that bundle. - * @param {string[]} [assets.jsLibs] urls of the javascript libraries to load - * @param {string[]} [assets.cssLibs] urls of the css libraries to load - * @returns {Promise<{[bundles: LoadedBundle[]]}>} An object describing the loaded - * assets. The js and css libs are loaded globally and will be loaded when - * this promise is resolved, the owl xml templates will be loaded in the - * `templates` key of each LoadedBundle object. + * Utility component that loads an asset bundle before instanciating a component */ -export async function loadAssets(assets) { - const proms = []; - const loadedAssets = {}; - if ("bundles" in assets) { - const bundles = Object.entries(assets.bundles); - const bundlesProm = Promise.all( - bundles.map(async ([name, options]) => [name, await loadBundle(name, options)]) - ).then((loadedBundles) => { - loadedAssets.bundles = Object.fromEntries(loadedBundles); +export class LazyComponent extends Component { + setup() { + onWillStart(async () => { + await loadBundle(this.props.bundle); + this.Component = registry.category("lazy_components").get(this.props.Component); }); - proms.push(bundlesProm); } - if ("jsLibs" in assets) { - proms.push(Promise.all(assets.jsLibs.map(loadJS))); - } - if ("cssLibs" in assets) { - proms.push(Promise.all(assets.cssLibs.map(loadCSS))); - } - await Promise.all(proms); - return loadedAssets; -} - -/** - * Loads the given assets, and adds the loaded owl templates into the current - * environment's qweb instance. - * - * @param {Object} assets - * @param {Bundles} [assets.bundles] the bundles to load. See above typedef for - * details. - * @param {string[]} [assets.jsLibs] urls of the javascript libraries to load - * @param {string[]} [assets.cssLibs] urls of the css libraries to load - */ -export function useAssets(assets) { - const env = useEnv(); - onWillStart(async () => { - const loadedAssets = await loadAssets(assets); - // TODO: { js, css } = loadedAssets when we support lazy loading js/css from bundles - const { bundles } = loadedAssets; - if (bundles) { - const templateDocs = Object.values(bundles).map(({ templates }) => templates); - // Some templates might already be defined by another bundle, but need - // to be included in the current bundle for primary inherits to work. - // We don't want to add those again. We also have to add them to qweb - // one bundle at a time in case multiple bundles were specified that - // include the same template (e.g. for inheritance) - for (const xmlDoc of templateDocs) { - for (const template of [...xmlDoc.querySelector("templates").children]) { - if (template.getAttribute("t-name") in env.qweb.templates) { - template.remove(); - } - } - env.qweb.addTemplates(xmlDoc); - } - } - }); } +LazyComponent.template = xml``; +LazyComponent.props = { + Component: String, + bundle: String, + props: { type: Object, optional: true }, +}; diff --git a/addons/web/static/src/core/errors/error_utils.js b/addons/web/static/src/core/errors/error_utils.js index 67ecdc60290..56364b5610e 100644 --- a/addons/web/static/src/core/errors/error_utils.js +++ b/addons/web/static/src/core/errors/error_utils.js @@ -1,6 +1,6 @@ /** @odoo-module **/ -import { loadAssets } from "../assets"; +import { loadJS } from "../assets"; import { isBrowserChrome } from "../browser/feature_detection"; /** @@ -49,9 +49,7 @@ export function formatTraceback(error) { export async function annotateTraceback(error) { const traceback = formatTraceback(error); try { - await loadAssets({ - jsLibs: ["/web/static/lib/stacktracejs/stacktrace.js"], - }); + await loadJS("/web/static/lib/stacktracejs/stacktrace.js"); } catch (_e) { return traceback; } diff --git a/addons/web/static/src/legacy/js/public/public_root.js b/addons/web/static/src/legacy/js/public/public_root.js index acf39b2c431..5401487f1f8 100644 --- a/addons/web/static/src/legacy/js/public/public_root.js +++ b/addons/web/static/src/legacy/js/public/public_root.js @@ -21,7 +21,7 @@ import { } from "../../utils"; import { standaloneAdapter } from "web.OwlCompatibility"; -import { loadBundleTemplates } from "@web/core/assets"; +import { fetchAndProcessTemplates } from "@web/core/assets"; import { makeEnv, startServices } from "@web/env"; import { MainComponentsContainer } from "@web/core/main_components_container"; import { browser } from '@web/core/browser/browser'; @@ -405,7 +405,7 @@ export async function createPublicRoot(RootWidget) { const wowlEnv = makeEnv(); - const templates = await loadBundleTemplates("web.assets_frontend"); + const templates = await fetchAndProcessTemplates("web.assets_frontend"); window.__OWL_TEMPLATES__ = templates; await startServices(wowlEnv); mapLegacyEnvToWowlEnv(legacyEnv, wowlEnv); diff --git a/addons/web/static/src/start.js b/addons/web/static/src/start.js index bf0bb0ead39..f0b3361cd30 100644 --- a/addons/web/static/src/start.js +++ b/addons/web/static/src/start.js @@ -3,7 +3,7 @@ import { makeEnv, startServices } from "./env"; import { legacySetupProm } from "./legacy/legacy_setup"; import { mapLegacyEnvToWowlEnv } from "./legacy/utils"; -import { processTemplates } from "./core/assets"; +import { processTemplates, loadBundle } from "./core/assets"; import { localization } from "@web/core/l10n/localization"; import { session } from "@web/session"; import { renderToString } from "./core/utils/render"; @@ -47,6 +47,7 @@ export async function startWebClient(Webclient) { translateFn: env._t, }); renderToString.app = app; + loadBundle.app = app; const root = await app.mount(document.body); const classList = document.body.classList; if (localization.direction === "rtl") { diff --git a/addons/web/static/src/views/graph/graph_renderer.js b/addons/web/static/src/views/graph/graph_renderer.js index e6c14294d10..1e998c8f350 100644 --- a/addons/web/static/src/views/graph/graph_renderer.js +++ b/addons/web/static/src/views/graph/graph_renderer.js @@ -5,10 +5,10 @@ import { BORDER_WHITE, DEFAULT_BG, getColor, hexToRGBA } from "./colors"; import { formatFloat } from "@web/fields/formatters"; import { SEP } from "./graph_model"; import { sortBy } from "@web/core/utils/arrays"; -import { useAssets } from "@web/core/assets"; +import { loadJS } from "@web/core/assets"; import { renderToString } from "@web/core/utils/render"; -const { Component, onWillUnmount, useEffect, useRef } = owl; +const { Component, onWillUnmount, useEffect, useRef, onWillStart } = owl; const NO_DATA = _lt("No data"); @@ -50,7 +50,7 @@ export class GraphRenderer extends Component { this.tooltip = null; this.legendTooltip = null; - useAssets({ jsLibs: ["/web/static/lib/Chart/Chart.js"] }); + onWillStart(() => loadJS(["/web/static/lib/Chart/Chart.js"])); useEffect(() => this.renderChart()); onWillUnmount(this.onWillUnmount); diff --git a/addons/web/static/src/views/onboarding_banner.js b/addons/web/static/src/views/onboarding_banner.js index ef809822c30..b886c386333 100644 --- a/addons/web/static/src/views/onboarding_banner.js +++ b/addons/web/static/src/views/onboarding_banner.js @@ -1,6 +1,6 @@ /**@odoo-module */ -import { loadAssets } from "@web/core/assets"; +import { loadCSS, loadJS } from "@web/core/assets"; import { useService } from "@web/core/utils/hooks"; import { useActionLinks } from "@web/views/helpers/view_hook"; @@ -33,21 +33,16 @@ export class OnboardingBanner extends Component { } const banner = new DOMParser().parseFromString(response.html, "text/html"); - const assets = { - jsLibs: [], - cssLibs: [], - }; - banner - .querySelectorAll(`link[rel="stylesheet"] , script[type="text/javascript"]`) - .forEach((elem) => { - if (elem.tagName === "SCRIPT") { - assets.jsLibs.push(elem.src); - } else if (elem.tagName === "LINK") { - assets.cssLibs.push(elem.href); - } - elem.remove(); - }); - await loadAssets(assets); + await Promise.all([ + ...[...banner.querySelectorAll(`script[type="text/javascript"]`)].map((el) => { + el.remove(); + return loadJS(el.getAttribute("src")); + }), + ...[...banner.querySelectorAll(`link[rel="stylesheet"]`)].map((el) => { + el.remove(); + return loadCSS(el.getAttribute("href")); + }), + ]); return markup(new XMLSerializer().serializeToString(banner)); } } diff --git a/addons/web/static/tests/core/utils/assets_tests.js b/addons/web/static/tests/core/utils/assets_tests.js index 61cd91f5478..b0ae0f682f6 100644 --- a/addons/web/static/tests/core/utils/assets_tests.js +++ b/addons/web/static/tests/core/utils/assets_tests.js @@ -1,15 +1,15 @@ /** @odoo-module **/ -import { loadAssets } from "@web/core/assets"; +import { loadJS, loadCSS, fetchAndProcessTemplates } from "@web/core/assets"; import { patchWithCleanup } from "@web/../tests/helpers/utils"; import { browser } from "@web/core/browser/browser"; QUnit.module("utils", () => { QUnit.module("Assets"); - QUnit.test("loadAssets: load invalid JS lib", function (assert) { + QUnit.test("loadJS: load invalid JS lib", function (assert) { assert.rejects( - loadAssets({ jsLibs: ["/some/invalid/file.js"] }), + loadJS("/some/invalid/file.js"), new RegExp("The loading of /some/invalid/file.js failed"), "Trying to load an invalid file rejects the promise" ); @@ -19,9 +19,9 @@ QUnit.module("utils", () => { ); }); - QUnit.test("loadAssets: load invalid CSS lib", function (assert) { + QUnit.test("loadCSS: load invalid CSS lib", function (assert) { assert.rejects( - loadAssets({ cssLibs: ["/some/invalid/file.css"] }), + loadCSS("/some/invalid/file.css"), new RegExp("The loading of /some/invalid/file.css failed"), "Trying to load an invalid file rejects the promise" ); @@ -31,7 +31,7 @@ QUnit.module("utils", () => { ); }); - QUnit.test("loadAssets: load invalid bundle", function (assert) { + QUnit.test("fetchAndProcessTemplates: load invalid bundle", function (assert) { let lastFetchedURL; patchWithCleanup(browser, { fetch: function (url) { @@ -40,11 +40,7 @@ QUnit.module("utils", () => { }, }); assert.rejects( - loadAssets({ - bundles: { - "web.some_invalid_bundle": { templates: true }, - }, - }), + fetchAndProcessTemplates("web.some_invalid_bundle"), "Trying to load an invalid bundle rejects the promise" ); assert.ok( diff --git a/odoo/addons/test_assetsbundle/__manifest__.py b/odoo/addons/test_assetsbundle/__manifest__.py index a324a952f72..850fff6e039 100644 --- a/odoo/addons/test_assetsbundle/__manifest__.py +++ b/odoo/addons/test_assetsbundle/__manifest__.py @@ -57,6 +57,12 @@ ('include', 'test_assetsbundle.manifest4'), ], 'test_assetsbundle.manifest_multi_module1': [], + 'web.qunit_suite_tests': [ + 'test_assetsbundle/static/tests/lazyloading_test.js' + ], + 'test_assetsbundle.lazy_test_component': [ + 'test_assetsbundle/static/tests/lazy_test_component/**/*', + ], }, 'license': 'LGPL-3', } diff --git a/odoo/addons/test_assetsbundle/static/tests/lazy_test_component/lazy_test_component.js b/odoo/addons/test_assetsbundle/static/tests/lazy_test_component/lazy_test_component.js new file mode 100644 index 00000000000..e32e83edb2f --- /dev/null +++ b/odoo/addons/test_assetsbundle/static/tests/lazy_test_component/lazy_test_component.js @@ -0,0 +1,14 @@ +/** @odoo-module */ + +import { registry } from "@web/core/registry"; + +const { Component } = owl; + +export class LazyTestComponent extends Component { + setup() { + this.props.onCreated(); + } +} +LazyTestComponent.template = "test_assetsbundle.LazyTestComponent"; + +registry.category("lazy_components").add("LazyTestComponent", LazyTestComponent); diff --git a/odoo/addons/test_assetsbundle/static/tests/lazy_test_component/lazy_test_component.scss b/odoo/addons/test_assetsbundle/static/tests/lazy_test_component/lazy_test_component.scss new file mode 100644 index 00000000000..4522b000cf6 --- /dev/null +++ b/odoo/addons/test_assetsbundle/static/tests/lazy_test_component/lazy_test_component.scss @@ -0,0 +1,3 @@ +.o_lazy_test_component { + background-color: rgb(165, 94, 117); +} diff --git a/odoo/addons/test_assetsbundle/static/tests/lazy_test_component/lazy_test_component.xml b/odoo/addons/test_assetsbundle/static/tests/lazy_test_component/lazy_test_component.xml new file mode 100644 index 00000000000..c36692927a3 --- /dev/null +++ b/odoo/addons/test_assetsbundle/static/tests/lazy_test_component/lazy_test_component.xml @@ -0,0 +1,8 @@ + + + + +
Lazy Component!
+ + + diff --git a/odoo/addons/test_assetsbundle/static/tests/lazyloading_test.js b/odoo/addons/test_assetsbundle/static/tests/lazyloading_test.js new file mode 100644 index 00000000000..401fa7e28a7 --- /dev/null +++ b/odoo/addons/test_assetsbundle/static/tests/lazyloading_test.js @@ -0,0 +1,47 @@ +/** @odoo-module */ + +import { loadBundle, LazyComponent } from "@web/core/assets"; +import { registerCleanup } from "@web/../tests/helpers/cleanup"; +import { makeTestEnv } from "@web/../tests/helpers/mock_env"; +import { getFixture, patchWithCleanup } from "@web/../tests/helpers/utils"; + +const { Component, App, xml } = owl; + +QUnit.module("utils", () => { + QUnit.module("Assets"); + + QUnit.test("LazyComponent loads the required bundle", async function (assert) { + class Test extends Component { + get childProps() { + return { + onCreated: () => assert.step("Lazy test component created"), + }; + } + } + Test.template = xml` + + `; + Test.components = { LazyComponent }; + + const target = getFixture(); + const app = new App(Test, { + test: true, + env: makeTestEnv(), + }); + registerCleanup(() => app.destroy()); + patchWithCleanup(loadBundle, { app }); + + assert.verifySteps([]); + await app.mount(target); + assert.verifySteps(["Lazy test component created"]); + assert.strictEqual( + target.innerHTML, + `
Lazy Component!
` + ); + assert.strictEqual( + window.getComputedStyle(target.querySelector(".o_lazy_test_component")).backgroundColor, + "rgb(165, 94, 117)", + "scss file was loaded" + ); + }); +});