Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: prevent style leaking with exported webcomponents #19722

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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);";
Expand Down Expand Up @@ -220,13 +227,25 @@ 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());
// 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<String> lines) {
if (lines != null) {
try {
Expand Down Expand Up @@ -292,7 +311,7 @@ private Map<File, List<String>> process(Map<ChunkInfo, List<CssData>> css,
start = System.nanoTime();

chunkLoader.add("");
chunkLoader.add("const loadOnDemand = (key) => {");
chunkLoader.add("const loadOnDemand = (key, isWebcomponent) => {");
chunkLoader.add(" const pending = [];");
Set<ChunkInfo> mergedChunkKeys = merge(lazyJavascript.keySet(),
lazyCss.keySet());
Expand All @@ -305,7 +324,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));
Expand All @@ -324,16 +344,46 @@ private Map<File, List<String>> process(Map<ChunkInfo, List<CssData>> 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
.add(chunkContentHash);
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<String> 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);
}
}
}

Expand All @@ -357,11 +407,42 @@ private Map<File, List<String>> process(Map<ChunkInfo, List<CssData>> css,
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
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 || {};");
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);
Expand Down
32 changes: 32 additions & 0 deletions flow-server/src/main/resources/META-INF/frontend/theme-util.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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`;
Expand Down Expand Up @@ -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('')}
Expand All @@ -232,7 +236,7 @@ function writeThemeFiles(themeFolder, themeName, themeProperties, options) {
}

}

`;
componentsFileContent += `
${componentCssImports.join('')}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> expectedChunks = Stream
Expand All @@ -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<String> expectedChunks = Stream
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<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 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();
Expand Down Expand Up @@ -566,9 +604,12 @@ 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"))
.filter(row -> !row.contains(
"window.Vaadin.Flow.restoreCustomElementDefine"))
.toList();

Assert.assertEquals(List.of("import 'Frontend/jsm-all.js';",
Expand All @@ -588,9 +629,12 @@ 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"))
.filter(row -> !row.contains(
"window.Vaadin.Flow.restoreCustomElementDefine"))
.toList();
Assert.assertEquals(List.of("import 'Frontend/jsm-all.js';",
"import 'Frontend/jsm-all2.js';",
Expand Down Expand Up @@ -715,6 +759,8 @@ public void assertFullSortOrder(boolean uiImportSeparated,

List<String> 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"));
Expand Down
Loading
Loading