From 681f821ac5fa9d315a69cd33c6cb463e68335370 Mon Sep 17 00:00:00 2001 From: caalador Date: Fri, 27 Aug 2021 15:42:13 +0300 Subject: [PATCH] fix: keep track of css injected to document (#11635) Stops css/fonts from being loaded multiple times to the document with embedded applications. Closes #11608 # Conflicts: # flow-server/src/main/resources/plugins/application-theme-plugin/theme-generator.js --- .../vaadin/flow/server/BootstrapHandler.java | 27 +++++++++ .../WebComponentBootstrapHandler.java | 1 + .../server/frontend/TaskUpdateImports.java | 6 +- .../theme-generator.js | 55 +++++++++++++++---- .../ApplicationThemeComponentIT.java | 14 +++++ 5 files changed, 90 insertions(+), 13 deletions(-) diff --git a/flow-server/src/main/java/com/vaadin/flow/server/BootstrapHandler.java b/flow-server/src/main/java/com/vaadin/flow/server/BootstrapHandler.java index bb757ae1bd9..250e0e65b8f 100644 --- a/flow-server/src/main/java/com/vaadin/flow/server/BootstrapHandler.java +++ b/flow-server/src/main/java/com/vaadin/flow/server/BootstrapHandler.java @@ -184,6 +184,8 @@ protected static class BootstrapContext { private JsonObject applicationParameters; private BootstrapUriResolver uriResolver; + private boolean initTheme = true; + /** * Creates a new context instance using the given parameters. * @@ -237,6 +239,22 @@ public VaadinSession getSession() { return session; } + /** + * Should custom theme be initialized. + * @return true if theme should initialize + */ + public boolean isInitTheme() { + return initTheme; + } + + /** + * Set if custom theme should be initialized. + * @param initTheme initializeTheme + */ + public void setInitTheme(boolean initTheme) { + this.initTheme = initTheme; + } + /** * Gets the UI. * @@ -567,6 +585,15 @@ public Document getBootstrapPage(BootstrapContext context) { handleThemeContents(context, document); } + // To not init theme for webcomponents we use a flag on the dom + if (context.isInitTheme() && context.getTheme().isPresent() + && !"".equals(context.getTheme().get().getName())) { + head.prependElement("script").attr("type", "text/javascript") + .appendChild(new DataNode( + "window.Vaadin = window.Vaadin || {}; window.Vaadin.theme = window.Vaadin.theme || {};" + + "window.Vaadin.theme.flowBootstrap = {}")); + } + if (!config.isProductionMode()) { if (config.isBowerMode()) { exportBowerUsageStatistics(document); diff --git a/flow-server/src/main/java/com/vaadin/flow/server/communication/WebComponentBootstrapHandler.java b/flow-server/src/main/java/com/vaadin/flow/server/communication/WebComponentBootstrapHandler.java index 4a9a15c1d93..0c60975c3cc 100644 --- a/flow-server/src/main/java/com/vaadin/flow/server/communication/WebComponentBootstrapHandler.java +++ b/flow-server/src/main/java/com/vaadin/flow/server/communication/WebComponentBootstrapHandler.java @@ -77,6 +77,7 @@ private WebComponentBootstrapContext(VaadinRequest request, Function callback) { super(request, response, ui.getInternals().getSession(), ui, callback); + setInitTheme(false); } @Override diff --git a/flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskUpdateImports.java b/flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskUpdateImports.java index f2d046f822a..d062115db08 100644 --- a/flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskUpdateImports.java +++ b/flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskUpdateImports.java @@ -146,10 +146,10 @@ protected Collection getThemeLines() { if (hasApplicationTheme) { // If we define a theme name we need to import - // theme/theme.js + // theme/theme.js on flow bootstrap + lines.add("import {applyTheme} from 'generated/theme.js';"); lines.add( - "import {applyTheme} from 'generated/theme.js';"); - lines.add("applyTheme(document);"); + "if(window.Vaadin.theme.flowBootstrap) applyTheme(document);"); } if (theme != null) { diff --git a/flow-server/src/main/resources/plugins/application-theme-plugin/theme-generator.js b/flow-server/src/main/resources/plugins/application-theme-plugin/theme-generator.js index 9ea9aa32e3b..6a717711cd5 100644 --- a/flow-server/src/main/resources/plugins/application-theme-plugin/theme-generator.js +++ b/flow-server/src/main/resources/plugins/application-theme-plugin/theme-generator.js @@ -82,7 +82,13 @@ const createLinkReferences = (css, target) => { const injectGlobalCssMethod = ` // target: Document | ShadowRoot export const injectGlobalCss = (css, target, first) => { - + if(target === document) { + const hash = getHash(css); + if (window.Vaadin.theme.injectedGlobalCss.indexOf(hash) !== -1) { + return; + } + window.Vaadin.theme.injectedGlobalCss.push(hash); + } const sheet = new CSSStyleSheet(); sheet.replaceSync(createLinkReferences(css,target)); if (first) { @@ -151,7 +157,6 @@ function generateThemeFile(themeFolder, themeName, themeProperties, productionMo const themeIdentifier = '_vaadintheme_' + themeName + '_'; const lumoCssFlag = '_vaadinthemelumoimports_'; - const globalCssFlag = themeIdentifier + 'globalCss'; const componentCssFlag = themeIdentifier + 'componentCss'; if (!fs.existsSync(styles)) { @@ -252,20 +257,50 @@ function generateThemeFile(themeFolder, themeName, themeProperties, productionMo themeFile += imports.join(''); themeFile += ` -window.Vaadin = window.Vaadin || {}; -window.Vaadin.Flow = window.Vaadin.Flow || {}; -window.Vaadin.Flow['${globalCssFlag}'] = window.Vaadin.Flow['${globalCssFlag}'] || []; +window.Vaadin = window.Vaadin || {}; +window.Vaadin.theme = window.Vaadin.theme || {}; +window.Vaadin.theme.injectedGlobalCss = []; + +/** + * Calculate a 32 bit FNV-1a hash + * Found here: https://gist.github.com/vaiorabbit/5657561 + * Ref.: http://isthe.com/chongo/tech/comp/fnv/ + * + * @param {string} str the input value + * @returns {string} 32 bit (as 8 byte hex string) + */ +function hashFnv32a(str) { + /*jshint bitwise:false */ + let i, l, hval = 0x811c9dc5; + + for (i = 0, l = str.length; i < l; i++) { + hval ^= str.charCodeAt(i); + hval += (hval << 1) + (hval << 4) + (hval << 7) + (hval << 8) + (hval << 24); + } + + // Convert to 8 digit hex string + return ("0000000" + (hval >>> 0).toString(16)).substr(-8); +} + +/** + * Calculate a 64 bit hash for the given input. + * Double hash is used to significantly lower the collision probability. + * + * @param {string} input value to get hash for + * @returns {string} 64 bit (as 16 byte hex string) + */ +function getHash(input) { + let h1 = hashFnv32a(input); // returns 32 bit (as 8 byte hex string) + return h1 + hashFnv32a(h1 + input); +} `; // Don't format as the generated file formatting will get wonky! // If targets check that we only register the style parts once, checks exist for global css and component css const themeFileApply = `export const applyTheme = (target) => { ${parentTheme} - const injectGlobal = (window.Vaadin.Flow['${globalCssFlag}'].length === 0) || (!window.Vaadin.Flow['${globalCssFlag}'].includes(target) && target !== document); - if (injectGlobal) { - ${globalCssCode.join('')} - window.Vaadin.Flow['${globalCssFlag}'].push(target); - } + ${globalCssCode.join('')} + if (!document['${componentCssFlag}']) { ${componentCssCode.join('')} document['${componentCssFlag}'] = true; diff --git a/flow-tests/test-embedding/test-embedding-application-theme/src/test/java/com/vaadin/flow/webcomponent/ApplicationThemeComponentIT.java b/flow-tests/test-embedding/test-embedding-application-theme/src/test/java/com/vaadin/flow/webcomponent/ApplicationThemeComponentIT.java index aecfaf97fc1..ef79c1c7bd3 100644 --- a/flow-tests/test-embedding/test-embedding-application-theme/src/test/java/com/vaadin/flow/webcomponent/ApplicationThemeComponentIT.java +++ b/flow-tests/test-embedding/test-embedding-application-theme/src/test/java/com/vaadin/flow/webcomponent/ApplicationThemeComponentIT.java @@ -220,4 +220,18 @@ public void stylesCssImport_externalLinkAddedToShadowroot() { Assert.assertTrue("Missing link for external url", linkUrls .contains("https://fonts.googleapis.com/css?family=Itim")); } + + @Test + public void multipleSameEmbedded_cssTargetingDocumentShouldOnlyAddElementsOneTime() { + open(); + checkLogsForErrors(); + Assert.assertEquals( + "document.css adds 2 font links and those should not duplicate", + 2l, getCommandExecutor().executeScript( + "return document.head.getElementsByTagName('link').length")); + Assert.assertEquals( + "Project contains 2 css injections to document and both should be hashed", + 2l, getCommandExecutor().executeScript( + "return window.Vaadin.theme.injectedGlobalCss.length")); + } }