From 79b22e2adad3575723ec527fa55736800ac88511 Mon Sep 17 00:00:00 2001 From: Marco Collovati Date: Tue, 23 Jul 2024 11:47:11 +0200 Subject: [PATCH 1/3] fix: prevent style leaking with exported webcomponents --- .../flow/component/internal/UIInternals.java | 7 +- .../frontend/AbstractUpdateImports.java | 97 +++++++++++++++++-- .../resources/META-INF/frontend/theme-util.js | 32 ++++++ .../theme-generator.js | 6 +- .../frontend/AbstractUpdateImportsTest.java | 44 ++++++++- .../UpdateImportsWithByteCodeScannerTest.java | 37 +++++-- .../main/frontend/css-import-component.css | 3 + .../flow/webcomponent/CssImportComponent.java | 34 +++++++ .../flow/webcomponent/ThemedComponent.java | 3 +- .../src/main/webapp/index.html | 35 ++++--- .../ApplicationThemeComponentIT.java | 30 ++++++ 11 files changed, 293 insertions(+), 35 deletions(-) create mode 100644 flow-tests/test-embedding/test-embedding-application-theme/src/main/frontend/css-import-component.css create mode 100644 flow-tests/test-embedding/test-embedding-application-theme/src/main/java/com/vaadin/flow/webcomponent/CssImportComponent.java diff --git a/flow-server/src/main/java/com/vaadin/flow/component/internal/UIInternals.java b/flow-server/src/main/java/com/vaadin/flow/component/internal/UIInternals.java index 0aea9ae6438..c26f7752b00 100644 --- a/flow-server/src/main/java/com/vaadin/flow/component/internal/UIInternals.java +++ b/flow-server/src/main/java/com/vaadin/flow/component/internal/UIInternals.java @@ -47,6 +47,7 @@ import com.vaadin.flow.component.internal.ComponentMetaData.DependencyInfo; import com.vaadin.flow.component.page.ExtendedClientDetails; import com.vaadin.flow.component.page.Page; +import com.vaadin.flow.component.webcomponent.WebComponentUI; import com.vaadin.flow.dom.ElementUtil; import com.vaadin.flow.dom.impl.BasicElementStateProvider; import com.vaadin.flow.function.SerializableConsumer; @@ -963,8 +964,10 @@ private void triggerChunkLoading( clazz = clazz.getSuperclass(); } } - chunkIds.forEach(chunkId -> ui.getPage().addDynamicImport( - "return window.Vaadin.Flow.loadOnDemand('" + chunkId + "');")); + boolean isWebcomponent = ui instanceof WebComponentUI; + chunkIds.forEach(chunkId -> ui.getPage() + .addDynamicImport("return window.Vaadin.Flow.loadOnDemand('" + + chunkId + "', " + isWebcomponent + ");")); } private void warnForUnavailableBundledDependencies( 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 index 6d3e0f1d162..3fa82a71024 100644 --- 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 @@ -39,6 +39,7 @@ 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 +77,8 @@ 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_COMPOSE_LOAD_ON_DEMAND = "import { composeLoadOnDemand } 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 { + "${$css_%1$d}" + 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,25 @@ protected void writeOutput(Map> outputFiles) { List filterWebComponentImports(List 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 copy = new ArrayList<>(lines); + copy.add(0, IMPORT_WC_INJECT); + copy.removeIf(VAADIN_LUMO_GLOBAL_IMPORT.asPredicate()); + // Replace global CSS imports with a per-webcomponent registration + // to prevent leaking styles into the embedding document + copy.replaceAll(this::adaptCssInjectForWebComponent); + return copy; } return lines; } + private String adaptCssInjectForWebComponent(String line) { + Matcher matcher = INJECT_CSS_PATTERN.matcher(line); + if (matcher.matches()) { + return String.format(INJECT_WC_CSS, matcher.group(1)); + } + return line; + } + private void writeWebComponentImports(List lines) { if (lines != null) { try { @@ -292,7 +311,7 @@ private Map> process(Map> css, start = System.nanoTime(); chunkLoader.add(""); - chunkLoader.add("const loadOnDemand = (key) => {"); + chunkLoader.add("const loadOnDemand = (key, isWebcomponent) => {"); chunkLoader.add(" const pending = [];"); Set mergedChunkKeys = merge(lazyJavascript.keySet(), lazyCss.keySet()); @@ -305,7 +324,8 @@ private Map> process(Map> 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)); @@ -324,9 +344,23 @@ private Map> process(Map> css, .map(hash -> String.format("key === '%s'", hash)) .collect(Collectors.joining(" || ")); chunkLoader.add(String.format(" if (%s) {", ifClauses)); - chunkLoader.add(String.format( + String importChunkExpression = String.format( " pending.push(import('./chunks/%s'));", - chunkFilename)); + chunkFilename); + // For lazy css, we need a separated chunk for webcomponents + // that prevents loading styles into the embedding document + if (hasLazyCss) { + // Using a runtime expression for the import directive will + // break Vite build when it parsers imports. + chunkLoader.add(" if (isWebcomponent) {"); + chunkLoader.add(" " + importChunkExpression + .replace("/chunks/", "/chunks/wc-")); + chunkLoader.add(" } else {"); + chunkLoader.add(" " + importChunkExpression); + chunkLoader.add(" }"); + } else { + chunkLoader.add(importChunkExpression); + } chunkLoader.add(" }"); boolean chunkNotExist = processedChunkHashes @@ -334,6 +368,22 @@ private Map> process(Map> css, if (chunkNotExist) { File chunkFile = new File(chunkFolder, chunkFilename); files.put(chunkFile, chunkLines); + + // create a separate chunk to be imported by exported + // webcomponents, to prevent leaking styles into embedding + // document + if (hasLazyCss) { + List wcChunkLines = new ArrayList<>(chunkLines); + wcChunkLines.replaceAll(line -> { + if (IMPORT_INJECT.equals(line)) { + return IMPORT_WC_INJECT; + } + return adaptCssInjectForWebComponent(line); + }); + chunkFile = new File(chunkFolder, + "wc-" + chunkFilename); + files.put(chunkFile, wcChunkLines); + } } } @@ -354,14 +404,45 @@ private Map> process(Map> css, List mainCssLines = getCssLines(eagerCssData); if (!mainCssLines.isEmpty()) { mainLines.add(IMPORT_INJECT); + mainLines.add(IMPORT_COMPOSE_LOAD_ON_DEMAND); mainLines.add(THEMABLE_MIXIN_IMPORT); mainLines.addAll(mainCssLines); } mainLines.addAll(getModuleLines(eagerJavascript)); + + // Move all imports to the top + List 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 || {};"); - mainLines.add("window.Vaadin.Flow.loadOnDemand = loadOnDemand;"); + // Ugly hack to prevent exported webcomponent to fail beacuse of + // dom-module being registered multiple time when embedding many Vaadin + // applications: wraps the window.customElements.define to add a guard + // before calling the original. + mainLines + .add(""" + if (!window.Vaadin.Flow.restoreCustomElementDefine) { + const customElementDefine = window.customElements.define; + window.Vaadin.Flow.restoreCustomElementDefine = () => { + window.customElements.define = customElementDefine; + }; + window.customElements.define = (name, constructor, options) => { + if (window.customElements.get(name)) { + console.debug(name + " custom element is already defined"); + } else { + customElementDefine.apply(window.customElements, [name, constructor, options]); + } + } + }; + """); + // When embedding multiple Vaadin applications, make `loadOnDemand` is + // not overwritten, preventing chunks lazy loading + mainLines.add( + "window.Vaadin.Flow.loadOnDemand = composeLoadOnDemand(loadOnDemand,window.Vaadin.Flow.loadOnDemand);"); mainLines.add("window.Vaadin.Flow.resetFocus = " + RESET_FOCUS_JS); files.put(generatedFlowImports, mainLines); 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 index 5e4ed19588a..2e3b32b8918 100644 --- 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,38 @@ 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); +}; + +export const composeLoadOnDemand = (f1, f2) => { + if (f1 !== undefined && f2 !== undefined) { + return (key, isWebcomponent) => Promise.all([f1(key, isWebcomponent), f2(key, isWebcomponent)]); + } + return f1 || f2; +} + /** * 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 index 1c8ee5fe15f..d1377ccd17e 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 @@ -77,6 +77,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 +223,9 @@ function writeThemeFiles(themeFolder, themeName, themeProperties, options) { const removers = []; if (target !== document) { ${shadowOnlyCss.join('')} + webcomponentGlobalCssInjector((css) => { + removers.push(injectGlobalCss(css, '', target)); + }); } ${parentTheme} ${globalCssCode.join('')} @@ -232,7 +236,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 index ff7a87cfb2b..d37872be0bf 100644 --- 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; @@ -435,12 +434,51 @@ public void generate_embeddedImports_doNotContainLumoGlobalThemeFiles() // Check that imports other than lumo globals are the same flowImports.removeAll(updater.webComponentImports); + flowImports.removeIf(line -> line.contains("injectGlobalCss")); assertTrue( "Flow and web-component imports must be the same, except for lumo globals", flowImports.stream().allMatch(lumoGlobalsMatcher)); } + @Test + public void generate_embeddedImports_doNotAddGlobalStyles() + 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 globalCssImporter = injectGlobalCssPattern + .asPredicate(); + + List 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 not inject global CSS", + updater.webComponentImports.stream() + .noneMatch(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(); @@ -569,6 +607,8 @@ public void developmentDependencies_includedInDevelopmentMode() .filter(row -> !row.contains("const loadOnDemand")) .filter(row -> !row.contains( "@vaadin/common-frontend/ConnectionIndicator")) + .filter(row -> !row.contains( + "window.Vaadin.Flow.restoreCustomElementDefine")) .toList(); Assert.assertEquals(List.of("import 'Frontend/jsm-all.js';", @@ -591,6 +631,8 @@ public void developmentDependencies_notIncludedInProductionMode() .filter(row -> !row.contains("const loadOnDemand")) .filter(row -> !row.contains( "@vaadin/common-frontend/ConnectionIndicator")) + .filter(row -> !row.contains( + "window.Vaadin.Flow.restoreCustomElementDefine")) .toList(); Assert.assertEquals(List.of("import 'Frontend/jsm-all.js';", "import 'Frontend/jsm-all2.js';", diff --git a/flow-server/src/test/java/com/vaadin/flow/server/frontend/UpdateImportsWithByteCodeScannerTest.java b/flow-server/src/test/java/com/vaadin/flow/server/frontend/UpdateImportsWithByteCodeScannerTest.java index c2c2d1ff1d8..31c842d3bc6 100644 --- a/flow-server/src/test/java/com/vaadin/flow/server/frontend/UpdateImportsWithByteCodeScannerTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/server/frontend/UpdateImportsWithByteCodeScannerTest.java @@ -149,7 +149,7 @@ public void lazyRouteIsLazyLoaded() throws Exception { Assert.assertTrue(output.containsKey(flowGeneratedImports)); Assert.assertTrue(output.containsKey(flowGeneratedImportsDTs)); - Optional chunkFile = findOptionalChunkFile(output); + Optional chunkFile = findOptionalChunkFile(output, false); Assert.assertTrue(chunkFile.isPresent()); Assert.assertTrue(output.containsKey(chunkFile.get())); @@ -187,10 +187,12 @@ public void lazyAndEagerRoutesProperlyHandled() throws Exception { Assert.assertTrue(output.containsKey(flowGeneratedImports)); Assert.assertTrue(output.containsKey(flowGeneratedImportsDTs)); - Optional chunkFile = findOptionalChunkFile(output); + Optional chunkFile = findOptionalChunkFile(output, false); Assert.assertTrue(chunkFile.isPresent()); Assert.assertTrue(output.containsKey(chunkFile.get())); + Assert.assertTrue(findOptionalChunkFile(output, true).isEmpty()); + String mainImportContent = String.join("\n", output.get(flowGeneratedImports)); String lazyChunkContent = String.join("\n", @@ -303,15 +305,18 @@ public void cssInLazyChunkWorks() throws Exception { Map> output = updater.getOutput(); Assert.assertNotNull(output); - Assert.assertEquals(3, output.size()); + // output should contain generated imports and chunks for both Flow app + // and exported webcomponets + Assert.assertEquals(4, output.size()); - Optional chunkFile = findOptionalChunkFile(output); + Optional chunkFile = findOptionalChunkFile(output, false); Assert.assertTrue(chunkFile.isPresent()); List chunkLines = output.get(chunkFile.get()); 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( @@ -320,12 +325,32 @@ public void cssInLazyChunkWorks() throws Exception { Assert.assertEquals( "import { css, unsafeCSS, registerStyles } from '@vaadin/vaadin-themable-mixin';", chunkLines.get(1)); + + // Webcomponents chunk + chunkFile = findOptionalChunkFile(output, true); + Assert.assertTrue(chunkFile.isPresent()); + + chunkLines = output.get(chunkFile.get()); + assertOnce("import { injectGlobalWebcomponentCss } from", chunkLines); + assertOnce("from 'Frontend/foo.css?inline';", chunkLines); + assertOnce("import $cssFromFile_0 from", chunkLines); + assertOnce("injectGlobalWebcomponentCss($cssFromFile_0", chunkLines); + + // assert lines order is preserved + Assert.assertEquals( + "import { injectGlobalWebcomponentCss } from 'Frontend/generated/jar-resources/theme-util.js';\n", + chunkLines.get(0)); + Assert.assertEquals( + "import { css, unsafeCSS, registerStyles } from '@vaadin/vaadin-themable-mixin';", + chunkLines.get(1)); + } private static Optional findOptionalChunkFile( - Map> output) { + Map> output, boolean forWebcomponent) { + String prefix = ((forWebcomponent) ? "wc-" : "") + "chunk-"; return output.keySet().stream() - .filter(file -> file.getName().contains("chunk-")).findAny(); + .filter(file -> file.getName().startsWith(prefix)).findAny(); } private void assertImports(String mainImportContent, diff --git a/flow-tests/test-embedding/test-embedding-application-theme/src/main/frontend/css-import-component.css b/flow-tests/test-embedding/test-embedding-application-theme/src/main/frontend/css-import-component.css new file mode 100644 index 00000000000..60394390016 --- /dev/null +++ b/flow-tests/test-embedding/test-embedding-application-theme/src/main/frontend/css-import-component.css @@ -0,0 +1,3 @@ +DIV.cssimport { + color: gold +} diff --git a/flow-tests/test-embedding/test-embedding-application-theme/src/main/java/com/vaadin/flow/webcomponent/CssImportComponent.java b/flow-tests/test-embedding/test-embedding-application-theme/src/main/java/com/vaadin/flow/webcomponent/CssImportComponent.java new file mode 100644 index 00000000000..f8465170569 --- /dev/null +++ b/flow-tests/test-embedding/test-embedding-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( + "CssImport styles should be apply, this should not be black"); + div.setClassName("cssimport"); + add(div); + } +} diff --git a/flow-tests/test-embedding/test-embedding-application-theme/src/main/java/com/vaadin/flow/webcomponent/ThemedComponent.java b/flow-tests/test-embedding/test-embedding-application-theme/src/main/java/com/vaadin/flow/webcomponent/ThemedComponent.java index 7752d6ea305..47794c5f2c6 100644 --- a/flow-tests/test-embedding/test-embedding-application-theme/src/main/java/com/vaadin/flow/webcomponent/ThemedComponent.java +++ b/flow-tests/test-embedding/test-embedding-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 index a9dbc13515c..33e84c251fa 100644 --- 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 @@ - - - - - - - -

Lumo styles should not be applied

-
Internal should not apply, this should be black
-
Document styles should apply, this should be blue
- - - - - - + + + + + +

Lumo styles should not be applied

+
+ Internal should not apply, this should be black +
+
+ CssImport styles should not apply, this should be black +
+
+ Document styles should apply, this should be blue +
+ + + + 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 35a4de0793d..4ef5f8746e8 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 @@ -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 @@ -246,4 +255,25 @@ public void lumoImports_doNotLeakEmbeddingPage() { "rgba(0, 0, 0, 1)", element.getCssValue("color")); } + + @Test + public void cssImportAnnotation_doNotLeakEmbeddingPage() { + 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.assertFalse( + "Lumo styles (typography) should not have been applied to elements in embedding page", + element.getCssValue("font-family").contains("Roboto")); + Assert.assertEquals( + "Lumo styles (colors) should not have been applied to elements in embedding page", + "rgba(0, 0, 0, 1)", element.getCssValue("color")); + + } + } From b62f4674db0e26c533494e1eca20d898263473d7 Mon Sep 17 00:00:00 2001 From: Marco Collovati Date: Fri, 26 Jul 2024 07:36:04 +0200 Subject: [PATCH 2/3] fix tests --- .../com/vaadin/flow/server/communication/UidlWriterTest.java | 4 ++-- .../server/frontend/UpdateImportsWithByteCodeScannerTest.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/flow-server/src/test/java/com/vaadin/flow/server/communication/UidlWriterTest.java b/flow-server/src/test/java/com/vaadin/flow/server/communication/UidlWriterTest.java index 4699c11d8c0..6b734c63ac2 100644 --- a/flow-server/src/test/java/com/vaadin/flow/server/communication/UidlWriterTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/server/communication/UidlWriterTest.java @@ -228,7 +228,7 @@ public void componentDependencies_productionMode_scanForParentClasses() .startsWith("return window.Vaadin.Flow.loadOnDemand('")) .map(key -> key .replace("return window.Vaadin.Flow.loadOnDemand('", "") - .replace("');", "")) + .replace("', false);", "")) .collect(Collectors.toSet()); Set expectedChunks = Stream @@ -254,7 +254,7 @@ public void componentDependencies_developmentMode_onlySendComponentSpecificChunk .startsWith("return window.Vaadin.Flow.loadOnDemand('")) .map(key -> key .replace("return window.Vaadin.Flow.loadOnDemand('", "") - .replace("');", "")) + .replace("', false);", "")) .collect(Collectors.toSet()); Set expectedChunks = Stream diff --git a/flow-server/src/test/java/com/vaadin/flow/server/frontend/UpdateImportsWithByteCodeScannerTest.java b/flow-server/src/test/java/com/vaadin/flow/server/frontend/UpdateImportsWithByteCodeScannerTest.java index 31c842d3bc6..34441114010 100644 --- a/flow-server/src/test/java/com/vaadin/flow/server/frontend/UpdateImportsWithByteCodeScannerTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/server/frontend/UpdateImportsWithByteCodeScannerTest.java @@ -191,7 +191,7 @@ public void lazyAndEagerRoutesProperlyHandled() throws Exception { Assert.assertTrue(chunkFile.isPresent()); Assert.assertTrue(output.containsKey(chunkFile.get())); - Assert.assertTrue(findOptionalChunkFile(output, true).isEmpty()); + Assert.assertTrue(findOptionalChunkFile(output, true).isPresent()); String mainImportContent = String.join("\n", output.get(flowGeneratedImports)); From 7cc9901e91680a5ad75d46eb01b6d5a6c862e496 Mon Sep 17 00:00:00 2001 From: Marco Collovati Date: Mon, 29 Jul 2024 07:32:05 +0200 Subject: [PATCH 3/3] fix import --- .../vaadin/flow/server/frontend/AbstractUpdateImports.java | 2 +- .../flow/server/frontend/AbstractUpdateImportsTest.java | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) 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 index 3fa82a71024..e386cbf86ac 100644 --- 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 @@ -404,10 +404,10 @@ private Map> process(Map> css, List mainCssLines = getCssLines(eagerCssData); if (!mainCssLines.isEmpty()) { mainLines.add(IMPORT_INJECT); - mainLines.add(IMPORT_COMPOSE_LOAD_ON_DEMAND); mainLines.add(THEMABLE_MIXIN_IMPORT); mainLines.addAll(mainCssLines); } + mainLines.add(0, IMPORT_COMPOSE_LOAD_ON_DEMAND); mainLines.addAll(getModuleLines(eagerJavascript)); // Move all imports to the top 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 index d37872be0bf..841c6962f41 100644 --- 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 @@ -604,6 +604,7 @@ public void developmentDependencies_includedInDevelopmentMode() .filter(row -> !row.startsWith("export ")) .filter(row -> !row.startsWith("window.Vaadin")) .filter(row -> !row.contains("Frontend/generated/flow")) + .filter(row -> !row.contains("composeLoadOnDemand")) .filter(row -> !row.contains("const loadOnDemand")) .filter(row -> !row.contains( "@vaadin/common-frontend/ConnectionIndicator")) @@ -628,6 +629,7 @@ public void developmentDependencies_notIncludedInProductionMode() .filter(row -> !row.startsWith("export ")) .filter(row -> !row.startsWith("window.Vaadin")) .filter(row -> !row.contains("Frontend/generated/flow")) + .filter(row -> !row.contains("composeLoadOnDemand")) .filter(row -> !row.contains("const loadOnDemand")) .filter(row -> !row.contains( "@vaadin/common-frontend/ConnectionIndicator")) @@ -757,6 +759,8 @@ public void assertFullSortOrder(boolean uiImportSeparated, List result = updater.getMergedOutput(); result.removeIf(line -> line.startsWith("import { injectGlobalCss }")); + result.removeIf( + line -> line.startsWith("import { composeLoadOnDemand }")); result.removeIf(line -> line.startsWith("export ")); result.removeIf(line -> line.isBlank()); result.removeIf(line -> line.contains("loadOnDemand"));