Skip to content

Commit bcc5452

Browse files
platoshaplatosha
authored
fix: move CSS imports from @theme to bootstrap JS file (#22518)
* fix: move CSS imports from @theme to bootstrap JS file Fixes #22517 * chore: formatting * test: unit test for app shell & theme css imports, fix existing cases * fix: align byte-code and full scanner behavior for Theme CSS imports * test: update assertion for AppShell dependencies test * test: update assertion in ApplicationThemeComponent test * test: restore embedded theme CSS include test assertion * chore: remove loadCss flag * chore: fix typo and formatting * fix: take care of null pointer * Revert "chore: remove loadCss flag" This reverts commit a3456d8. * fix: rollback ignoring theme css in bundle analysis * fix: keep theme CSS import in generated web component imports * fix: handle null map key * fix: bug in generated web component imports implementation * chore: formatting * fix: avoid duplicate lines in wc imports file * fix: write app-shell-imports.js info to stats.json for prod bundle
1 parent 7072678 commit bcc5452

File tree

11 files changed

+166
-39
lines changed

11 files changed

+166
-39
lines changed

‎flow-server/src/main/java/com/vaadin/flow/server/frontend/AbstractUpdateImports.java‎

Lines changed: 48 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,8 @@ abstract class AbstractUpdateImports implements Runnable {
118118
final File generatedFlowImports;
119119
final File generatedFlowWebComponentImports;
120120
private final File generatedFlowDefinitions;
121+
final File appShellImports;
122+
final File appShellDefinitions;
121123
private File chunkFolder;
122124

123125
private final GeneratedFilesSupport generatedFilesSupport;
@@ -141,6 +143,12 @@ abstract class AbstractUpdateImports implements Runnable {
141143
generatedFlowDefinitions = new File(
142144
generatedFlowImports.getParentFile(),
143145
FrontendUtils.IMPORTS_D_TS_NAME);
146+
var generatedFolder = FrontendUtils
147+
.getFrontendGeneratedFolder(options.getFrontendDirectory());
148+
appShellImports = new File(generatedFolder,
149+
FrontendUtils.APP_SHELL_IMPORTS_NAME);
150+
appShellDefinitions = new File(generatedFolder,
151+
FrontendUtils.APP_SHELL_IMPORTS_D_TS_NAME);
144152

145153
generatedFlowWebComponentImports = FrontendUtils
146154
.getFlowGeneratedWebComponentsImports(
@@ -160,8 +168,8 @@ public void run() {
160168

161169
Map<File, List<String>> output = process(css, javascript);
162170
writeOutput(output);
163-
writeWebComponentImports(
164-
filterWebComponentImports(output.get(generatedFlowImports)));
171+
writeWebComponentImports(filterWebComponentImports(
172+
mergeWebComponentOutputLines(output)));
165173

166174
getLogger().debug("Imports and chunks update took {} ms.",
167175
TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start));
@@ -250,6 +258,17 @@ private void adaptCssInjectForWebComponent(ListIterator<String> iterator,
250258
}
251259
}
252260

261+
private List<String> mergeWebComponentOutputLines(
262+
Map<File, List<String>> outputFiles) {
263+
return Stream.concat(
264+
outputFiles
265+
.getOrDefault(appShellImports, Collections.emptyList())
266+
.stream(),
267+
outputFiles.getOrDefault(generatedFlowImports,
268+
Collections.emptyList()).stream())
269+
.distinct().toList();
270+
}
271+
253272
private void writeWebComponentImports(List<String> lines) {
254273
if (lines != null) {
255274
try {
@@ -276,6 +295,7 @@ private void writeWebComponentImports(List<String> lines) {
276295
private Map<File, List<String>> process(Map<ChunkInfo, List<CssData>> css,
277296
Map<ChunkInfo, List<String>> javascript) {
278297
getLogger().debug("Start sorting imports to lazy and eager.");
298+
int cssLineOffset = 0;
279299
long start = System.nanoTime();
280300

281301
Map<File, List<String>> files = new HashMap<>();
@@ -284,6 +304,7 @@ private Map<File, List<String>> process(Map<ChunkInfo, List<CssData>> css,
284304
List<String> eagerJavascript = new ArrayList<>();
285305
Map<ChunkInfo, List<String>> lazyCss = new LinkedHashMap<>();
286306
List<CssData> eagerCssData = new ArrayList<>();
307+
List<CssData> appShellCssData = new ArrayList<>();
287308
for (Entry<ChunkInfo, List<String>> entry : javascript.entrySet()) {
288309
if (isLazyRoute(entry.getKey())) {
289310
lazyJavascript.put(entry.getKey(), entry.getValue());
@@ -296,12 +317,18 @@ private Map<File, List<String>> process(Map<ChunkInfo, List<CssData>> css,
296317
boolean hasThemeFor = entry.getValue().stream()
297318
.anyMatch(cssData -> cssData.getThemefor() != null);
298319
if (isLazyRoute(entry.getKey()) && !hasThemeFor) {
299-
List<String> cssLines = getCssLines(entry.getValue());
320+
List<String> cssLines = getCssLines(entry.getValue(),
321+
cssLineOffset);
322+
cssLineOffset += cssLines.size();
300323
if (!cssLines.isEmpty()) {
301324
lazyCss.put(entry.getKey(), cssLines);
302325
}
303326
} else {
304-
eagerCssData.addAll(entry.getValue());
327+
if (entry.getKey().equals(ChunkInfo.APP_SHELL)) {
328+
appShellCssData.addAll(entry.getValue());
329+
} else {
330+
eagerCssData.addAll(entry.getValue());
331+
}
305332
}
306333
}
307334

@@ -372,10 +399,22 @@ private Map<File, List<String>> process(Map<ChunkInfo, List<CssData>> css,
372399
"const loadOnDemand = (key) => { return Promise.resolve(0); }");
373400
}
374401

402+
List<String> appShellLines = new ArrayList<>();
403+
List<String> appShellCssLines = getCssLines(appShellCssData,
404+
cssLineOffset);
405+
cssLineOffset += appShellCssLines.size();
406+
if (!appShellCssLines.isEmpty()) {
407+
appShellLines.add(IMPORT_INJECT);
408+
appShellLines.addAll(appShellCssLines);
409+
}
410+
files.put(appShellImports, appShellLines);
411+
files.put(appShellDefinitions, Collections.singletonList("export {}"));
412+
375413
List<String> mainLines = new ArrayList<>();
376414

377415
// Convert eager CSS data to JS and deduplicate it
378-
List<String> mainCssLines = getCssLines(eagerCssData);
416+
List<String> mainCssLines = getCssLines(eagerCssData, cssLineOffset);
417+
cssLineOffset += mainCssLines.size();
379418
if (!mainCssLines.isEmpty()) {
380419
mainLines.add(IMPORT_INJECT);
381420
mainLines.add(THEMABLE_MIXIN_IMPORT);
@@ -470,12 +509,12 @@ String resolveGeneratedModule(String module) {
470509
* the CSS import data
471510
* @return the JS statements needed to import and apply the CSS data
472511
*/
473-
protected List<String> getCssLines(List<CssData> css) {
512+
private List<String> getCssLines(List<CssData> css, int startOffset) {
474513
List<String> lines = new ArrayList<>();
475514

476515
Set<String> cssNotFound = new HashSet<>();
477516
LinkedHashSet<CssData> allCss = new LinkedHashSet<>(css);
478-
int i = 0;
517+
int i = startOffset;
479518
for (CssData cssData : allCss) {
480519
if (!addCssLines(lines, cssData, i)) {
481520
cssNotFound.add(cssData.getValue());
@@ -561,9 +600,9 @@ private Map<ChunkInfo, List<String>> mergeJavascript(
561600

562601
}
563602

564-
protected <T> List<String> merge(Map<T, List<String>> css) {
603+
protected <T> List<String> merge(Map<T, List<String>> outputFiles) {
565604
List<String> result = new ArrayList<>();
566-
css.forEach((key, value) -> result.addAll(value));
605+
outputFiles.forEach((key, value) -> result.addAll(value));
567606
return result;
568607
}
569608

‎flow-server/src/main/java/com/vaadin/flow/server/frontend/FrontendUtils.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,18 @@ public class FrontendUtils {
189189
public static final String THEME_IMPORTS_D_TS_NAME = "theme.d.ts";
190190
public static final String THEME_IMPORTS_NAME = "theme.js";
191191

192+
/**
193+
* The name of the file that contains application shell imports, such as
194+
* style imports for the theme.
195+
*/
196+
public static final String APP_SHELL_IMPORTS_NAME = "app-shell-imports.js";
197+
198+
/**
199+
* The TypeScript definitions for the
200+
* {@link FrontendUtils#APP_SHELL_IMPORTS_NAME}
201+
*/
202+
public static final String APP_SHELL_IMPORTS_D_TS_NAME = "app-shell-imports.d.ts";
203+
192204
/**
193205
* File name of the bootstrap file that is generated in frontend
194206
* {@link #GENERATED} folder. The bootstrap file is always executed in a

‎flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskGenerateBootstrap.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ private String getIndexTsEntryPath() {
116116

117117
private Collection<String> getThemeLines() {
118118
Collection<String> lines = new ArrayList<>();
119+
lines.add("import './app-shell-imports.js';");
119120
ThemeDefinition themeDef = frontDeps.getThemeDefinition();
120121
if (themeDef != null && !"".equals(themeDef.getName())) {
121122
lines.add("import './theme-" + themeDef.getName()

‎flow-server/src/main/java/com/vaadin/flow/server/frontend/scanner/ChunkInfo.java‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,20 @@
2626
* loaded immediately when the JS bundle is loaded while chunks marked as not
2727
* eager (i.e. lazy) are loaded on demand later.
2828
* <p>
29-
* There is one special, global chunk, defined as {@link #GLOBAL} in this class,
29+
* There is a special application shell chunk, defined as {@link #APP_SHELL} in
30+
* this class, which is used for gathering all data that relates to the
31+
* application shell.
32+
* <p>
33+
* There is a special global chunk, defined as {@link #GLOBAL} in this class,
3034
* which is used for gathering all data that relates to internal entry points.
3135
* <p>
3236
* For internal use only. May be renamed or removed in a future release.
3337
**/
3438
public class ChunkInfo {
3539

40+
public static final ChunkInfo APP_SHELL = new ChunkInfo(
41+
EntryPointType.INTERNAL, null, null, true);
42+
3643
public static final ChunkInfo GLOBAL = new ChunkInfo(
3744
EntryPointType.INTERNAL, null, null, false);
3845

‎flow-server/src/main/java/com/vaadin/flow/server/frontend/scanner/EntryPointType.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,5 +24,5 @@
2424
* For internal use only. May be renamed or removed in a future release.
2525
*/
2626
public enum EntryPointType {
27-
ROUTE, WEB_COMPONENT, INTERNAL;
27+
ROUTE, WEB_COMPONENT, INTERNAL, APP_SHELL;
2828
}

‎flow-server/src/main/java/com/vaadin/flow/server/frontend/scanner/FrontendDependencies.java‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -125,11 +125,8 @@ public FrontendDependencies(ClassFinder finder,
125125
if (themeDefinition != null && themeDefinition.getTheme() != null) {
126126
Class<? extends AbstractTheme> themeClass = themeDefinition
127127
.getTheme();
128-
if (!visitedClasses.containsKey(themeClass.getName())) {
129-
addInternalEntryPoint(themeClass);
130-
visitEntryPoint(entryPoints.get(themeClass.getName()));
131-
visitedClasses.get(themeClass.getName()).loadCss = true;
132-
}
128+
addAppShellEntryPoint(themeClass);
129+
visitEntryPoint(entryPoints.get(themeClass.getName()));
133130
}
134131
if (reactEnabled) {
135132
computeReactClasses(finder);
@@ -186,14 +183,17 @@ private void computeReactClasses(ClassFinder finder) throws IOException {
186183
}
187184

188185
private void aggregateEntryPointInformation() {
186+
var activeTheme = themeDefinition != null
187+
&& themeDefinition.getTheme() != null;
189188
for (Entry<String, EntryPointData> entry : entryPoints.entrySet()) {
190189
EntryPointData entryPoint = entry.getValue();
191190
for (String className : entryPoint.reachableClasses) {
192191
ClassInfo classInfo = visitedClasses.get(className);
193192
entryPoint.getModules().addAll(classInfo.modules);
194193
entryPoint.getModulesDevelopmentOnly()
195194
.addAll(classInfo.modulesDevelopmentOnly);
196-
if (classInfo.loadCss) {
195+
if (classInfo.loadCss || activeTheme
196+
&& entryPoint.getType() == EntryPointType.APP_SHELL) {
197197
entryPoint.getCss().addAll(classInfo.css);
198198
}
199199
entryPoint.getScripts().addAll(classInfo.scripts);
@@ -322,6 +322,9 @@ public Map<ChunkInfo, List<String>> getModulesDevelopment() {
322322
}
323323

324324
private ChunkInfo getChunkInfo(EntryPointData data) {
325+
if (data.getType() == EntryPointType.APP_SHELL) {
326+
return ChunkInfo.APP_SHELL;
327+
}
325328
if (data.getType() == EntryPointType.INTERNAL) {
326329
return ChunkInfo.GLOBAL;
327330
}
@@ -458,7 +461,7 @@ private void collectEntryPoints(boolean generateEmbeddableWebComponents)
458461

459462
for (Class<?> appShell : getFinder().getSubTypesOf(
460463
getFinder().loadClass(AppShellConfigurator.class.getName()))) {
461-
addInternalEntryPoint(appShell);
464+
addAppShellEntryPoint(appShell);
462465
}
463466

464467
try {
@@ -547,6 +550,10 @@ private List<String> getDependencyTriggers(Class<?> route,
547550
return null;
548551
}
549552

553+
private void addAppShellEntryPoint(Class<?> entryPointClass) {
554+
addEntryPoint(entryPointClass, EntryPointType.APP_SHELL, null, true);
555+
}
556+
550557
private void addInternalEntryPoint(Class<?> entryPointClass) {
551558
addEntryPoint(entryPointClass, EntryPointType.INTERNAL, null, true);
552559
}
@@ -631,9 +638,6 @@ private void computeApplicationTheme() throws ClassNotFoundException,
631638
themeDefinition = new ThemeDefinition(theme, variant,
632639
themeName);
633640
themeInstance = new ThemeWrapper(theme);
634-
classesWithTheme.get(themeData).children.stream()
635-
.map(visitedClasses::get).filter(Objects::nonNull)
636-
.forEach(classInfo -> classInfo.loadCss = true);
637641
}
638642
}
639643

‎flow-server/src/main/java/com/vaadin/flow/server/frontend/scanner/FullDependenciesScanner.java‎

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ class FullDependenciesScanner extends AbstractDependenciesScanner {
7373
private Map<String, String> devPackages;
7474
private HashMap<String, List<String>> assets = new HashMap<>();
7575
private HashMap<String, List<String>> devAssets = new HashMap<>();
76+
private List<CssData> themeCssData;
7677
private List<CssData> cssData;
7778
private List<String> scripts;
7879
private List<String> scriptsDevelopment;
@@ -150,7 +151,10 @@ class FullDependenciesScanner extends AbstractDependenciesScanner {
150151

151152
collectScripts(modulesSet, modulesSetDevelopment, JsModule.class);
152153
collectScripts(scriptsSet, scriptsSetDevelopment, JavaScript.class);
153-
cssData = discoverCss();
154+
155+
themeCssData = new ArrayList<>();
156+
cssData = new ArrayList<>();
157+
discoverCss();
154158

155159
if (!reactEnabled) {
156160
modulesSet.removeIf(
@@ -214,8 +218,9 @@ public Map<ChunkInfo, List<String>> getScriptsDevelopment() {
214218

215219
@Override
216220
public Map<ChunkInfo, List<CssData>> getCss() {
217-
return Collections.singletonMap(ChunkInfo.GLOBAL,
218-
new ArrayList<>(cssData));
221+
// Map theme CSS to the APP_SHELL chunk
222+
return Map.ofEntries(Map.entry(ChunkInfo.APP_SHELL, themeCssData),
223+
Map.entry(ChunkInfo.GLOBAL, cssData));
219224
}
220225

221226
@Override
@@ -307,27 +312,34 @@ private void discoverPackages(final Map<String, String> packages,
307312
}
308313
}
309314

310-
private List<CssData> discoverCss() {
315+
private void discoverCss() {
311316
try {
312317
Class<? extends Annotation> loadedAnnotation = getFinder()
313318
.loadClass(CssImport.class.getName());
314319
Set<Class<?>> annotatedClasses = getFinder()
315320
.getAnnotatedClasses(loadedAnnotation);
316-
LinkedHashSet<CssData> result = new LinkedHashSet<>();
321+
var themeCss = new LinkedHashSet<CssData>();
322+
var globalCss = new LinkedHashSet<CssData>();
317323
for (Class<?> clazz : annotatedClasses) {
318324
classes.add(clazz.getName());
319-
if (AbstractTheme.class.isAssignableFrom(clazz)
320-
&& (themeDefinition == null
321-
|| !clazz.equals(themeDefinition.getTheme()))) {
325+
var isAppShellClass = AppShellConfigurator.class
326+
.isAssignableFrom(clazz);
327+
var isThemeClass = AbstractTheme.class.isAssignableFrom(clazz);
328+
if (isThemeClass && (themeDefinition == null
329+
|| !clazz.equals(themeDefinition.getTheme()))) {
322330
// Do not add css from all found theme classes,
323331
// only defined theme.
324332
continue;
325333
}
326334
List<? extends Annotation> imports = annotationFinder
327335
.apply(clazz, loadedAnnotation);
328-
imports.stream().forEach(imp -> result.add(createCssData(imp)));
336+
imports.stream()
337+
.forEach(imp -> ((isAppShellClass || isThemeClass)
338+
? themeCss
339+
: globalCss).add(createCssData(imp)));
329340
}
330-
return new ArrayList<>(result);
341+
themeCssData.addAll(themeCss);
342+
cssData.addAll(globalCss);
331343
} catch (ClassNotFoundException exception) {
332344
throw new IllegalStateException(
333345
COULD_NOT_LOAD_ERROR_MSG + CssData.class.getName(),

‎flow-server/src/main/resources/vite.generated.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,10 @@ function statsExtracterPlugin(): PluginOption {
181181
path.resolve(themeOptions.frontendGeneratedFolder, 'flow', 'generated-flow-imports.js'),
182182
generatedImportsSet
183183
);
184+
parseImports(
185+
path.resolve(themeOptions.frontendGeneratedFolder, 'app-shell-imports.js'),
186+
generatedImportsSet
187+
);
184188
const generatedImports = Array.from(generatedImportsSet).sort();
185189

186190
const frontendFiles: Record<string, string> = {};

0 commit comments

Comments
 (0)