fix: Warn to delete component CSS with theme generated in one run (#1…

…0779)

Workaround for #9948. Warn devs to delete component CSS file along with theme generated file in one run to avoid webpack compilation errors and application restart.

Related-to: #9948

(cherry picked from commit e33ec10)

flow-server/src/main/resources/plugins/theme-live-reload-plugin/theme-live-reload-plugin.js

@@ -35,6 +35,12 @@ class ThemeLiveReloadPlugin {
           " callback as a ThemeLiveReloadPlugin constructor parameter.");
       }
       this.processThemeResourcesCallback = processThemeResourcesCallback;
+      // Component style sheet might be deleted from parent theme folder, so
+      // the regexp does not contain the exact theme name
+      this.componentStyleFileRegexp = /(\\|\/)themes\1([\s\S]*)\1components\1(.*)\.css$/;
+      // There might be several theme generated files in the generated
+      // folder, so the regexp does not contain the exact theme name
+      this.themeGeneratedFileRegexp = /[\s\S]*?\.generated\.js$/;
     }
 
     apply(compiler) {
@@ -43,20 +49,52 @@ class ThemeLiveReloadPlugin {
         const logger = compiler.getInfrastructureLogger("ThemeLiveReloadPlugin");
         const changedFilesMap = compiler.watchFileSystem.watcher.mtimes;
         if (changedFilesMap !== {}) {
+          let themeName = undefined;
           let themeGeneratedFileChanged = false;
+          let themeGeneratedFileDeleted = false;
+          let deletedComponentStyleFile = undefined;
           const changedFilesPaths = Object.keys(changedFilesMap);
           logger.debug("Detected changes in the following files " + changedFilesPaths);
-          changedFilesPaths.map(file => `${file}`).forEach(file => {
-            // Webpack watches to the changes in [my-theme].generated.js
-            // because it is referenced from theme-generated.js. Changes in
-            // this file should not trigger the theme handling callback (which
-            // re-generates [my-theme].generated.js),
-            // otherwise it will get into infinite re-compilation loop.
-            if (file.endsWith('.generated.js')) {
+          changedFilesPaths.forEach(changedFilePath => {
+            const file = `${changedFilePath}`;
+            const themeGeneratedFileChangedNow = file.match(this.themeGeneratedFileRegexp);
+            const timestamp = changedFilesMap[changedFilePath];
+            // null or negative timestamp means file delete
+            const fileRemoved = timestamp === null || timestamp < 0;
+
+            if (themeGeneratedFileChangedNow) {
               themeGeneratedFileChanged = true;
+              if (fileRemoved) {
+                themeGeneratedFileDeleted = true;
+              }
+            } else if (fileRemoved) {
+              const matchResult = file.match(this.componentStyleFileRegexp);
+              if (matchResult) {
+                themeName = matchResult[2];
+                deletedComponentStyleFile = file;
+              }
             }
           });
-          if (!themeGeneratedFileChanged) {
+          // This is considered as a workaround for
+          // https://github.com/vaadin/flow/issues/9948: delete component
+          // styles and theme generated file in one run to not have webpack
+          // compile error
+          if (deletedComponentStyleFile && !themeGeneratedFileDeleted) {
+            logger.warn("Custom theme component style sheet '" + deletedComponentStyleFile + "' has been deleted.\n\n" +
+              "You should also delete './frontend/generated/theme-" + themeName + ".generated.js' (simultaneously) with the component stylesheet'.\n" +
+              "Otherwise it will cause a webpack compilation error 'no such file or directory', as component style sheets are referenced from " +
+              "'./frontend/generated/theme-" + themeName + ".generated.js'.\n\n" +
+              "If you encounter a 'no such file or directory' error in your application, just click on the overlay (or refresh the browser page), and it should disappear.\n\n" +
+              "It should then be possible to continue working on the application and theming.\n" +
+              "If it doesn't help, you need to restart the application.");
+          }
+
+          // Webpack watches to the changes in [my-theme].generated.js
+          // because it is referenced from theme.js. Changes in this file
+          // should not trigger the theme handling callback (which
+          // re-generates [my-theme].generated.js),
+          // otherwise it will get into infinite re-compilation loop.
+          if (themeGeneratedFileDeleted || !themeGeneratedFileChanged) {
             this.processThemeResourcesCallback(logger);
           }
         }

flow-tests/test-application-theme/test-theme-component-live-reload/frontend/themes/app-theme/components/empty

@@ -0,0 +1 @@
+Empty file here to ensure the components folder is created

flow-tests/test-application-theme/test-theme-component-live-reload/src/test/java/com/vaadin/flow/uitest/ui/ComponentThemeLiveReloadIT.java

@@ -19,6 +19,7 @@
 import java.io.File;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
+import java.util.logging.Level;
 
 import net.jcip.annotations.NotThreadSafe;
 import org.apache.commons.io.FileUtils;
@@ -30,6 +31,7 @@
 import org.openqa.selenium.NoSuchElementException;
 import org.openqa.selenium.StaleElementReferenceException;
 import org.openqa.selenium.TimeoutException;
+import org.openqa.selenium.WebElement;
 import org.openqa.selenium.support.ui.ExpectedCondition;
 
 import com.vaadin.flow.component.html.testbench.DivElement;
@@ -46,31 +48,26 @@ public class ComponentThemeLiveReloadIT extends ChromeBrowserTest {
     private static final String OTHER_BORDER_RADIUS = "6px";
     private static final String THEME_FOLDER = "frontend/themes/app-theme/";
 
-    private File componentsDir;
     private File componentCSSFile;
+    private File themeGeneratedFile;
 
     @Before
     public void init() {
         File baseDir = new File(System.getProperty("user.dir", "."));
         final File themeFolder = new File(baseDir, THEME_FOLDER);
-
-        componentsDir = new File(themeFolder, "components");
-        createDirectoryIfAbsent(componentsDir);
-
         componentCSSFile = new File(new File(themeFolder, "components"),
                 "vaadin-text-field.css");
+        themeGeneratedFile = new File(themeFolder,
+                "app-theme.generated.js");
     }
 
     @After
     public void cleanUp() {
-        if (componentsDir.exists()) {
+        if (componentCSSFile.exists()) {
             // This waits until live reload complete to not affect the second
             // re-run in CI (if any) and to not affect other @Test methods
             // (if any appear in the future)
-            doActionAndWaitUntilLiveReloadComplete(() -> {
-                deleteComponentStyles();
-                deleteFile(componentsDir);
-            }, true);
+            doActionAndWaitUntilLiveReloadComplete(this::deleteComponentStyles);
         }
     }
 
@@ -79,9 +76,9 @@ public void webpackLiveReload_newComponentStylesCreatedAndDeleted_stylesUpdatedO
         open();
         Assert.assertFalse(
                 "Border radius for themed component is not expected before "
-                + "applying the styles",
+                        + "applying the styles",
                 isComponentCustomStyle(BORDER_RADIUS)
-                || isComponentCustomStyle(OTHER_BORDER_RADIUS));
+                        || isComponentCustomStyle(OTHER_BORDER_RADIUS));
 
         // Live reload upon adding a new component styles file
         doActionAndWaitUntilLiveReloadComplete(
@@ -94,14 +91,15 @@ public void webpackLiveReload_newComponentStylesCreatedAndDeleted_stylesUpdatedO
         waitUntilComponentCustomStyle(OTHER_BORDER_RADIUS);
 
         // Live reload upon file deletion
-        doActionAndWaitUntilLiveReloadComplete(this::deleteComponentStyles,
-                true);
+        doActionAndWaitUntilLiveReloadComplete(this::deleteComponentStyles);
         waitUntilComponentInitialStyle();
+        checkNoWebpackErrors();
     }
 
     private void waitUntilComponentInitialStyle() {
-        waitUntilWithMessage(driver -> !isComponentCustomStyle(BORDER_RADIUS)
-                            && !isComponentCustomStyle(OTHER_BORDER_RADIUS),
+        waitUntilWithMessage(
+                driver -> !isComponentCustomStyle(BORDER_RADIUS)
+                        && !isComponentCustomStyle(OTHER_BORDER_RADIUS),
                 "Wait for component initial styles timeout");
     }
 
@@ -141,25 +139,25 @@ private void createOrUpdateComponentCSSFile(String borderRadius) {
     }
 
     private void deleteComponentStyles() {
+        Assert.assertTrue("Expected theme generated file to be present",
+                themeGeneratedFile.exists());
+        // workaround for https://github.com/vaadin/flow/issues/9948
+        // delete theme generated with component styles in one run
+        deleteFile(themeGeneratedFile);
         deleteFile(componentCSSFile);
     }
 
     private void deleteFile(File fileToDelete) {
         if (fileToDelete != null && fileToDelete.exists()
-            && !fileToDelete.delete()) {
+                && !fileToDelete.delete()) {
             Assert.fail("Unable to delete " + fileToDelete);
         }
     }
 
     private void doActionAndWaitUntilLiveReloadComplete(Runnable action) {
-        doActionAndWaitUntilLiveReloadComplete(action, false);
-    }
-
-    private void doActionAndWaitUntilLiveReloadComplete(Runnable action,
-                                                        boolean deleted) {
         final String initialAttachId = getAttachIdentifier();
         action.run();
-        waitForLiveReload(initialAttachId, deleted);
+        waitForLiveReload(initialAttachId);
     }
 
     private String getAttachIdentifier() {
@@ -184,21 +182,7 @@ private String getAttachIdentifier() {
         return null;
     }
 
-    private void createDirectoryIfAbsent(File dir) {
-        if (!dir.exists() && !dir.mkdir()) {
-            Assert.fail("Unable to create folder " + dir);
-        }
-    }
-
-    private void waitForLiveReload(final String initialAttachId,
-                                   boolean deleted) {
-        if (deleted) {
-            // TODO: workaround for https://github.com/vaadin/flow/issues/9948.
-            //  one more page update is needed when a first webpack
-            //  re-compilation fails due to issue above.
-            getDriver().navigate().refresh();
-            getCommandExecutor().waitForVaadin();
-        }
+    private void waitForLiveReload(final String initialAttachId) {
         waitUntilWithMessage(d -> {
             try {
                 final String newViewId = getAttachIdentifier();
@@ -210,11 +194,32 @@ private void waitForLiveReload(final String initialAttachId,
     }
 
     private void waitUntilWithMessage(ExpectedCondition<?> condition,
-                                      String message) {
+            String message) {
         try {
             waitUntil(condition);
         } catch (TimeoutException te) {
             Assert.fail(message);
         }
     }
+
+    private void checkNoWebpackErrors() {
+        getLogEntries(Level.ALL).forEach(logEntry -> {
+            if (logEntry.getMessage().contains("Module build failed")) {
+                Assert.fail(
+                        "Webpack error detected in the browser console after "
+                                + "deleting component style sheet:\n\n"
+                                + logEntry.getMessage());
+            }
+        });
+
+        final By byErrorOverlayClass = By.className("v-system-error");
+        try {
+            waitForElementNotPresent(byErrorOverlayClass);
+        } catch (TimeoutException e) {
+            WebElement error = findElement(byErrorOverlayClass);
+            Assert.fail(
+                    "Webpack error overlay detected after deleting component "
+                            + "style sheet:\n\n" + error.getText());
+        }
+    }
 }