Skip to content

Commit 0f940cc

Browse files
caaladormcollovati
andauthored
feat!: remove theme url translation (#21754)
* feat!: remove theme url translation Drop translation of url for theme files. * Apply suggestions from code review Co-authored-by: Marco Collovati <marco@vaadin.com> --------- Co-authored-by: Marco Collovati <marco@vaadin.com>
1 parent 1d496aa commit 0f940cc

File tree

9 files changed

+37
-129
lines changed

9 files changed

+37
-129
lines changed

flow-plugins/flow-maven-plugin/src/test/java/com/vaadin/flow/plugin/maven/BuildFrontendMojoTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -753,7 +753,7 @@ private List<String> getExpectedImports() {
753753
"@vaadin/vaadin-mixed-component/src/vaadin-something-else.js",
754754
"./generated/jar-resources/ExampleConnector.js",
755755
"./local-p3-template.js", "./foo.js",
756-
"./vaadin-mixed-component/theme/lumo/vaadin-mixed-component.js",
756+
"./vaadin-mixed-component/src/vaadin-mixed-component.js",
757757
"./local-template.js", "./foo-dir/vaadin-npm-component.js");
758758
}
759759

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -581,7 +581,9 @@ private Set<String> getUniqueEs6ImportPaths(Collection<String> modules) {
581581
String translatedModulePath = originalModulePath;
582582
String localModulePath = null;
583583
if (theme != null
584-
&& translatedModulePath.contains(theme.getBaseUrl())) {
584+
&& (originalModulePath.startsWith(theme.getBaseUrl())
585+
|| originalModulePath
586+
.startsWith("./" + theme.getBaseUrl()))) {
585587
translatedModulePath = theme.translateUrl(translatedModulePath);
586588
localModulePath = themeToLocalPathConverter
587589
.apply(translatedModulePath);
@@ -896,8 +898,7 @@ private void visitImportsRecursively(Path filePath, String path,
896898
File file = getImportedFrontendFile(resolvedPath);
897899
if (file == null && !importedPath.startsWith("./")) {
898900
// In case such file doesn't exist it may be
899-
// external: inside
900-
// node_modules folder
901+
// external: inside node_modules folder
901902
file = getFile(options.getNodeModulesFolder(),
902903
importedPath);
903904
if (!file.exists()) {
@@ -916,13 +917,17 @@ private void visitImportsRecursively(Path filePath, String path,
916917
List<String> resolvedPaths = resolvedImportPathsCache.get(filePath);
917918

918919
for (String resolvedPath : resolvedPaths) {
919-
if (resolvedPath.contains(theme.getBaseUrl())) {
920+
if (resolvedPath.startsWith(theme.getBaseUrl())
921+
|| resolvedPath.startsWith("./" + theme.getBaseUrl())) {
920922
String translatedPath = theme.translateUrl(resolvedPath);
921923
if (!visitedImports.contains(translatedPath)
922924
&& importedFileExists(translatedPath)) {
923925
visitedImports.add(translatedPath);
924926
imports.add(normalizeImportPath(translatedPath));
925927
}
928+
} else {
929+
visitedImports.add(resolvedPath);
930+
imports.add(normalizeImportPath(resolvedPath));
926931
}
927932
handleImports(resolvedPath, theme, imports, visitedImports);
928933
}

flow-server/src/main/java/com/vaadin/flow/theme/AbstractTheme.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,9 @@ public interface AbstractTheme extends Serializable {
3434
* e.g. src/
3535
*
3636
* @return the base component path
37+
* @deprecated base url will not be translated to the theme url anymore
3738
*/
39+
@Deprecated(since = "25.0", forRemoval = true)
3840
String getBaseUrl();
3941

4042
/**
@@ -43,7 +45,9 @@ public interface AbstractTheme extends Serializable {
4345
* e.g. theme/lumo/
4446
*
4547
* @return the themed component path
48+
* @deprecated base url will not be translated to the theme url anymore
4649
*/
50+
@Deprecated(since = "25.0", forRemoval = true)
4751
String getThemeUrl();
4852

4953
/**
@@ -91,10 +95,11 @@ default Map<String, String> getHtmlAttributes(String variant) {
9195
* @param url
9296
* the URL to translate using the theme
9397
* @return translated URL if possible or the same given {@code url} if not.
98+
* @deprecated No translation of the url will be done in 25
9499
*/
100+
@Deprecated(since = "25.0", forRemoval = true)
95101
default String translateUrl(String url) {
96102
if (url.contains(getBaseUrl())) {
97-
98103
String baseUrl = getBaseUrl();
99104
String themeUrl = getThemeUrl();
100105
if (baseUrl.endsWith("/") && !themeUrl.endsWith("/")) {

flow-server/src/test/java/com/vaadin/flow/server/frontend/AbstractNodeUpdateImportsTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ public void generateImportsFile_fileContainsThemeLinesAndExpectedImportsAndCssIm
110110

111111
// An import without `.js` extension
112112
expectedLines.add(
113-
"import '@vaadin/vaadin-mixed-component/theme/lumo/vaadin-something-else'");
113+
"import '@vaadin/vaadin-mixed-component/src/vaadin-something-else'");
114114
// An import not found in node_modules
115115
expectedLines.add("import 'unresolved/component';");
116116

@@ -157,7 +157,7 @@ public void generateImportsFile_fileContainsThemeLinesAndExpectedImportsAndCssIm
157157
"Use the './' prefix for files in JAR files: 'ExampleConnector.js'",
158158
"Use the './' prefix for files in the '"
159159
+ frontendDirectory.getPath()
160-
+ "' folder: 'vaadin-mixed-component/theme/lumo/vaadin-mixed-component.js'");
160+
+ "' folder: 'vaadin-mixed-component/src/vaadin-mixed-component.js'");
161161

162162
// Using regex match because of the ➜ character in TC
163163
assertContains(output, true,

flow-server/src/test/java/com/vaadin/flow/server/frontend/AbstractUpdateImportsTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ public void generateLines_resultingLinesContainsThemeLinesAndExpectedImportsAndC
317317

318318
// An import without `.js` extension
319319
expectedLines.add(
320-
"import '@vaadin/vaadin-mixed-component/theme/lumo/vaadin-something-else';");
320+
"import '@vaadin/vaadin-mixed-component/src/vaadin-something-else';");
321321
// An import not found in node_modules
322322
expectedLines.add("import 'unresolved/component';");
323323

@@ -381,7 +381,7 @@ public void generateLines_resultingLinesContainsThemeLinesAndExpectedImportsAndC
381381
MatcherAssert.assertThat(output, CoreMatchers
382382
.containsString("Use the './' prefix for files in the '"
383383
+ frontendDirectory.getPath()
384-
+ "' folder: 'vaadin-mixed-component/theme/lumo/vaadin-mixed-component.js'"));
384+
+ "' folder: 'vaadin-mixed-component/src/vaadin-mixed-component.js'"));
385385

386386
// Using regex match because of the ➜ character in TC
387387
MatcherAssert.assertThat(output, CoreMatchers.containsString(

flow-server/src/test/java/com/vaadin/flow/server/frontend/BundleValidationTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -896,7 +896,7 @@ public void themedGeneratedFlowImports_bundleUsesTheme_noBuildRequired()
896896
ArrayNode bundleImports = (ArrayNode) stats.get(BUNDLE_IMPORTS);
897897
bundleImports.add("@polymer/paper-checkbox/paper-checkbox.js");
898898
bundleImports.add("@polymer/paper-input/paper-input.js");
899-
bundleImports.add("@vaadin/grid/theme/lumo/vaadin-grid.js");
899+
bundleImports.add("@vaadin/grid/src/vaadin-grid.js");
900900
bundleImports
901901
.add("Frontend/generated/jar-resources/dndConnector-es6.js");
902902

flow-server/src/test/java/com/vaadin/flow/server/frontend/NodeUpdateTestUtil.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -89,15 +89,15 @@ List<String> getExpectedImports() {
8989
"@vaadin/vaadin-lumo-styles/color.js",
9090
"@vaadin/vaadin-lumo-styles/color-global.js",
9191
"@vaadin/vaadin-lumo-styles/sizing.js",
92-
"@vaadin/vaadin-date-picker/theme/lumo/vaadin-date-picker.js",
92+
"@vaadin/vaadin-date-picker/src/vaadin-date-picker.js",
9393
"@vaadin/vaadin-date-picker/src/vaadin-month-calendar.js",
9494
"@vaadin/vaadin-element-mixin/vaadin-element-mixin.js",
95-
"@vaadin/vaadin-mixed-component/theme/lumo/vaadin-mixed-component.js",
96-
"@vaadin/vaadin-mixed-component/theme/lumo/vaadin-something-else.js",
97-
"./theme/lumo/vaadin-custom-themed-component.js",
95+
"@vaadin/vaadin-mixed-component/src/vaadin-mixed-component.js",
96+
"@vaadin/vaadin-mixed-component/src/vaadin-something-else.js",
97+
"@vaadin/vaadin-mixed-component/src/vaadin-custom-themed-component.js",
9898
"./generated/jar-resources/ExampleConnector.js",
9999
"3rdparty/component.js", "./local-p3-template.js", "./foo.js",
100-
"./vaadin-mixed-component/theme/lumo/vaadin-mixed-component.js",
100+
"./vaadin-mixed-component/src/vaadin-mixed-component.js",
101101
"./local-template.js", "./foo-dir/vaadin-npm-component.js",
102102
"./foo.css", "@vaadin/vaadin-mixed-component/bar.css",
103103
"./common-js-file.js",

flow-server/src/test/java/com/vaadin/flow/server/frontend/UpdateThemedImportsTest.java

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -155,17 +155,17 @@ public void themedClientSideModulesAreWrittenIntoImportsFile()
155155

156156
String content = FileUtils.readFileToString(importsFile,
157157
Charset.defaultCharset());
158-
MatcherAssert.assertThat(content, CoreMatchers.allOf(
159-
CoreMatchers.containsString(
160-
"import 'Frontend/theme/myTheme/main-template.js';"),
161-
CoreMatchers.containsString(
162-
"import 'Frontend/theme/myTheme/client-side-template.js';"),
163-
CoreMatchers.containsString(
164-
"import 'Frontend/theme/myTheme/subfolder/sub-template.js';"),
165-
CoreMatchers.containsString(
166-
"import '@vaadin/vaadin-button/theme/myTheme/vaadin-button.js';"),
158+
MatcherAssert.assertThat(content, CoreMatchers.containsString(
159+
"import 'Frontend/theme/myTheme/main-template.js';"));
160+
MatcherAssert.assertThat(content, CoreMatchers.containsString(
161+
"import 'Frontend/theme/myTheme/client-side-template.js';"));
162+
MatcherAssert.assertThat(content, CoreMatchers.containsString(
163+
"import 'Frontend/theme/myTheme/subfolder/sub-template.js';"));
164+
MatcherAssert.assertThat(content, CoreMatchers.containsString(
165+
"import '@vaadin/vaadin-button/src/vaadin-button.js';"));
166+
MatcherAssert.assertThat(content,
167167
CoreMatchers.not(CoreMatchers.containsString(
168-
"import 'theme/myTheme/wrong-themed-template.js';"))));
168+
"import 'theme/myTheme/wrong-themed-template.js';")));
169169
}
170170

171171
@Test
@@ -176,7 +176,7 @@ public void noDuplicateImportEntryIsWrittenIntoImportsFile()
176176
String content = FileUtils.readFileToString(importsFile,
177177
Charset.defaultCharset());
178178
int count = StringUtils.countMatches(content,
179-
"import '@vaadin/vaadin-button/theme/myTheme/vaadin-button.js';");
179+
"import '@vaadin/vaadin-button/src/vaadin-button.js';");
180180
Assert.assertEquals(
181181
"Import entries in the imports file should be unique.", 1,
182182
count);

flow-server/src/test/java/com/vaadin/flow/theme/AbstractThemeTest.java

Lines changed: 0 additions & 102 deletions
This file was deleted.

0 commit comments

Comments
 (0)