fix: apply CssImport to exported webcomponent (#19740) (#19745)

CssImport annotation with just a value attribute can be injected automatically to shadow root of all exported web components (embedded applications) using Constructable StyleSheets. WebComponentExporter should have a theme to make automation work properly in theme-generator.js. Theme property "autoInjectGlobalCssImports": true in theme.json enable auto injection. Disabled by default. Fixes: #19700 Co-authored-by: Tomi Virtanen <[email protected]> Co-authored-by: Marco Collovati <[email protected]>
vaadin · Aug 5, 2024 · 5c077f7 · 5c077f7
1 parent ccc2e17
commit 5c077f7
Show file tree

Hide file tree

Showing 11 changed files with 193 additions and 25 deletions.
diff --git a/flow-server/src/main/java/com/vaadin/flow/server/frontend/AbstractUpdateImports.java b/flow-server/src/main/java/com/vaadin/flow/server/frontend/AbstractUpdateImports.java
@@ -33,12 +33,14 @@
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.List;
+import java.util.ListIterator;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Objects;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 import java.util.function.UnaryOperator;
+import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -76,6 +78,7 @@ abstract class AbstractUpdateImports implements Runnable {
             + " tpl.innerHTML = block;\n"
             + " document.head.appendChild(tpl.content);\n" + "}";
     private static final String IMPORT_INJECT = "import { injectGlobalCss } from 'Frontend/generated/jar-resources/theme-util.js';\n";
+    private static final String IMPORT_WC_INJECT = "import { injectGlobalWebcomponentCss } from 'Frontend/generated/jar-resources/theme-util.js';\n";
 
     private static final String CSS_IMPORT = "import $cssFromFile_%d from '%s';%n";
     private static final String CSS_IMPORT_AND_MAKE_LIT_CSS = CSS_IMPORT
@@ -87,6 +90,10 @@ abstract class AbstractUpdateImports implements Runnable {
             + "<style%s>${$css_%1$d}</style>" + CSS_POST;
     private static final String INJECT_CSS = CSS_IMPORT
             + "%ninjectGlobalCss($cssFromFile_%1$d.toString(), 'CSSImport end', document);%n";
+    private static final Pattern INJECT_CSS_PATTERN = Pattern
+            .compile("^\\s*injectGlobalCss\\(([^,]+),.*$");
+    private static final String INJECT_WC_CSS = "injectGlobalWebcomponentCss(%s);";
+
     private static final String THEMABLE_MIXIN_IMPORT = "import { css, unsafeCSS, registerStyles } from '@vaadin/vaadin-themable-mixin';";
     private static final String REGISTER_STYLES_FOR_TEMPLATE = CSS_IMPORT_AND_MAKE_LIT_CSS
             + "%n" + "registerStyles('%s', $css_%1$d%s);";
@@ -220,13 +227,27 @@ protected void writeOutput(Map<File, List<String>> outputFiles) {
     List<String> filterWebComponentImports(List<String> lines) {
         if (lines != null) {
             // Exclude Lumo global imports for exported web-component
-            return lines.stream()
-                    .filter(VAADIN_LUMO_GLOBAL_IMPORT.asPredicate().negate())
-                    .collect(Collectors.toList());
+            List<String> copy = new ArrayList<>(lines);
+            copy.add(0, IMPORT_WC_INJECT);
+            copy.removeIf(VAADIN_LUMO_GLOBAL_IMPORT.asPredicate());
+            // Add global CSS imports with a per-webcomponent registration
+            final ListIterator<String> li = copy.listIterator();
+            while (li.hasNext()) {
+                adaptCssInjectForWebComponent(li, li.next());
+            }
+            return copy;
         }
         return lines;
     }
 
+    private void adaptCssInjectForWebComponent(ListIterator<String> iterator,
+            String line) {
+        Matcher matcher = INJECT_CSS_PATTERN.matcher(line);
+        if (matcher.matches()) {
+            iterator.add(String.format(INJECT_WC_CSS, matcher.group(1)));
+        }
+    }
+
     private void writeWebComponentImports(List<String> lines) {
         if (lines != null) {
             try {
@@ -305,7 +326,8 @@ private Map<File, List<String>> process(Map<ChunkInfo, List<CssData>> css,
                     chunkLines.addAll(
                             getModuleLines(lazyJavascript.get(chunkInfo)));
                 }
-                if (lazyCss.containsKey(chunkInfo)) {
+                boolean hasLazyCss = lazyCss.containsKey(chunkInfo);
+                if (hasLazyCss) {
                     chunkLines.add(IMPORT_INJECT);
                     chunkLines.add(THEMABLE_MIXIN_IMPORT);
                     chunkLines.addAll(lazyCss.get(chunkInfo));
@@ -358,6 +380,13 @@ private Map<File, List<String>> process(Map<ChunkInfo, List<CssData>> css,
             mainLines.addAll(mainCssLines);
         }
         mainLines.addAll(getModuleLines(eagerJavascript));
+
+        // Move all imports to the top
+        List<String> copy = new ArrayList<>(mainLines);
+        copy.removeIf(line -> !line.startsWith("import "));
+        mainLines.removeIf(line -> line.startsWith("import "));
+        mainLines.addAll(0, copy);
+
         mainLines.addAll(chunkLoader);
         mainLines.add("window.Vaadin = window.Vaadin || {};");
         mainLines.add("window.Vaadin.Flow = window.Vaadin.Flow || {};");

diff --git a/flow-server/src/main/resources/META-INF/frontend/theme-util.js b/flow-server/src/main/resources/META-INF/frontend/theme-util.js
@@ -114,6 +114,31 @@ window.Vaadin = window.Vaadin || {};
 window.Vaadin.theme = window.Vaadin.theme || {};
 window.Vaadin.theme.injectedGlobalCss = [];
 
+const webcomponentGlobalCss = {
+  css: [],
+  importers: []
+};
+
+export const injectGlobalWebcomponentCss = (css) => {
+  webcomponentGlobalCss.css.push(css);
+  webcomponentGlobalCss.importers.forEach(registrar => {
+    registrar(css);
+  });
+};
+
+export const webcomponentGlobalCssInjector = (registrar) => {
+  const registeredCss = [];
+  const wrapper = (css) => {
+    const hash = getHash(css);
+    if (!registeredCss.includes(hash)) {
+      registeredCss.push(hash);
+      registrar(css);
+    }
+  };
+  webcomponentGlobalCss.importers.push(wrapper);
+  webcomponentGlobalCss.css.forEach(wrapper);
+};
+
 /**
  * Calculate a 32 bit FNV-1a hash
  * Found here: https://gist.github.com/vaiorabbit/5657561

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
@@ -51,6 +51,7 @@ function writeThemeFiles(themeFolder, themeName, themeProperties, options) {
   const styles = resolve(themeFolder, stylesCssFilename);
   const documentCssFile = resolve(themeFolder, documentCssFilename);
   const autoInjectComponents = themeProperties.autoInjectComponents ?? true;
+  const autoInjectGlobalCssImports = themeProperties.autoInjectGlobalCssImports ?? false;
   const globalFilename = 'theme-' + themeName + '.global.generated.js';
   const componentsFilename = 'theme-' + themeName + '.components.generated.js';
   const themeFilename = 'theme-' + themeName + '.generated.js';
@@ -77,6 +78,7 @@ function writeThemeFiles(themeFolder, themeName, themeProperties, options) {
   }
 
   themeFileContent += `import { injectGlobalCss } from 'Frontend/generated/jar-resources/theme-util.js';\n`;
+  themeFileContent += `import { webcomponentGlobalCssInjector } from 'Frontend/generated/jar-resources/theme-util.js';\n`;
   themeFileContent += `import './${componentsFilename}';\n`;
 
   themeFileContent += `let needsReloadOnChanges = false;\n`;
@@ -222,6 +224,11 @@ function writeThemeFiles(themeFolder, themeName, themeProperties, options) {
     const removers = [];
     if (target !== document) {
       ${shadowOnlyCss.join('')}
+      ${autoInjectGlobalCssImports ? `
+        webcomponentGlobalCssInjector((css) => {
+          removers.push(injectGlobalCss(css, '', target));
+        });
+        ` : ''}
     }
     ${parentTheme}
     ${globalCssCode.join('')}
@@ -232,7 +239,7 @@ function writeThemeFiles(themeFolder, themeName, themeProperties, options) {
     }
 
   }
-  
+
 `;
   componentsFileContent += `
 ${componentCssImports.join('')}

diff --git a/flow-server/src/test/java/com/vaadin/flow/server/frontend/AbstractUpdateImportsTest.java b/flow-server/src/test/java/com/vaadin/flow/server/frontend/AbstractUpdateImportsTest.java
@@ -56,7 +56,6 @@
 import com.vaadin.flow.component.dependency.JavaScript;
 import com.vaadin.flow.component.dependency.JsModule;
 import com.vaadin.flow.component.page.AppShellConfigurator;
-import com.vaadin.flow.di.Lookup;
 import com.vaadin.flow.router.Route;
 import com.vaadin.flow.server.Constants;
 import com.vaadin.flow.server.LoadDependenciesOnStartup;
@@ -441,6 +440,44 @@ public void generate_embeddedImports_doNotContainLumoGlobalThemeFiles()
 
     }
 
+    @Test
+    public void generate_embeddedImports_addAlsoGlobalStyles()
+            throws IOException {
+        Class<?>[] testClasses = { FooCssImport.class, FooCssImport2.class,
+                UI.class, AllEagerAppConf.class };
+        ClassFinder classFinder = getClassFinder(testClasses);
+        updater = new UpdateImports(getScanner(classFinder), options);
+        updater.run();
+
+        Pattern injectGlobalCssPattern = Pattern
+                .compile("^\\s*injectGlobalCss\\(([^,]+),.*");
+        Predicate<String> globalCssImporter = injectGlobalCssPattern
+                .asPredicate();
+
+        List<String> globalCss = updater.getOutput()
+                .get(updater.generatedFlowImports).stream()
+                .filter(globalCssImporter).map(line -> {
+                    Matcher matcher = injectGlobalCssPattern.matcher(line);
+                    matcher.find();
+                    return matcher.group(1);
+                }).collect(Collectors.toList());
+
+        assertTrue("Import for web-components should also inject global CSS",
+                updater.webComponentImports.stream()
+                        .anyMatch(globalCssImporter));
+
+        assertTrue(
+                "Should contain function to import global CSS into embedded component",
+                updater.webComponentImports.stream().anyMatch(line -> line
+                        .contains("import { injectGlobalWebcomponentCss }")));
+        globalCss.forEach(css -> assertTrue(
+                "Should register global CSS " + css + " for webcomponent",
+                updater.webComponentImports.stream()
+                        .anyMatch(line -> line.contains(
+                                "injectGlobalWebcomponentCss(" + css + ");"))));
+
+    }
+
     @Test
     public void jsModulesOrderIsPreservedAnsAfterJsModules() {
         updater.run();

diff --git a/...r/src/test/java/com/vaadin/flow/server/frontend/UpdateImportsWithByteCodeScannerTest.java b/...r/src/test/java/com/vaadin/flow/server/frontend/UpdateImportsWithByteCodeScannerTest.java
@@ -312,6 +312,7 @@ public void cssInLazyChunkWorks() throws Exception {
         assertOnce("import { injectGlobalCss } from", chunkLines);
         assertOnce("from 'Frontend/foo.css?inline';", chunkLines);
         assertOnce("import $cssFromFile_0 from", chunkLines);
+        assertOnce("injectGlobalCss($cssFromFile_0", chunkLines);
 
         // assert lines order is preserved
         Assert.assertEquals(

diff --git a/...est-embedding/test-embedding-application-theme/src/main/frontend/css-import-component.css b/...est-embedding/test-embedding-application-theme/src/main/frontend/css-import-component.css
@@ -0,0 +1,3 @@
+DIV.cssimport {
+    color: gold
+}
diff --git a/...dding/test-embedding-application-theme/src/main/frontend/themes/embedded-theme/theme.json b/...dding/test-embedding-application-theme/src/main/frontend/themes/embedded-theme/theme.json
@@ -1,4 +1,5 @@
 {
+  "autoInjectGlobalCssImports": true,
   "documentCss": ["@fortawesome/fontawesome-free/css/all.css"],
   "assets": {
     "@fortawesome/fontawesome-free": {

diff --git a/...ding-application-theme/src/main/java/com/vaadin/flow/webcomponent/CssImportComponent.java b/...ding-application-theme/src/main/java/com/vaadin/flow/webcomponent/CssImportComponent.java
@@ -0,0 +1,34 @@
+/*
+ * Copyright 2000-2024 Vaadin Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package com.vaadin.flow.webcomponent;
+
+import com.vaadin.flow.component.Tag;
+import com.vaadin.flow.component.dependency.CssImport;
+import com.vaadin.flow.component.html.Div;
+
+@Tag("css-import-component")
+@CssImport("./css-import-component.css")
+public class CssImportComponent extends Div {
+
+    public CssImportComponent(String id) {
+        setId(id);
+        Div div = new Div(
+                "Global CssImport styles should be applied inside embedded web component, this should not be black");
+        div.setClassName("cssimport");
+        add(div);
+    }
+}
diff --git a/...bedding-application-theme/src/main/java/com/vaadin/flow/webcomponent/ThemedComponent.java b/...bedding-application-theme/src/main/java/com/vaadin/flow/webcomponent/ThemedComponent.java
@@ -18,7 +18,6 @@
 import com.vaadin.flow.component.dependency.NpmPackage;
 import com.vaadin.flow.component.html.Div;
 import com.vaadin.flow.component.html.Span;
-
 import com.vaadin.flow.uitest.ui.dependencies.TestVersion;
 
 @NpmPackage(value = "@fortawesome/fontawesome-free", version = TestVersion.FONTAWESOME)
@@ -27,6 +26,7 @@ public class ThemedComponent extends Div {
     public static final String TEST_TEXT_ID = "test-text";
 
     public static final String MY_COMPONENT_ID = "field";
+    public static final String CSS_IMPORT_COMPONENT_ID = "embedded-cssimport";
     public static final String EMBEDDED_ID = "embedded";
 
     public static final String HAND_ID = "sparkle-hand";
@@ -45,5 +45,6 @@ public ThemedComponent() {
 
         add(new Div());
         add(new MyComponent().withId(MY_COMPONENT_ID));
+        add(new CssImportComponent(CSS_IMPORT_COMPONENT_ID));
     }
 }
diff --git a/flow-tests/test-embedding/test-embedding-application-theme/src/main/webapp/index.html b/flow-tests/test-embedding/test-embedding-application-theme/src/main/webapp/index.html
@@ -1,17 +1,20 @@
 <!doctype html>
-
-<head>
-  <script type="module" src="web-component/themed-component.js"></script>
-</head>
-
-<body>
-
-  <h1>Lumo styles should not be applied</h1>
-  <div class="internal" id="internal">Internal should not apply, this should be black</div>
-  <div class="global" id="global">Document styles should apply, this should be blue</div>
-
-
-  <themed-component id="first"></themed-component>
-  <themed-component id="second"></themed-component>
-
-</body>
+<html>
+    <head>
+        <script type="module" src="web-component/themed-component.js"></script>
+    </head>
+    <body>
+        <h1>Lumo styles should not be applied</h1>
+        <div class="internal" id="internal">
+            Internal should not apply, this should be black
+        </div>
+        <div class="cssimport" id="cssimport">
+            CssImport styles should apply, this should not be black
+        </div>
+        <div class="global" id="global">
+            Document styles should apply, this should be blue
+        </div>
+        <themed-component id="first"></themed-component>
+        <themed-component id="second"></themed-component>
+    </body>
+</html>
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
@@ -18,6 +18,7 @@
 import java.util.List;
 import java.util.stream.Collectors;
 
+import com.vaadin.flow.component.html.Div;
 import com.vaadin.flow.component.html.testbench.DivElement;
 import com.vaadin.flow.component.html.testbench.H1Element;
 import com.vaadin.flow.component.html.testbench.SpanElement;
@@ -103,6 +104,14 @@ private void validateEmbeddedComponent(TestBenchElement themedComponent,
 
         Assert.assertEquals("Color should have been applied",
                 "rgba(0, 128, 0, 1)", handElement.getCssValue("color"));
+
+        // Ensure @CssImport styles are applied
+        final WebElement cssImportElement = embeddedComponent
+                .$("css-import-component").first().$(DivElement.class).single();
+        Assert.assertEquals(
+                "Color fom CSSImport annotation should have been applied",
+                "rgba(255, 215, 0, 1)", cssImportElement.getCssValue("color"));
+
     }
 
     @Test
@@ -223,8 +232,8 @@ public void multipleSameEmbedded_cssTargetingDocumentShouldOnlyAddElementsOneTim
                 2l, getCommandExecutor().executeScript(
                         "return document.head.querySelectorAll('link[rel=stylesheet][href^=\"https://fonts.googleapis.com\"]').length"));
         Assert.assertEquals(
-                "Project contains 2 css injections to document and both should be hashed",
-                2l, getCommandExecutor().executeScript(
+                "Project contains 3 css injections to document and all should be hashed",
+                3l, getCommandExecutor().executeScript(
                         "return window.Vaadin.theme.injectedGlobalCss.length"));
     }
 
@@ -246,4 +255,22 @@ public void lumoImports_doNotLeakEmbeddingPage() {
                 "rgba(0, 0, 0, 1)", element.getCssValue("color"));
 
     }
+
+    @Test
+    public void cssImportAnnotation_applyToEmbeddingPage() {
+        open();
+        checkLogsForErrors();
+
+        // Ensure embedded components are loaded before testing embedding page
+        validateEmbeddedComponent($("themed-component").id("first"), "first");
+        validateEmbeddedComponent($("themed-component").id("second"), "second");
+
+        final DivElement element = $(DivElement.class).withId("cssimport")
+                .waitForFirst();
+        Assert.assertEquals(
+                "CssImport styles (colors) should have been applied to elements in embedding page",
+                "rgba(255, 215, 0, 1)", element.getCssValue("color"));
+
+    }
+
 }