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: Warn to delete component CSS with theme generated in one run #10828

Merged
merged 1 commit into from
Apr 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
@@ -0,0 +1 @@
Empty file here to ensure the components folder is created
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 @@ -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(
Expand All @@ -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");
}

Expand Down Expand Up @@ -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() {
Expand All @@ -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();
Expand All @@ -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());
}
}
}