Skip to content

Commit

Permalink
fix: Warn to delete component CSS with theme generated in one run (#1…
Browse files Browse the repository at this point in the history 
…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
  • Loading branch information
@mshabarov
mshabarov committed Apr 27, 2021
1 parent 2808cff commit e33ec10
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 46 deletions.
61 changes: 48 additions & 13 deletions flow-server/src/main/resources/plugins/theme-live-reload-plugin/theme-live-reload-plugin.js
View file
Expand Up @@ -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 = /theme-[\s\S]*?\.generated\.js$/;
}

apply(compiler) {
Expand All @@ -43,23 +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 theme-[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 theme-[my-theme].generated.js),
// otherwise it will get into infinite re-compilation loop.
// There might be several theme generated files in the
// generated folder, so the condition does not contain the exact
// theme name
if (file.match(/theme-[\s\S]*?\.generated\.js$/)) {
themeGeneratedFileChanged = true;
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 theme-[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 theme-[my-theme].generated.js),
// otherwise it will get into infinite re-compilation loop.
if (themeGeneratedFileDeleted || !themeGeneratedFileChanged) {
this.processThemeResourcesCallback(logger);
}
}
Expand Down
1 change: 1 addition & 0 deletions ...ication-theme/test-theme-component-live-reload/frontend/themes/app-theme/components/empty
View file
@@ -0,0 +1 @@
Empty file here to ensure the components folder is created
70 changes: 37 additions & 33 deletions ...onent-live-reload/src/test/java/com/vaadin/flow/uitest/ui/ComponentThemeLiveReloadIT.java
View file
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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(baseDir,
"frontend/generated/theme-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);
}
}

Expand All @@ -94,9 +91,9 @@ 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() {
Expand Down Expand Up @@ -142,6 +139,11 @@ 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);
}

Expand All @@ -153,14 +155,9 @@ private void deleteFile(File 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() {
Expand All @@ -185,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();
Expand All @@ -218,4 +201,25 @@ private void waitUntilWithMessage(ExpectedCondition<?> condition,
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());
}
}
}

0 comments on commit e33ec10

Please sign in to comment.