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 · Aug 31, 2021 · 681f821 · 681f821
1 parent 5dfc303
commit 681f821
Show file tree

Hide file tree

Showing 5 changed files with 90 additions and 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
@@ -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/...rver/src/main/java/com/vaadin/flow/server/communication/WebComponentBootstrapHandler.java b/...rver/src/main/java/com/vaadin/flow/server/communication/WebComponentBootstrapHandler.java
@@ -77,6 +77,7 @@ private WebComponentBootstrapContext(VaadinRequest request,
                 Function<VaadinRequest, String> 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
@@ -146,10 +146,10 @@ protected Collection<String> 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
@@ -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/...ication-theme/src/test/java/com/vaadin/flow/webcomponent/ApplicationThemeComponentIT.java b/...ication-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"));
+    }
 }