diff --git a/flow-build-tools/src/main/java/com/vaadin/flow/server/frontend/BundleValidationUtil.java b/flow-build-tools/src/main/java/com/vaadin/flow/server/frontend/BundleValidationUtil.java index 9a922e9cab5..5627c1c6d95 100644 --- a/flow-build-tools/src/main/java/com/vaadin/flow/server/frontend/BundleValidationUtil.java +++ b/flow-build-tools/src/main/java/com/vaadin/flow/server/frontend/BundleValidationUtil.java @@ -278,6 +278,17 @@ private static boolean needsBuildInternal(Options options, ((ObjectNode) statsJson.get(FRONTEND_HASHES_STATS_KEY)).remove( FrontendUtils.GENERATED + FrontendUtils.COMMERCIAL_BANNER_JS); + if (jsDefinitionsChanged(options, statsJson)) { + UsageStatistics.markAsUsed( + "flow/rebundle-reason-changed-js-definitions", null); + return true; + } + // JavaScript definition file hash has already been checked + // removing it from hashes map to prevent other unnecessary checks + ((ObjectNode) statsJson.get(FRONTEND_HASHES_STATS_KEY)) + .remove(FrontendUtils.GENERATED + + FrontendUtils.JS_DEFINITIONS_FILE_NAME); + if (!BundleValidationUtil.frontendImportsFound(statsJson, options)) { UsageStatistics.markAsUsed( "flow/rebundle-reason-missing-frontend-import", null); @@ -993,6 +1004,37 @@ private static boolean isCommercialBannerConditionChanged(Options options, return false; } + /** + * Checks whether the JavaScript that the {@code @JsDefinition} interfaces + * of the application declare differs from what the bundle was built with. + *

+ * The functions are generated into the bundle, so a declaration that + * changed, a definition that was added and a bundle built before any + * definition existed all mean that the bundle no longer contains what a + * call would look up, which shows up at runtime as a call that cannot be + * run. + */ + private static boolean jsDefinitionsChanged(Options options, + JsonNode statsJson) { + JsonNode frontendHashes = statsJson.get(FRONTEND_HASHES_STATS_KEY); + String jsDefinitionsPath = FrontendUtils.GENERATED + + FrontendUtils.JS_DEFINITIONS_FILE_NAME; + String content = new TaskGenerateJsDefinitions(options) + .getFileContent(); + + List faultyContent = new ArrayList<>(); + compareFrontendHashes(frontendHashes, faultyContent, jsDefinitionsPath, + content); + if (!faultyContent.isEmpty()) { + // Either the declarations changed, or the bundle was built before + // they existed and carries none of their JavaScript + getLogger().info( + "Detected JavaScript declared by the JavaScript definitions that the bundle does not carry"); + return true; + } + return false; + } + private static Map getRemainingImports( List jarImports, List projectImports, JsonNode frontendHashes) { diff --git a/flow-build-tools/src/main/java/com/vaadin/flow/server/frontend/NodeTasks.java b/flow-build-tools/src/main/java/com/vaadin/flow/server/frontend/NodeTasks.java index 8bd6909e6d3..ed8d2df20ff 100644 --- a/flow-build-tools/src/main/java/com/vaadin/flow/server/frontend/NodeTasks.java +++ b/flow-build-tools/src/main/java/com/vaadin/flow/server/frontend/NodeTasks.java @@ -76,6 +76,7 @@ public class NodeTasks implements FallibleCommand { TaskGenerateWebComponentHtml.class, TaskGenerateWebComponentBootstrap.class, TaskGenerateFeatureFlags.class, + TaskGenerateJsDefinitions.class, TaskInstallFrontendBuildPlugins.class, TaskUpdatePackages.class, TaskRunNpmInstall.class, @@ -262,6 +263,8 @@ public NodeTasks(Options options) { commands.add(new TaskGenerateFeatureFlags(options)); + commands.add(new TaskGenerateJsDefinitions(options)); + if (options.getJarFiles() != null && options.getJarFrontendResourcesFolder() != null) { commands.add(new TaskCopyFrontendFiles(options)); diff --git a/flow-build-tools/src/main/java/com/vaadin/flow/server/frontend/TaskGenerateBootstrap.java b/flow-build-tools/src/main/java/com/vaadin/flow/server/frontend/TaskGenerateBootstrap.java index ed849341f99..51cb15975ed 100644 --- a/flow-build-tools/src/main/java/com/vaadin/flow/server/frontend/TaskGenerateBootstrap.java +++ b/flow-build-tools/src/main/java/com/vaadin/flow/server/frontend/TaskGenerateBootstrap.java @@ -32,6 +32,7 @@ import static com.vaadin.flow.internal.FrontendUtils.INDEX_JS; import static com.vaadin.flow.internal.FrontendUtils.INDEX_TS; import static com.vaadin.flow.internal.FrontendUtils.INDEX_TSX; +import static com.vaadin.flow.internal.FrontendUtils.JS_DEFINITIONS_FILE_NAME; /** * A task for generating the bootstrap file @@ -83,6 +84,8 @@ protected String getFileContent() { for (TypeScriptBootstrapModifier modifier : modifiers) { modifier.modify(lines, options); } + lines.add(0, + String.format("import './%s';%n", JS_DEFINITIONS_FILE_NAME)); lines.add(0, String.format("import './%s';%n", FEATURE_FLAGS_FILE_NAME)); return String.join(System.lineSeparator(), lines); diff --git a/flow-build-tools/src/main/java/com/vaadin/flow/server/frontend/TaskGenerateJsDefinitions.java b/flow-build-tools/src/main/java/com/vaadin/flow/server/frontend/TaskGenerateJsDefinitions.java new file mode 100644 index 00000000000..8ec54c1496c --- /dev/null +++ b/flow-build-tools/src/main/java/com/vaadin/flow/server/frontend/TaskGenerateJsDefinitions.java @@ -0,0 +1,386 @@ +/* + * Copyright 2000-2026 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.flow.server.frontend; + +import java.io.File; +import java.io.IOException; +import java.lang.reflect.Method; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Comparator; +import java.util.List; +import java.util.Objects; +import java.util.stream.IntStream; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.vaadin.flow.internal.FrontendUtils; +import com.vaadin.flow.js.JsCall; +import com.vaadin.flow.js.JsDefinition; +import com.vaadin.flow.js.JsExpression; +import com.vaadin.flow.server.frontend.scanner.ClassFinder; + +import static com.vaadin.flow.internal.FrontendUtils.GENERATED; +import static com.vaadin.flow.internal.FrontendUtils.JS_DEFINITIONS_FILE_NAME; + +/** + * Generates {@link FrontendUtils#JS_DEFINITIONS_FILE_NAME}, which registers the + * JavaScript of every {@link JsDefinition} interface on the class path as an + * ordinary function of the bundle. + *

+ * This is what lets the client run a server-initiated call without compiling + * anything from a string: the server sends the identifier of the function to + * run, which is a hash of the JavaScript, and the function is already in the + * bundle. The call survives a content security policy that does not allow + * unsafe-eval, the JavaScript an application can be made to run is + * known when it is built, and what declared it in Java stays there. + *

+ * Outside production mode the file also registers what a developer wrote for + * each function, so that a message about a call in the browser names it rather + * than a hash. A production bundle carries the functions alone. + *

+ * For internal use only. May be renamed or removed in a future release. + */ +public class TaskGenerateJsDefinitions extends AbstractTaskClientGenerator { + + private static final List HEADER = List.of("// @ts-nocheck", + "window.Vaadin = window.Vaadin || {};", + "window.Vaadin.Flow = window.Vaadin.Flow || {};", + "window.Vaadin.Flow.jsDefinitions = window.Vaadin.Flow.jsDefinitions || {};"); + + // What a message about a call names it by, which is of no use to a browser + // running the application and is left out of a production bundle + private static final String NAMES = "window.Vaadin.Flow.jsDefinitionNames = window.Vaadin.Flow.jsDefinitionNames || {};"; + + // Writing this file again while the application runs replaces it in the + // browser that has it: everything above only writes into the registry, so + // the module can accept its own update and nothing else has to be reloaded + // for a changed declaration to take effect. The dev server drops the block + // from a production build, where import.meta.hot is not defined. + // The export is for https://github.com/vaadin/flow/issues/14184 + private static final List FOOTER = List.of("if (import.meta.hot) {", + " import.meta.hot.accept();", "}", "export {};"); + + private final Options options; + + TaskGenerateJsDefinitions(Options options) { + this.options = options; + } + + @Override + protected String getFileContent() { + // Generating the file is scanning for what goes into it, which a + // caller that writes it again while the application runs does not do: + // it passes the definitions in, through updateJsDefinitions + ClassFinder classFinder = Objects.requireNonNull( + options.getClassFinder(), + "Generating the file needs a class finder to scan for the JavaScript definitions with"); + return renderFileContent( + classFinder.getAnnotatedClasses(JsDefinition.class), + !options.isProductionMode()); + } + + /** + * Renders the file that registers the JavaScript of the given definition + * interfaces. + *

+ * Package private: what regenerating the file outside a build looks like is + * {@link #updateJsDefinitions(Options, Collection)}, which goes through + * this. + * + * @param definitions + * the JavaScript definitions to render, not null + * @param withNames + * whether to register what a message about a call names it by, + * which is for a development bundle + * @return the content of the generated file + */ + static String renderFileContent(Collection> definitions, + boolean withNames) { + List lines = new ArrayList<>(renderHeader(withNames)); + definitions.stream().sorted(Comparator.comparing(Class::getName)) + .forEach(definition -> lines + .addAll(renderDefinitionLines(definition, withNames))); + lines.addAll(FOOTER); + return String.join(System.lineSeparator(), lines); + } + + /** + * The JavaScript definitions of the given ones whose JavaScript the + * generated file does not carry, which is the JavaScript a browser can run + * of them. + * + * @param options + * where the file is, not null + * @param definitions + * the JavaScript definitions to look for, not null + * @return those the file does not carry, empty when it carries all of them + */ + public static List> findMissingFromGeneratedFile(Options options, + Collection> definitions) { + String generated = readGeneratedFile(options); + return definitions.stream() + .filter(definition -> !isInGeneratedFile(definition, generated)) + .toList(); + } + + /** + * Writes the generated file again so that it carries what the given + * definitions declare, for a caller that has to update it while the + * application runs rather than as part of a build. + *

+ * What the file already holds is kept: a function it registers is what a + * browser that has the file can run, and the caller only knows about the + * definitions it passes in. A function nothing declares any longer stays in + * the file with nothing calling it, until a build renders the file again. + *

+ * Goes through the same write as {@link #execute()}, which leaves the file + * alone when its content would not change and writes it atomically + * otherwise, so a dev server is not told about an update that is not one + * and never reads a file that is half written. + * + * @param options + * where the file is, not null + * @param definitions + * the JavaScript definitions to write it for, not + * null and not empty + * @return those of them the file does not carry afterwards, empty when it + * carries all of them + */ + public static List> updateJsDefinitions(Options options, + Collection> definitions) { + String generated = readGeneratedFile(options); + String content = withMissingEntries(generated, definitions); + + TaskGenerateJsDefinitions task = new TaskGenerateJsDefinitions(options); + try { + task.writeIfChanged(task.getGeneratedFile(), content); + } catch (IOException e) { + getLogger().debug("Could not write {}", task.getGeneratedFile(), e); + // The file is as it was, so only what it was already missing is + // missing now + return definitions.stream().filter( + definition -> !isInGeneratedFile(definition, generated)) + .toList(); + } + // Everything asked for went into the content that was written + return List.of(); + } + + /** + * Whether the given content carries what the definition declares, compared + * as this class renders it for a development build - the one the caller of + * this runs in - so the JavaScript of every method, the number of arguments + * it takes and its name have to match. A method that was removed does not + * show up as a difference: its function stays in the file with nothing + * calling it. + */ + private static boolean isInGeneratedFile(Class definition, + String generated) { + if (generated == null) { + return false; + } + List declared = renderDefinitionLines(definition, true); + if (declared.isEmpty()) { + // Declares no JavaScript, so there is nothing to carry + return true; + } + return generated + .contains(String.join(System.lineSeparator(), declared)); + } + + /** + * The given content with the functions of the given definitions that it + * does not hold yet put in front of what closes the file, or the whole file + * rendered when there is nothing to add to. + *

+ * Only the functions that are not in the content are added, so editing one + * method of an interface does not write the others a second time. Nothing + * the content holds is taken out of it: it is what a browser that has the + * file can run, and this is asked for the definitions that changed rather + * than for everything an application declares. + */ + private static String withMissingEntries(String generated, + Collection> definitions) { + if (generated == null || generated.isBlank()) { + return renderFileContent(definitions, true); + } + List missing = definitions.stream() + .sorted(Comparator.comparing(Class::getName)) + .flatMap(definition -> renderFunctions(definition, true) + .stream()) + .filter(function -> !generated.contains(function)) + .flatMap(function -> Arrays + .stream(function.split(System.lineSeparator()))) + .toList(); + if (missing.isEmpty()) { + return generated; + } + String separator = System.lineSeparator(); + String footer = String.join(separator, FOOTER); + List added = new ArrayList<>(); + if (!generated.contains(NAMES)) { + // Written before names were registered at all, and a name is + // assigned into an object that has to be there + added.add(NAMES); + } + added.addAll(missing); + if (generated.contains(footer)) { + return generated.replace(footer, + String.join(separator, added) + separator + footer); + } + // Written by another version of this class: what it holds is what a + // browser has, so the functions go after it rather than instead of it. + // The header only assigns what is not there, so repeating it is what + // makes the content that follows land in the registry. + return generated + separator + + String.join(separator, renderHeader(true)) + separator + + String.join(separator, missing) + separator + footer; + } + + /** + * What a generated file opens with: the registry a function is assigned + * into, and the one a name is assigned into when names are rendered. Both + * assign only what is not there, so a file can carry them more than once. + */ + private static List renderHeader(boolean withNames) { + List header = new ArrayList<>(HEADER); + if (withNames) { + header.add(NAMES); + } + return header; + } + + private static String readGeneratedFile(Options options) { + File generatedFile = new TaskGenerateJsDefinitions(options) + .getGeneratedFile(); + if (!generatedFile.exists()) { + return null; + } + try { + return Files.readString(generatedFile.toPath(), + StandardCharsets.UTF_8); + } catch (IOException e) { + getLogger().debug("Could not read {}", generatedFile, e); + return null; + } + } + + private static Logger getLogger() { + return LoggerFactory.getLogger(TaskGenerateJsDefinitions.class); + } + + /** + * Renders what one JavaScript definition contributes to the generated file: + * one function per method that declares JavaScript, registered under the + * identifier of that function, which is what the server sends. The name of + * the interface and of the method are not in it, so a browser is not told + * what declared the JavaScript it runs. + *

+ * Package private: whether a file carries what an interface declares is + * answered by {@link #findMissingFromGeneratedFile(Options, Collection)}, + * which compares against this. + * + * @param definition + * the JavaScript definition to render, not null + * @return the lines this definition contributes, empty if it declares no + * JavaScript + */ + static List renderDefinitionLines(Class definition, + boolean withNames) { + return renderFunctions(definition, withNames).stream() + .flatMap(function -> Arrays + .stream(function.split(System.lineSeparator()))) + .toList(); + } + + /** + * What one JavaScript definition contributes, one registered function per + * method that declares JavaScript, each as the lines it is written as. + */ + private static List renderFunctions(Class definition, + boolean withNames) { + List methods = new ArrayList<>(); + for (Method method : definition.getMethods()) { + if (method.isAnnotationPresent(JsExpression.class)) { + methods.add(method); + } + } + methods.sort( + Comparator.comparing(TaskGenerateJsDefinitions::functionId)); + + List functions = new ArrayList<>(); + for (Method method : methods) { + // The parameters of the generated function are the arguments of the + // call, referenced as $0, $1, ... by the declared expression, and + // the element the definition was obtained from is its `this` - the + // same contract as an executeJs expression has. + String parameters = IntStream.range(0, method.getParameterCount()) + .mapToObj(index -> "$" + index) + .reduce((first, second) -> first + ", " + second) + .orElse(""); + List function = new ArrayList<>(List.of(String.format( + "window.Vaadin.Flow.jsDefinitions[%s] = async function (%s) {", + quote(functionId(method)), parameters), + method.getAnnotation(JsExpression.class).value(), "};")); + if (withNames) { + function.add(String.format( + "window.Vaadin.Flow.jsDefinitionNames[%s] = %s;", + quote(functionId(method)), + quote(nameOf(definition, method)))); + } + functions.add(String.join(System.lineSeparator(), function)); + } + return functions; + } + + /** + * What a message about a call of the given method names it by: what a + * developer wrote, rather than the hash the call itself carries. + */ + private static String nameOf(Class definition, Method method) { + return definition.getName() + "." + method.getName() + "/" + + method.getParameterCount(); + } + + private static String functionId(Method method) { + return JsCall.functionId( + method.getAnnotation(JsExpression.class).value(), + method.getParameterCount()); + } + + private static String quote(String value) { + return "\"" + value.replace("\\", "\\\\").replace("\"", "\\\"") + "\""; + } + + @Override + protected File getGeneratedFile() { + File frontendGeneratedDirectory = new File( + options.getFrontendDirectory(), GENERATED); + return new File(frontendGeneratedDirectory, JS_DEFINITIONS_FILE_NAME); + } + + @Override + protected boolean shouldGenerate() { + // Whether an application declares any JavaScript is answered by + // scanning for it, which a build that scans for anything can do + return true; + } +} diff --git a/flow-build-tools/src/main/java/com/vaadin/flow/server/frontend/TaskGenerateWebComponentBootstrap.java b/flow-build-tools/src/main/java/com/vaadin/flow/server/frontend/TaskGenerateWebComponentBootstrap.java index 29fe303c917..0ca8243590a 100644 --- a/flow-build-tools/src/main/java/com/vaadin/flow/server/frontend/TaskGenerateWebComponentBootstrap.java +++ b/flow-build-tools/src/main/java/com/vaadin/flow/server/frontend/TaskGenerateWebComponentBootstrap.java @@ -24,6 +24,7 @@ import static com.vaadin.flow.internal.FrontendUtils.FEATURE_FLAGS_FILE_NAME; import static com.vaadin.flow.internal.FrontendUtils.GENERATED; +import static com.vaadin.flow.internal.FrontendUtils.JS_DEFINITIONS_FILE_NAME; import static com.vaadin.flow.internal.FrontendUtils.WEB_COMPONENT_BOOTSTRAP_FILE_NAME; /** @@ -59,6 +60,7 @@ public class TaskGenerateWebComponentBootstrap protected String getFileContent() { List lines = new ArrayList<>(); lines.add(String.format("import './%s';%n", FEATURE_FLAGS_FILE_NAME)); + lines.add(String.format("import './%s';%n", JS_DEFINITIONS_FILE_NAME)); lines.add("import 'Frontend/generated/flow/" + FrontendUtils.IMPORTS_WEB_COMPONENT_NAME + "';"); // By path rather than through the `vaadin-flow-client` specifier that diff --git a/flow-build-tools/src/test/java/com/vaadin/flow/server/frontend/BundleValidationTest.java b/flow-build-tools/src/test/java/com/vaadin/flow/server/frontend/BundleValidationTest.java index d0fa157551f..fd0a764a5d0 100644 --- a/flow-build-tools/src/test/java/com/vaadin/flow/server/frontend/BundleValidationTest.java +++ b/flow-build-tools/src/test/java/com/vaadin/flow/server/frontend/BundleValidationTest.java @@ -31,6 +31,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.io.TempDir; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; import org.mockito.MockedStatic; import org.mockito.Mockito; @@ -203,6 +204,14 @@ public void execute() { frontendHashes.put("theme-util.js", BundleValidationUtil.calculateHash(THEME_UTIL_JS)); jarResources.put("theme-util.js", THEME_UTIL_JS); + // A bundle carries the JavaScript declared by the JavaScript + // definitions + frontendHashes.put( + FrontendUtils.GENERATED + + FrontendUtils.JS_DEFINITIONS_FILE_NAME, + BundleValidationUtil + .calculateHash(new TaskGenerateJsDefinitions(options) + .getFileContent())); return stats; } @@ -1067,6 +1076,40 @@ void frontendFileHashMatches_noBundleRebuild(Mode mode) throws IOException { assertFalse(needsBuild, "Jar fronted file content hash should match."); } + static Stream modesAndBundleHashes() { + // What the stats say the bundle was built with: a hash the declarations + // do not produce, so it was built with another version of them, and no + // hash at all, as in a bundle built before any definition existed + return modes().flatMap(mode -> Stream.of( + Arguments.of(mode, + "not the hash of what the interfaces declare"), + Arguments.of(mode, null))); + } + + @ParameterizedTest + @MethodSource("modesAndBundleHashes") + void jsDefinitionJavaScriptNotInTheBundle_bundleRebuild(Mode mode, + String bundleHash) { + setupMode(mode); + + ObjectNode stats = getBasicStats(); + ObjectNode hashes = (ObjectNode) stats.get(FRONTEND_HASHES); + String generatedFile = FrontendUtils.GENERATED + + FrontendUtils.JS_DEFINITIONS_FILE_NAME; + if (bundleHash == null) { + hashes.remove(generatedFile); + } else { + hashes.put(generatedFile, bundleHash); + } + setupFrontendUtilsMock(stats); + + boolean needsBuild = BundleValidationUtil.needsBuild(options, + depScanner, mode); + + assertTrue(needsBuild, + "JavaScript declared by a JavaScript definition that the bundle was not built with should trigger a rebuild, whether the bundle carries another version of it or none at all"); + } + @ParameterizedTest @MethodSource("modes") void noFrontendFileHash_bundleRebuild(Mode mode) throws IOException { diff --git a/flow-build-tools/src/test/java/com/vaadin/flow/server/frontend/TaskGenerateJsDefinitionsTest.java b/flow-build-tools/src/test/java/com/vaadin/flow/server/frontend/TaskGenerateJsDefinitionsTest.java new file mode 100644 index 00000000000..fbe0abfa220 --- /dev/null +++ b/flow-build-tools/src/test/java/com/vaadin/flow/server/frontend/TaskGenerateJsDefinitionsTest.java @@ -0,0 +1,384 @@ +/* + * Copyright 2000-2026 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.flow.server.frontend; + +import java.io.File; +import java.io.IOException; +import java.io.Serializable; +import java.nio.file.Files; +import java.util.List; +import java.util.Set; +import java.util.regex.Pattern; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.mockito.Mockito; + +import com.vaadin.flow.di.Lookup; +import com.vaadin.flow.internal.FrontendUtils; +import com.vaadin.flow.js.JsCall; +import com.vaadin.flow.js.JsDefinition; +import com.vaadin.flow.js.JsExpression; +import com.vaadin.flow.server.frontend.scanner.ClassFinder.DefaultClassFinder; + +import static com.vaadin.flow.internal.FrontendUtils.FRONTEND; +import static com.vaadin.flow.internal.FrontendUtils.GENERATED; +import static com.vaadin.flow.internal.FrontendUtils.JS_DEFINITIONS_FILE_NAME; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +class TaskGenerateJsDefinitionsTest { + + private static final String GREETING_EXPRESSION = "window.alert({ text: $0, kind: 'greeting' })"; + + private static final String COUNT_EXPRESSION = "this.count = ($0 || 0) + 1"; + + private static final String NAMES_REGISTRY = "window.Vaadin.Flow.jsDefinitionNames = window.Vaadin.Flow.jsDefinitionNames || {};"; + + @JsDefinition + public interface GreeterJs extends Serializable { + @JsExpression(GREETING_EXPRESSION) + void showGreeting(String greeting); + + @JsExpression("window.alert('Hello')") + void showGreeting(); + } + + @JsDefinition + public interface CounterJs extends Serializable { + @JsExpression(COUNT_EXPRESSION) + void count(Integer from); + } + + @JsDefinition + public interface NothingJs extends Serializable { + void notDeclared(); + } + + @TempDir + File temporaryFolder; + + private TaskGenerateJsDefinitions task; + private Options options; + private File frontendFolder; + + @BeforeEach + void setUp() { + frontendFolder = new File(temporaryFolder, FRONTEND); + frontendFolder.mkdirs(); + options = new Options(Mockito.mock(Lookup.class), + new DefaultClassFinder( + Set.of(GreeterJs.class, NothingJs.class)), + null).withFrontendDirectory(frontendFolder) + .withProductionMode(false); + task = new TaskGenerateJsDefinitions(options); + } + + @Test + void generatesAFunctionPerDeclaredExpression() + throws ExecutionFailedException { + task.execute(); + String content = task.getFileContent(); + + assertTrue( + content.contains("window.Vaadin.Flow.jsDefinitions[\"" + + JsCall.functionId(GREETING_EXPRESSION, 1) + + "\"] = async function ($0) {"), + "a function should be registered under the identifier of the JavaScript it runs: " + + content); + assertTrue(content.contains(GREETING_EXPRESSION), + "the declared expression should be the body of the function, as it was written: " + + content); + assertTrue( + content.contains("window.Vaadin.Flow.jsDefinitions[\"" + + JsCall.functionId("window.alert('Hello')", 0) + + "\"] = async function () {"), + "the overload that takes no arguments is another function: " + + content); + } + + @Test + void generatedFile_developmentMode_namesWhatDeclaredTheJavaScript() + throws ExecutionFailedException { + // Which is what a message about a call says instead of a hash, and is + // of no use to a browser running the application + task.execute(); + String content = task.getFileContent(); + + assertTrue(content.contains(NAMES_REGISTRY), + "the registry a name is assigned into has to be there, or the module throws: " + + content); + assertTrue( + content.contains("window.Vaadin.Flow.jsDefinitionNames[\"" + + JsCall.functionId(GREETING_EXPRESSION, 1) + "\"] = \"" + + GreeterJs.class.getName() + ".showGreeting/1\";"), + "and the name should be registered next to the function: " + + content); + } + + @Test + void updateJsDefinitions_writesTheNamesAndWhatHoldsThem() + throws IOException { + // Into a file that a build wrote before names were rendered at all + File generated = new File( + FrontendUtils.getFrontendGeneratedFolder(frontendFolder), + FrontendUtils.JS_DEFINITIONS_FILE_NAME); + generated.getParentFile().mkdirs(); + Files.writeString(generated.toPath(), TaskGenerateJsDefinitions + .renderFileContent(List.of(GreeterJs.class), false)); + + List> missing = TaskGenerateJsDefinitions + .updateJsDefinitions(options, List.of(CounterJs.class)); + + assertTrue(missing.isEmpty()); + String written = Files.readString(generated.toPath()); + assertTrue(written.contains(NAMES_REGISTRY), + "the registry the names are assigned into should be added to a file that has none: " + + written); + assertTrue( + written.indexOf(NAMES_REGISTRY) < written + .indexOf("window.Vaadin.Flow.jsDefinitionNames[\""), + "and it should come before the name it holds: " + written); + assertTrue( + written.contains("window.Vaadin.Flow.jsDefinitionNames[\"" + + JsCall.functionId(COUNT_EXPRESSION, 1) + "\"] = \"" + + CounterJs.class.getName() + ".count/1\";"), + "the name of what was asked for should be in the file: " + + written); + assertTrue(written.contains(JsCall.functionId(GREETING_EXPRESSION, 1)), + "and what the file held should still be in it: " + written); + } + + @Test + void updateJsDefinitions_nothingMissing_leavesTheFileAlone() + throws IOException { + // The same file a build writes: everything it is asked for is in it, + // names and all, so there is nothing to add + File generated = new File( + FrontendUtils.getFrontendGeneratedFolder(frontendFolder), + FrontendUtils.JS_DEFINITIONS_FILE_NAME); + generated.getParentFile().mkdirs(); + String carried = TaskGenerateJsDefinitions + .renderFileContent(List.of(GreeterJs.class), true); + Files.writeString(generated.toPath(), carried); + + TaskGenerateJsDefinitions.updateJsDefinitions(options, + List.of(GreeterJs.class)); + + assertEquals(carried, Files.readString(generated.toPath()), + "a definition the file carries should not be written into it again"); + } + + @Test + void generatedFile_productionMode_namesNothingOfTheJava() { + String content = new TaskGenerateJsDefinitions( + options.withProductionMode(true)).getFileContent(); + + assertFalse(content.contains(GreeterJs.class.getName()), + "a production bundle should not tell a browser what declared the JavaScript: " + + content); + assertFalse(content.contains("showGreeting"), + "and not what the methods are called either: " + content); + } + + @Test + void definitionWithoutDeclaredJavaScript_isNotRegistered() + throws ExecutionFailedException { + task.execute(); + String content = task.getFileContent(); + + assertEquals(2, + content.split("window.Vaadin.Flow.jsDefinitions\\[\"", + -1).length - 1, + "only the two methods that declare JavaScript should be registered: " + + content); + } + + @Test + void updateJsDefinitions_fileNotWritable_answersWithWhatItDoesNotCarry() + throws ExecutionFailedException { + // A file that carries one of the two interfaces, and a folder nothing + // can be written into + task.execute(); + File generatedFolder = FrontendUtils + .getFrontendGeneratedFolder(frontendFolder); + assumeTrue(generatedFolder.setWritable(false), + "the folder has to be made read only for this"); + + try { + List> missing = TaskGenerateJsDefinitions + .updateJsDefinitions(options, + List.of(GreeterJs.class, CounterJs.class)); + + assertEquals(List.of(CounterJs.class), missing, + "the interface the file carries is not missing because the write failed"); + } finally { + generatedFolder.setWritable(true); + } + } + + @Test + void findMissingFromGeneratedFile_answersForWhatTheFileCarries() + throws ExecutionFailedException, IOException { + task.execute(); + File generated = new File( + FrontendUtils.getFrontendGeneratedFolder(frontendFolder), + FrontendUtils.JS_DEFINITIONS_FILE_NAME); + String carried = Files.readString(generated.toPath()); + + assertTrue( + TaskGenerateJsDefinitions.findMissingFromGeneratedFile(options, + List.of(GreeterJs.class)).isEmpty(), + "the file was written from this interface"); + + // The JavaScript it declared before it was shortened: the browser + // would keep running the extra statement + Files.writeString(generated.toPath(), + carried.replace("window.alert({ text: $0, kind: 'greeting' })", + "window.alert($0)")); + assertEquals(List.of(GreeterJs.class), + TaskGenerateJsDefinitions.findMissingFromGeneratedFile(options, + List.of(GreeterJs.class)), + "another version of the declarations is not the declarations"); + + Files.delete(generated.toPath()); + assertEquals(List.of(GreeterJs.class), + TaskGenerateJsDefinitions.findMissingFromGeneratedFile(options, + List.of(GreeterJs.class)), + "no file carries nothing"); + } + + @Test + void updateJsDefinitions_writesWhatIsAskedForBesideWhatTheFileHolds() + throws ExecutionFailedException, IOException { + // A file written before the other interface was annotated: nothing has + // scanned for it, and what the file holds has to stay in it + task.execute(); + File generated = new File( + FrontendUtils.getFrontendGeneratedFolder(frontendFolder), + FrontendUtils.JS_DEFINITIONS_FILE_NAME); + + List> missing = TaskGenerateJsDefinitions + .updateJsDefinitions(options, List.of(CounterJs.class)); + + assertTrue(missing.isEmpty()); + String written = Files.readString(generated.toPath()); + assertTrue(written.contains(JsCall.functionId(COUNT_EXPRESSION, 1)), + "the interface that was asked for should be in the file: " + + written); + assertTrue(written.contains(JsCall.functionId(GREETING_EXPRESSION, 1)), + "what the file held should still be in it: " + written); + assertTrue( + written.indexOf("import.meta.hot") > written + .indexOf(JsCall.functionId(COUNT_EXPRESSION, 1)), + "and what was added should be part of the module: " + written); + } + + @Test + void updateJsDefinitions_fileWrittenByAnotherVersion_keepsWhatItHolds() + throws IOException { + // What a file written by another version of this class looks like: + // functions a browser has, and nothing this one recognizes to write + // around + File generated = new File( + FrontendUtils.getFrontendGeneratedFolder(frontendFolder), + FrontendUtils.JS_DEFINITIONS_FILE_NAME); + generated.getParentFile().mkdirs(); + Files.writeString(generated.toPath(), + "window.Vaadin.Flow.jsDefinitions = {\n \"fromsomewhereelse\": async function () {}\n};\n"); + + List> missing = TaskGenerateJsDefinitions + .updateJsDefinitions(options, List.of(CounterJs.class)); + + assertTrue(missing.isEmpty()); + String written = Files.readString(generated.toPath()); + assertTrue(written.contains("fromsomewhereelse"), + "a function a browser has should not be taken out of the file: " + + written); + assertTrue(written.contains(JsCall.functionId(COUNT_EXPRESSION, 1)), + "and the one that was asked for should be in it: " + written); + } + + @Test + void updateJsDefinitions_oneMethodEdited_writesOnlyThatFunction() + throws ExecutionFailedException, IOException { + // An interface of two methods, of which one declares something else + // than what the file was written with + task.execute(); + File generated = new File( + FrontendUtils.getFrontendGeneratedFolder(frontendFolder), + FrontendUtils.JS_DEFINITIONS_FILE_NAME); + String unchanged = "window.Vaadin.Flow.jsDefinitions[\"" + + JsCall.functionId("window.alert('Hello')", 0) + "\"]"; + Files.writeString(generated.toPath(), + Files.readString(generated.toPath()).replace( + GREETING_EXPRESSION, "window.alert('what it was')")); + + TaskGenerateJsDefinitions.updateJsDefinitions(options, + List.of(GreeterJs.class)); + + String written = Files.readString(generated.toPath()); + assertEquals(1, countOf(written, unchanged), + "the method that was not edited should not be written again: " + + written); + assertTrue(written.contains(GREETING_EXPRESSION), + "and the edited one should be in the file: " + written); + } + + private static int countOf(String content, String value) { + return content.split(Pattern.quote(value), -1).length - 1; + } + + @Test + void acceptsItsOwnUpdate() throws ExecutionFailedException { + task.execute(); + String content = task.getFileContent(); + + assertTrue(content.contains("import.meta.hot.accept()"), + "the file should accept its own update, or writing it again while the application runs is ignored by the browser instead of replacing the module: " + + content); + } + + @Test + void getFileContent_noClassFinder_saysWhatItIsFor() { + // The options a caller that writes the file again while the + // application runs builds: it knows the definitions, so it has nothing + // to scan with, and going through the generating side is a mistake + // that should say so + Options withoutAClassFinder = new Options(Mockito.mock(Lookup.class), + null, null).withFrontendDirectory(frontendFolder); + + assertTrue( + assertThrows(NullPointerException.class, + () -> new TaskGenerateJsDefinitions(withoutAClassFinder) + .getFileContent()) + .getMessage().contains("scan")); + } + + @Test + void writesTheFileTheBootstrapImports() throws ExecutionFailedException { + task.execute(); + + assertTrue( + new File(new File(frontendFolder, GENERATED), + JS_DEFINITIONS_FILE_NAME).exists(), + "the generated file should be where the bootstrap imports it from"); + } +} diff --git a/flow-client/src/main/frontend/internal/client/communication/MessageHandler.ts b/flow-client/src/main/frontend/internal/client/communication/MessageHandler.ts index 242073b9a91..b7af39f30b5 100644 --- a/flow-client/src/main/frontend/internal/client/communication/MessageHandler.ts +++ b/flow-client/src/main/frontend/internal/client/communication/MessageHandler.ts @@ -25,6 +25,7 @@ // EagerDependencyTracker, and the helpers above; everything else is a // Registry contract. +import type { ConstantPool } from '../flow/ConstantPool'; import type { StateNode } from '../flow/StateNode'; import type { Registry } from '../Registry'; import type { Command } from '../Command'; @@ -181,6 +182,15 @@ export class MessageHandler { const serverId = getServerId(valueMap); const hasResynchronize = isResynchronize(valueMap); + // Before anything decides what to do with the message, since what an + // invocation of it runs is a constant of it, and that decides whether a + // forced reload is what arrived. A message read more than once - queued + // and handled later, or re-sent by the server - carries the constants it + // carried before, which the pool takes as the values it already holds. + if ('constants' in valueMap) { + this.#registry.getConstantPool().importFromJson(valueMap.constants as Record); + } + if ( !hasResynchronize && this.#registry.getMessageSender().getResynchronizationState() === ResynchronizationState.WAITING_FOR_RESPONSE @@ -188,7 +198,7 @@ export class MessageHandler { if (UIDL_KEY_EXECUTE in valueMap) { const commands = valueMap[UIDL_KEY_EXECUTE] as unknown[][]; for (const command of commands) { - if (command.length > 0 && command[0] === 'window.location.reload();') { + if (resolveWhatRuns(command, this.#registry.getConstantPool()) === 'window.location.reload();') { Console.warn('Executing forced page reload while a resync request is ongoing.'); window.location.reload(); return; @@ -338,9 +348,6 @@ export class MessageHandler { } try { const processUidlStart = performance.now(); - if ('constants' in valueMap) { - this.#registry.getConstantPool().importFromJson(valueMap.constants as Record); - } if ('changes' in valueMap) { this.#processChanges(valueMap); } @@ -657,6 +664,22 @@ export class MessageHandler { * @param jsonText - The JSON to parse * @returns A ValueMap created from the JSON */ +/** + * Resolves what an invocation runs, which the invocation names rather than + * carries. + * + * @param invocation - the invocation, whose last element is the name + * @param constantPool - the constants the client has been sent + * @returns what to run, or `null` when nothing is named + */ +export function resolveWhatRuns(invocation: unknown[], constantPool: ConstantPool): unknown { + const name = invocation[invocation.length - 1]; + if (typeof name !== 'string') { + return null; + } + return constantPool.get(name); +} + export function parseJson(jsonText: string | null): ValueMap | null { if (jsonText === null) { return null; diff --git a/flow-client/src/main/frontend/internal/client/flow/ConstantPool.ts b/flow-client/src/main/frontend/internal/client/flow/ConstantPool.ts index 5bd3917fdc7..97238921128 100644 --- a/flow-client/src/main/frontend/internal/client/flow/ConstantPool.ts +++ b/flow-client/src/main/frontend/internal/client/flow/ConstantPool.ts @@ -26,14 +26,27 @@ export class ConstantPool { /** * Imports new constants into this pool. * + * A key is a hash of the value it names, so a key that is already here + * names what is already here: the server sends a constant once, but the + * message carrying it can reach the client more than once - it is re-sent, + * or it is read once as it arrives and again when it is handled - and every + * one of those carries the same value. What is refused is a key that names + * something else. + * * @param json - a JSON object mapping constant keys to constant values, not * `null` */ importFromJson(json: Record): void { for (const key of Object.keys(json)) { - assert(!this.#constants.has(key), 'ConstantPool already contains a value for the imported key'); const value = json[key]; assert(value !== null && value !== undefined, 'ConstantPool constant value must not be null'); + if (this.#constants.has(key)) { + assert( + JSON.stringify(this.#constants.get(key)) === JSON.stringify(value), + 'ConstantPool already contains another value for the imported key' + ); + continue; + } this.#constants.set(key, value); } } diff --git a/flow-client/src/main/frontend/internal/client/flow/ExecuteJavaScriptProcessor.ts b/flow-client/src/main/frontend/internal/client/flow/ExecuteJavaScriptProcessor.ts index f46b8373d09..47e48fdccf6 100644 --- a/flow-client/src/main/frontend/internal/client/flow/ExecuteJavaScriptProcessor.ts +++ b/flow-client/src/main/frontend/internal/client/flow/ExecuteJavaScriptProcessor.ts @@ -48,6 +48,7 @@ import { Reactive } from './reactive/Reactive'; import type { StateNode } from './StateNode'; import { UIState } from '../UILifecycle'; import { Console } from '../Console'; +import { JsonConstants } from '../../flow/shared/JsonConstants'; // NodeFeatures.NodeFeatures.ELEMENT_DATA / NodeProperties @@ -66,6 +67,69 @@ interface ContextCallbacks { disposeInitializer: (node: StateNode, id: number) => void; } +type JsDefinitionFunction = (this: unknown, ...args: unknown[]) => unknown; + +/** + * What an invocation of declared JavaScript names instead of an expression: + * the function of the bundle to run, identified by a hash of the JavaScript it + * runs. An invocation that runs an expression names the expression itself, a + * string, so the two are told apart by what the constant is rather than by + * what it says. + */ +type JsFunctionConstant = Record; + +type ReturnChannel = (value: unknown) => void; + +/** + * Reports the given message through the error channel of an invocation, which + * is its last parameter when it was subscribed to, so that the pending result + * of the call is completed rather than left hanging on the server. + */ +function reportThroughChannel(parameters: unknown[], message: string): void { + const lastParameter = parameters[parameters.length - 1]; + if (typeof lastParameter === 'function') { + (lastParameter as ReturnChannel)(message); + } +} + +/** + * What the generated bundle registers on the page: a function per declared + * expression, and, outside production, what a developer wrote for each of + * them. + */ +function getDeclaredJavaScript(): { + jsDefinitions?: Record; + jsDefinitionNames?: Record; +} { + return ( + ( + window as unknown as { + Vaadin?: { + Flow?: { jsDefinitions?: Record; jsDefinitionNames?: Record }; + }; + } + ).Vaadin?.Flow ?? {} + ); +} + +/** + * Looks up the function that the build generated for declared JavaScript. The + * registry is populated by the generated bundle, so the function is ordinary + * bundled code and nothing has to be compiled from a string here. + */ +function findDeclaredFunction(functionId: string): JsDefinitionFunction | undefined { + return getDeclaredJavaScript().jsDefinitions?.[functionId]; +} + +/** + * What to call a function in a message: what a developer wrote, which a + * development bundle registers next to the function itself, and the identifier + * of the function when it does not, as in production. + */ +function getNameOf(functionId: string): string { + return getDeclaredJavaScript().jsDefinitionNames?.[functionId] ?? functionId; +} + /** * Processes the result of `Page.executeJs` on the client. `Page` is a * flow-server class, outside this port, so the reference stays a code span. @@ -95,7 +159,7 @@ export class ExecuteJavaScriptProcessor { #handleInvocation(invocation: unknown[]): void { const tree = this.#registry.getStateTree(); - // Last item is the script, the rest are parameters. + // Last item names what to run in the constant pool, the rest are parameters. const parameterCount = invocation.length - 1; const parameterNamesAndCode: string[] = []; @@ -124,7 +188,30 @@ export class ExecuteJavaScriptProcessor { } } - parameterNamesAndCode.push(invocation[invocation.length - 1] as string); + // What to run is a constant of the message, the same way for an + // expression and for a call of declared JavaScript, so an expression the + // server runs again costs a reference rather than its own text. + const whatToRun = this.#registry + .getConstantPool() + .get(invocation[invocation.length - 1] as string); + if (whatToRun === null) { + Console.error( + `No constant for the invocation ${JSON.stringify(invocation)}. Reload the page to pick up the current state.` + ); + return; + } + + if (typeof whatToRun === 'object') { + // A call of declared JavaScript: the bundle has the function, the + // server sent only which one to run. The node parameters are for the + // context object an expression runs against, whose `getNode` maps an + // element back to its state node; a declared function runs against the + // element itself and has no context, so there is nothing that could ask. + this.invokeFromBundle(whatToRun[JsonConstants.UIDL_KEY_JS_FUNCTION], parameters); + return; + } + + parameterNamesAndCode.push(whatToRun); this.invoke(parameterNamesAndCode, parameters, nodeParameters); } @@ -195,6 +282,68 @@ export class ExecuteJavaScriptProcessor { }); invokeJavaScript(parameterNamesAndCode, parameters, context, configuration.isProductionMode()); } + + /** + * Executes a call made through a JavaScript definition: looks the function up + * in the registry that the generated bundle populates and applies it to the + * element, with the arguments of the call. Nothing is compiled from a string, + * which is what makes this path work under a content security policy that + * does not allow `unsafe-eval`. + * + * Protected instead of private for testing purposes, as `invoke` is. + * + * @param functionId - the identifier of the function to run + * @param parameters - the decoded parameters: the arguments of the call, the + * element to apply the function to, and the return value channels + * when the call is subscribed to + */ + protected invokeFromBundle(functionId: string, parameters: unknown[]): void { + const name = getNameOf(functionId); + const fn = findDeclaredFunction(functionId); + if (fn === undefined) { + const message = `No JavaScript in the bundle for ${name}. The JavaScript definition is annotated with @JsDefinition, but the build did not collect it.`; + Console.error(message); + // The server appends the two channels after everything else, or neither + // of them, so the error channel is the last parameter. Report through it + // when there is one, or the pending result of the call is never + // completed on the server. + reportThroughChannel(parameters, message); + return; + } + + // The function takes the arguments of the call, so what follows them is + // the element to apply it to, and then the two return value channels when + // the call is subscribed to. Nothing else may be in there, so a count that + // does not add up means the invocation was not built for this function, + // and reading the element out of it by index would bind an argument as + // `this`. Say so instead of running the call. + const argumentCount = fn.length; + const afterTheArguments = parameters.length - argumentCount; + if (afterTheArguments !== 1 && afterTheArguments !== 3) { + const message = `Expected ${argumentCount} arguments and the element for ${name} but the invocation carries ${parameters.length} parameters. Reload the page to pick up the current signature.`; + Console.error(message); + reportThroughChannel(parameters, message); + return; + } + + const returns = afterTheArguments === 3; + const onSuccess = returns ? (parameters[argumentCount + 1] as ReturnChannel) : undefined; + const onError = returns ? (parameters[argumentCount + 2] as ReturnChannel) : undefined; + + // The element the definition was obtained from is the parameter after the + // arguments, and it is what the function runs against. + const thisArg = parameters[argumentCount]; + try { + const result = fn.apply(thisArg, parameters.slice(0, argumentCount)); + if (onSuccess !== undefined) { + Promise.resolve(result).then(onSuccess, (error: unknown) => onError?.(`${error}`)); + } + } catch (exception) { + Console.reportStacktrace(exception); + Console.error(`Exception is thrown while running ${name}. Stacktrace will be dumped separately.`); + onError?.(`${exception}`); + } + } } /** diff --git a/flow-client/src/main/frontend/internal/flow/shared/JsonConstants.ts b/flow-client/src/main/frontend/internal/flow/shared/JsonConstants.ts index e58908cb9e4..6c90ba46822 100644 --- a/flow-client/src/main/frontend/internal/flow/shared/JsonConstants.ts +++ b/flow-client/src/main/frontend/internal/flow/shared/JsonConstants.ts @@ -148,6 +148,13 @@ export const JsonConstants = { */ RPC_EVENT_DATA: 'data', + /** + * Key of the function to run in the constant that an invocation of declared + * JavaScript names, in place of the expression that an invocation of an + * expression names. + */ + UIDL_KEY_JS_FUNCTION: 'f', + /** * Key used to hold the feature id when synchronizing node values. */ diff --git a/flow-client/src/test/frontend/internal/client/communication/MessageHandlerTests.ts b/flow-client/src/test/frontend/internal/client/communication/MessageHandlerTests.ts index 77b4de1330d..0f739fe9b63 100644 --- a/flow-client/src/test/frontend/internal/client/communication/MessageHandlerTests.ts +++ b/flow-client/src/test/frontend/internal/client/communication/MessageHandlerTests.ts @@ -5,7 +5,13 @@ import type { ResourceLoadListener } from '../../../../../main/frontend/internal/client/ResourceRegistry'; import { expect } from '@open-wc/testing'; -import { MessageHandler, parseJson } from '../../../../../main/frontend/internal/client/communication/MessageHandler'; +import { + MessageHandler, + parseJson, + resolveWhatRuns +} from '../../../../../main/frontend/internal/client/communication/MessageHandler'; +import { ConstantPool } from '../../../../../main/frontend/internal/client/flow/ConstantPool'; +import { ResynchronizationState } from '../../../../../main/frontend/internal/client/communication/MessageSender'; import { DependencyLoader } from '../../../../../main/frontend/internal/client/DependencyLoader'; import { ResourceLoader } from '../../../../../main/frontend/internal/client/ResourceLoader'; import { runWhenEagerDependenciesLoaded } from '../../../../../main/frontend/internal/client/EagerDependencyTracker'; @@ -240,7 +246,10 @@ describe('MessageHandler', () => { // syncId 5 while expecting 1 -> queued, not applied. handler.handleMessage({ syncId: 5, constants: { skipped: 1 } }); - expect(registry.log.constants).to.deep.equal([{ first: 1 }]); // second not imported + // The constants of a message go into the pool as it arrives, since what + // an invocation of it runs is read out of there, but nothing of the + // message itself is applied + expect(registry.log.constants).to.deep.equal([{ first: 1 }, { skipped: 1 }]); expect(handler.getLastSeenServerSyncId()).to.equal(0); }); @@ -257,7 +266,9 @@ describe('MessageHandler', () => { // is ended. registry.startRequest(); handler.handleMessage({ syncId: 0, constants: { stale: 1 } }); - expect(registry.log.constants).to.deep.equal([]); // never applied any constants + // Nothing of the message is applied; its constants are in the pool the + // way those of any message that arrives are + expect(registry.log.constants).to.deep.equal([{ stale: 1 }]); expect(registry.log.endRequests).to.equal(endRequestsBefore + 1); }); @@ -516,6 +527,55 @@ describe('MessageHandler', () => { expect(profiling[0]).to.be.at.least(0); }); + it('reads what an invocation runs out of the pool', () => { + // Which decides whether a forced reload during a resynchronization is + // seen; the constants of a message go into the pool as it arrives, so + // the one it carries is there along with the ones before it. + const pool = new ConstantPool(); + pool.importFromJson({ earlier: 'window.location.reload();' }); + + expect(resolveWhatRuns([{}, 'earlier'], pool)).to.equal('window.location.reload();'); + expect(resolveWhatRuns([{}, 'neither'], pool)).to.be.null; + expect(resolveWhatRuns([], pool)).to.be.null; + }); + + it('takes the constants of a message the server re-sends', () => { + // The message is ignored as already seen, but its constants are read + // before that, as they are for any message that arrives. They name + // what the pool holds, which is what the pool makes of a key it has. + const registry = makeRegistry(); + const handler = new MessageHandler(registry.registry); + handler.handleMessage({ syncId: 0, constants: { c: 'window.alert($0)' } }); + + registry.startRequest(); + handler.handleMessage({ syncId: 0, constants: { c: 'window.alert($0)' } }); + + expect(registry.log.constants).to.deep.equal([{ c: 'window.alert($0)' }, { c: 'window.alert($0)' }]); + }); + + it('takes the constants of a message in before deciding what to do with it', () => { + // What an invocation runs is read out of the pool, and a message that + // arrives while a resynchronization is ongoing is only queued, so its + // constants have to be in the pool by then. + const pool = new ConstantPool(); + const registry = testRegistry({ + MessageSender: { + getResynchronizationState: () => ResynchronizationState.WAITING_FOR_RESPONSE, + clearResynchronizationState: () => {}, + setClientToServerMessageId: () => {} + }, + ConstantPool: pool + }); + + new TestMessageHandler(registry).callHandleJSON({ + syncId: 3, + constants: { c: 'window.alert($0)' }, + execute: [['c']] + }); + + expect(pool.get('c')).to.equal('window.alert($0)'); + }); + it('keeps processing a message whose stylesheetRemovals is null', () => { const registry = makeRegistry(); const handler = new MessageHandler(registry.registry); diff --git a/flow-client/src/test/frontend/internal/client/flow/ConstantPoolTests.ts b/flow-client/src/test/frontend/internal/client/flow/ConstantPoolTests.ts index 7765e4e98fe..4e63efdbf3e 100644 --- a/flow-client/src/test/frontend/internal/client/flow/ConstantPoolTests.ts +++ b/flow-client/src/test/frontend/internal/client/flow/ConstantPoolTests.ts @@ -26,6 +26,19 @@ describe('ConstantPool', () => { expect(pool.get('missing')).to.equal(null); }); + it('takes a key it already holds, and refuses another value under it', () => { + // The message a constant arrives in can reach the client more than once, + // and a key is a hash of the value it names, so the same key is the same + // value. Anything else is a key that means two things. + const pool = new ConstantPool(); + pool.importFromJson({ a: { text: 'value-a' } }); + + pool.importFromJson({ a: { text: 'value-a' } }); + expect(pool.get>('a')).to.deep.equal({ text: 'value-a' }); + + expect(() => pool.importFromJson({ a: { text: 'something else' } })).to.throw(); + }); + it('accumulates constants across imports', () => { const pool = new ConstantPool(); pool.importFromJson({ a: '1' }); diff --git a/flow-client/src/test/frontend/internal/client/flow/ExecuteJavaScriptProcessorTests.ts b/flow-client/src/test/frontend/internal/client/flow/ExecuteJavaScriptProcessorTests.ts index c6225785d41..0a1c5e476d1 100644 --- a/flow-client/src/test/frontend/internal/client/flow/ExecuteJavaScriptProcessorTests.ts +++ b/flow-client/src/test/frontend/internal/client/flow/ExecuteJavaScriptProcessorTests.ts @@ -7,9 +7,24 @@ import { StateTree } from '../../../../../main/frontend/internal/client/flow/Sta import { Reactive } from '../../../../../main/frontend/internal/client/flow/reactive/Reactive'; import { NodeFeatures } from '../../../../../main/frontend/internal/flow/internal/nodefeature/NodeFeatures'; import { NodeProperties } from '../../../../../main/frontend/internal/flow/internal/nodefeature/NodeProperties'; +import { ConstantPool } from '../../../../../main/frontend/internal/client/flow/ConstantPool'; import { type RecordedCalls, recordingRegistry } from './stateTreeTestRegistry'; import { TestRegistry, testRegistry } from '../testRegistry'; +// What to run is a constant of the message rather than part of the invocation, +// so a case writes it at the end of an invocation, as the message reads, and +// this puts it in the pool and names it the way the server does. +let nextConstant = 0; +function execute(processor: ExecuteJavaScriptProcessor, registry: TestRegistry, invocations: unknown[][]): void { + const named = invocations.map((invocation) => { + nextConstant += 1; + const key = `constant-${nextConstant}`; + registry.getConstantPool().importFromJson({ [key]: invocation[invocation.length - 1] }); + return [...invocation.slice(0, -1), key]; + }); + processor.execute(named); +} + // Ported from com.vaadin.client.flow.ExecuteJavaScriptProcessorTest and // com.vaadin.client.GwtExecuteJavaScriptElementUtilsTest (the return-channel // case, which drives this class). The cases that exercise the expression @@ -54,6 +69,7 @@ class TestJsProcessor extends ExecuteJavaScriptProcessor { // Registry does. function treeRegistry(services: { existingElementMap?: boolean } = {}): TestRegistry { const registry = new TestRegistry(); + registry.register('ConstantPool', new ConstantPool()); registry.register('StateTree', new StateTree(registry)); if (services.existingElementMap === true) { registry.register('ExistingElementMap', new ExistingElementMap()); @@ -69,12 +85,145 @@ function registeredNode(registry: TestRegistry, id: number): StateNode { } describe('ExecuteJavaScriptProcessor', () => { + describe('JavaScript definition calls', () => { + // What the server sends: an object naming a function of the bundle, which + // is identified by a hash of the JavaScript it runs + const GREETING = 'a'.repeat(64); + const VALUE = 'b'.repeat(64); + const greeting = { f: GREETING }; + const value = { f: VALUE }; + + type DefinitionFunction = (this: unknown, ...args: unknown[]) => unknown; + + type DefinitionWindow = Window & { + Vaadin?: { + Flow?: { jsDefinitions?: Record; jsDefinitionNames?: Record }; + }; + }; + + // Registers a function the way the generated bundle does, with the name a + // message calls it by, which a development bundle registers with it. + function registerDefinition(functionId: string, fn: DefinitionFunction, name?: string): void { + const vaadin = (window as DefinitionWindow).Vaadin ?? {}; + (window as DefinitionWindow).Vaadin = vaadin; + vaadin.Flow = vaadin.Flow ?? {}; + vaadin.Flow.jsDefinitions = vaadin.Flow.jsDefinitions ?? {}; + vaadin.Flow.jsDefinitions[functionId] = fn; + if (name !== undefined) { + vaadin.Flow.jsDefinitionNames = vaadin.Flow.jsDefinitionNames ?? {}; + vaadin.Flow.jsDefinitionNames[functionId] = name; + } + } + + function fixture(): { processor: ExecuteJavaScriptProcessor; registry: TestRegistry } { + const registry = testRegistry({ + ConstantPool: new ConstantPool(), + StateTree: { getNode: () => null }, + ApplicationConfiguration: { getApplicationId: () => 'ROOT-1', isProductionMode: () => false } + }); + return { processor: new ExecuteJavaScriptProcessor(registry), registry }; + } + + function run(invocation: unknown[]): void { + const { processor, registry } = fixture(); + execute(processor, registry, [invocation]); + } + + afterEach(() => { + const flow = (window as DefinitionWindow).Vaadin?.Flow; + for (const functionId of [GREETING, VALUE]) { + delete flow?.jsDefinitions?.[functionId]; + delete flow?.jsDefinitionNames?.[functionId]; + } + }); + + it('runs the function from the bundle against the element', () => { + const calls: Array<{ thisArg: unknown; args: unknown[] }> = []; + registerDefinition(GREETING, function (this: unknown, greeting: unknown) { + calls.push({ thisArg: this, args: [greeting] }); + }); + const element = { tagName: 'div' }; + + run(['Hello', element, greeting]); + + expect(calls).to.have.lengthOf(1); + expect(calls[0].thisArg).to.equal(element); + expect(calls[0].args).to.eql(['Hello']); + }); + + it('passes the return value to the success channel', async () => { + registerDefinition(VALUE, () => 'answer'); + const resolved: unknown[] = []; + const element = { tagName: 'div' }; + + run([element, (returned: unknown) => resolved.push(returned), () => {}, value]); + // Settled in microtasks: a macrotask wait would also pick up the + // asynchronous rethrow that the expression cases leave behind. + await Promise.resolve(); + await Promise.resolve(); + + expect(resolved).to.eql(['answer']); + }); + + it('does not run a call whose parameters do not match the function', () => { + let calls = 0; + registerDefinition(GREETING, (_greeting: unknown) => { + calls += 1; + }); + + // One argument the function takes, but no element to apply it to: the + // invocation and the bundle disagree about the signature, which is the + // same disagreement as an invocation that carries one parameter too + // many. + run(['Hello', greeting]); + + expect(calls).to.equal(0); + }); + + it('reports a mismatch to the error channel, naming the function as it was written', () => { + let calls = 0; + registerDefinition( + VALUE, + () => { + calls += 1; + return 'answer'; + }, + 'com.acme.GreeterJs.readValue/0' + ); + const errors: unknown[] = []; + const element = { tagName: 'div' }; + + // Subscribed to, but one channel short of what the server sends. + run([element, (error: unknown) => errors.push(error), value]); + + expect(calls).to.equal(0); + // Reported rather than left hanging: the pending result on the server + // would otherwise never complete. + expect(errors).to.have.lengthOf(1); + // A development bundle registers what a developer wrote next to the + // function, so a message says more than a hash does + expect(String(errors[0])).to.contain('com.acme.GreeterJs.readValue/0'); + }); + + it('reports a function that is not in the bundle to the error channel', () => { + const errors: unknown[] = []; + const element = { tagName: 'div' }; + + run([element, () => {}, (error: unknown) => errors.push(error), value]); + + expect(errors).to.have.lengthOf(1); + // Nothing registered it, so the message has only the identifier + expect(String(errors[0])).to.contain(VALUE); + }); + }); + describe('execute', () => { it('passes the parameters and code of each invocation on', () => { // Ported from execute_parametersAndCodeAreValidAndNoNodeParameters. - const processor = new CollectingExecuteJavaScriptProcessor(treeRegistry()); + const registry = treeRegistry(); + const processor = new CollectingExecuteJavaScriptProcessor(registry); - processor.execute([['script1'], ['param1', 'param2', 'script2']]); + execute(processor, registry, [['script1'], ['param1', 'param2', 'script2']]); expect(processor.parameterNamesAndCodeList).to.have.length(2); expect(processor.parametersList).to.have.length(2); @@ -90,6 +239,29 @@ describe('ExecuteJavaScriptProcessor', () => { expect(processor.nodeParametersList[1].size).to.equal(0); }); + it('runs nothing for an invocation whose constant is not there', () => { + // A message that named a constant of one that never arrived, or arrived + // out of order: running the name as a script is the one thing that must + // not happen. + const registry = treeRegistry(); + const processor = new CollectingExecuteJavaScriptProcessor(registry); + + processor.execute([['neverimported']]); + + expect(processor.parameterNamesAndCodeList).to.have.length(0); + }); + + it('runs a string constant as an expression, whatever it looks like', () => { + // A function is named by an object, so an expression that happens to + // read like the identifier of one is still an expression + const registry = treeRegistry(); + const processor = new CollectingExecuteJavaScriptProcessor(registry); + + execute(processor, registry, [['a'.repeat(64)]]); + + expect(processor.parameterNamesAndCodeList).to.deep.equal([['a'.repeat(64)]]); + }); + it('passes a node parameter as the element it is bound to', () => { // Ported from execute_nodeParametersAreCorrectlyPassed. const registry = treeRegistry({ existingElementMap: true }); @@ -98,7 +270,7 @@ describe('ExecuteJavaScriptProcessor', () => { const element = document.createElement('div'); node.setDomNode(element); - processor.execute([[{ '@v-node': node.getId() }, '$0']]); + execute(processor, registry, [[{ '@v-node': node.getId() }, '$0']]); expect(processor.nodeParametersList).to.have.length(1); expect(processor.nodeParametersList[0].size).to.equal(1); @@ -116,7 +288,7 @@ describe('ExecuteJavaScriptProcessor', () => { .setValue({ [NodeProperties.TYPE]: NodeProperties.INJECT_BY_ID }); registry.getStateTree().registerNode(node); - processor.execute([[{ '@v-node': node.getId() }, '$0']]); + execute(processor, registry, [[{ '@v-node': node.getId() }, '$0']]); // The invocation has not been executed expect(processor.nodeParametersList).to.have.length(0); @@ -138,7 +310,7 @@ describe('ExecuteJavaScriptProcessor', () => { const node = registeredNode(registry, 31); processor.bound = false; - processor.execute([[{ '@v-node': node.getId() }, '$0']]); + execute(processor, registry, [[{ '@v-node': node.getId() }, '$0']]); expect(processor.nodeParametersList).to.have.length(0); @@ -159,7 +331,7 @@ describe('ExecuteJavaScriptProcessor', () => { const processor = new CollectingExecuteJavaScriptProcessor(registry); const node = registeredNode(registry, 12); - processor.execute([[{ '@v-node': node.getId() }, '$0']]); + execute(processor, registry, [[{ '@v-node': node.getId() }, '$0']]); // The invocation has been executed expect(processor.nodeParametersList).to.have.length(1); @@ -246,6 +418,7 @@ describe('ExecuteJavaScriptProcessor', () => { return { lifecycleStates, registry: testRegistry({ + ConstantPool: new ConstantPool(), StateTree: { getNode: () => null }, ApplicationConfiguration: { getApplicationId: () => 'ROOT-1', isProductionMode: () => false }, UILifecycle: { isTerminated: () => false, setState: (state: UIState) => lifecycleStates.push(state) } @@ -253,6 +426,12 @@ describe('ExecuteJavaScriptProcessor', () => { }; } + // The invocation of one expression that most cases here run + function runExpression(expression: string): void { + const registry = makeRegistry().registry; + execute(new ExecuteJavaScriptProcessor(registry), registry, [[expression]]); + } + afterEach(() => { delete (globalThis as Record).__ejpRan; delete (globalThis as Record).__ejpParam; @@ -264,6 +443,7 @@ describe('ExecuteJavaScriptProcessor', () => { const recorded: RecordedCalls = built.recorded; const tree = new StateTree(built.registry); const registry = testRegistry({ + ConstantPool: new ConstantPool(), StateTree: tree, ApplicationConfiguration: { getApplicationId: () => 'test', isProductionMode: () => false }, UILifecycle: { isTerminated: () => false, setState: () => {} } @@ -273,7 +453,7 @@ describe('ExecuteJavaScriptProcessor', () => { const expectedChannelId = 20; // The @v-return parameter decodes to a callback; the expression calls it. - new ExecuteJavaScriptProcessor(registry).execute([ + execute(new ExecuteJavaScriptProcessor(registry), registry, [ [{ '@v-return': [expectedNodeId, expectedChannelId] }, '$0(2)'] ]); @@ -284,27 +464,26 @@ describe('ExecuteJavaScriptProcessor', () => { it('runs an invocation expression', () => { // Beyond the Java suite. - new ExecuteJavaScriptProcessor(makeRegistry().registry).execute([['globalThis.__ejpRan = true;']]); + runExpression('globalThis.__ejpRan = true;'); expect((globalThis as Record).__ejpRan).to.be.true; }); it('binds invocation parameters to $0, $1, ...', () => { // Beyond the Java suite. - new ExecuteJavaScriptProcessor(makeRegistry().registry).execute([['hello', 'globalThis.__ejpParam = $0;']]); + const registry = makeRegistry().registry; + execute(new ExecuteJavaScriptProcessor(registry), registry, [['hello', 'globalThis.__ejpParam = $0;']]); expect((globalThis as Record).__ejpParam).to.equal('hello'); }); it('exposes the app id with the per-UI suffix stripped', () => { // Beyond the Java suite. - new ExecuteJavaScriptProcessor(makeRegistry().registry).execute([['globalThis.__ejpParam = this.$appId;']]); + runExpression('globalThis.__ejpParam = this.$appId;'); expect((globalThis as Record).__ejpParam).to.equal('ROOT'); }); it('exposes the registry on the context', () => { // Beyond the Java suite. - new ExecuteJavaScriptProcessor(makeRegistry().registry).execute([ - ['globalThis.__ejpParam = this.registry === undefined;'] - ]); + runExpression('globalThis.__ejpParam = this.registry === undefined;'); expect((globalThis as Record).__ejpParam).to.equal(false); }); @@ -312,23 +491,21 @@ describe('ExecuteJavaScriptProcessor', () => { // Beyond the Java suite. // getNode throws when the argument is not a state-node parameter; the // executed code sees that as a thrown ReferenceError. - new ExecuteJavaScriptProcessor(makeRegistry().registry).execute([ - ['try { this.attachExistingElement({}); } catch (e) { globalThis.__ejpParam = e.constructor.name; }'] - ]); + runExpression( + 'try { this.attachExistingElement({}); } catch (e) { globalThis.__ejpParam = e.constructor.name; }' + ); expect((globalThis as Record).__ejpParam).to.equal('ReferenceError'); }); it('catches exceptions thrown by the executed code', () => { // Beyond the Java suite. - expect(() => - new ExecuteJavaScriptProcessor(makeRegistry().registry).execute([['throw new Error("boom");']]) - ).to.not.throw(); + expect(() => runExpression('throw new Error("boom");')).to.not.throw(); }); it('exposes stopApplication on the context, terminating the UI lifecycle', () => { // Beyond the Java suite. const fixture = makeRegistry(); - new ExecuteJavaScriptProcessor(fixture.registry).execute([['this.stopApplication();']]); + execute(new ExecuteJavaScriptProcessor(fixture.registry), fixture.registry, [['this.stopApplication();']]); expect(fixture.lifecycleStates).to.deep.equal([UIState.TERMINATED]); }); }); diff --git a/flow-server/src/main/java/com/vaadin/flow/component/Focusable.java b/flow-server/src/main/java/com/vaadin/flow/component/Focusable.java index c6ac930cac3..29899f100a2 100644 --- a/flow-server/src/main/java/com/vaadin/flow/component/Focusable.java +++ b/flow-server/src/main/java/com/vaadin/flow/component/Focusable.java @@ -15,9 +15,14 @@ */ package com.vaadin.flow.component; +import java.io.Serializable; + +import org.jspecify.annotations.Nullable; import tools.jackson.databind.node.ObjectNode; import com.vaadin.flow.dom.Element; +import com.vaadin.flow.js.JsDefinition; +import com.vaadin.flow.js.JsExpression; /** * Represents a component that can gain and lose focus. @@ -134,34 +139,8 @@ default int getTabIndex() { * @since 25.0 */ default void focus(FocusOption... options) { - Element element = getElement(); - ObjectNode json = FocusOption.buildOptions(options); - - if (json == null) { - // No options, call focus() without arguments - element.executeJs(""" - setTimeout(() => { - try { - this._nextFocusIsFromClient = false; - this.focus(); - } finally { - this._nextFocusIsFromClient = true; - } - }, 0) - """); - } else { - // Call focus with options object passed as parameter - element.executeJs(""" - setTimeout(() => { - try { - this._nextFocusIsFromClient = false; - this.focus($0); - } finally { - this._nextFocusIsFromClient = true; - } - }, 0) - """, json); - } + getElement().executeJs(FocusJs.class) + .focus(FocusOption.buildOptions(options)); } // for binary compatibility with the previous Vaadin versions @@ -190,16 +169,7 @@ default void focus() { * at MDN */ default void blur() { - getElement().executeJs(""" - setTimeout(() => { - try { - this._nextBlurIsFromClient = false; - this.blur(); - } finally { - this._nextBlurIsFromClient = true; - } - }, 0) - """); + getElement().executeJs(FocusJs.class).blur(); } /** @@ -241,4 +211,52 @@ default ShortcutRegistration addFocusShortcut(Key key, () -> new Component[] { thisComponent.getUI().get() }, event -> this.focus(), key).withModifiers(keyModifiers); } + + /** + * The client-side operations behind {@link Focusable}, as a JavaScript + * definition for {@link Element#executeJs(Class)}. + *

+ * Focus and blur are marked as server-initiated for the client, so that the + * resulting event reports {@code isFromClient() == false}. A driver of the + * client side that implements this interface instead of running the scripts + * is responsible for the same. + */ + @JsDefinition + interface FocusJs extends Serializable { + + /** + * Focuses the element. + * + * @param options + * the options of the browser's focus function, + * or null for its defaults, which is what the + * browser makes of an empty set of options + */ + @JsExpression(""" + setTimeout(() => { + try { + this._nextFocusIsFromClient = false; + this.focus($0); + } finally { + this._nextFocusIsFromClient = true; + } + }, 0) + """) + void focus(@Nullable ObjectNode options); + + /** + * Removes focus from the element. + */ + @JsExpression(""" + setTimeout(() => { + try { + this._nextBlurIsFromClient = false; + this.blur(); + } finally { + this._nextBlurIsFromClient = true; + } + }, 0) + """) + void blur(); + } } diff --git a/flow-server/src/main/java/com/vaadin/flow/component/internal/UIInternals.java b/flow-server/src/main/java/com/vaadin/flow/component/internal/UIInternals.java index 9a60b473134..666ae0910b5 100644 --- a/flow-server/src/main/java/com/vaadin/flow/component/internal/UIInternals.java +++ b/flow-server/src/main/java/com/vaadin/flow/component/internal/UIInternals.java @@ -77,6 +77,7 @@ import com.vaadin.flow.internal.nodefeature.PushConfigurationMap; import com.vaadin.flow.internal.nodefeature.ReconnectDialogConfigurationMap; import com.vaadin.flow.internal.streams.ActiveTransfer; +import com.vaadin.flow.js.JsCall; import com.vaadin.flow.router.AfterNavigationListener; import com.vaadin.flow.router.BeforeEnterListener; import com.vaadin.flow.router.BeforeLeaveEvent.ContinueNavigationAction; @@ -128,6 +129,7 @@ public class UIInternals implements Serializable { public static class JavaScriptInvocation implements Serializable { private final String expression; private final List parameters = new ArrayList<>(); + private final @Nullable JsCall jsCall; /** * Creates a new invocation. @@ -139,6 +141,23 @@ public static class JavaScriptInvocation implements Serializable { * @since 25.0 */ public JavaScriptInvocation(String expression, Object... parameters) { + this((JsCall) null, expression, parameters); + } + + /** + * Creates a new invocation for the given call, whose expression and + * parameters the caller has already resolved. + * + * @param jsCall + * the call that this invocation performs, or + * null if the invocation is plain JavaScript + * @param expression + * the expression to invoke + * @param parameters + * a list of parameters to use when invoking the script + */ + public JavaScriptInvocation(@Nullable JsCall jsCall, String expression, + Object... parameters) { /* * To ensure attached elements are actually attached, the parameters * won't be serialized until the phase the UIDL message is created. @@ -152,6 +171,7 @@ public JavaScriptInvocation(String expression, Object... parameters) { this.expression = expression; Collections.addAll(this.parameters, parameters); + this.jsCall = jsCall; } /** @@ -171,6 +191,19 @@ public String getExpression() { public List getParameters() { return Collections.unmodifiableList(parameters); } + + /** + * Gets the call that this invocation performs, for a caller that acts + * on the invocation instead of running its JavaScript — the client, + * which looks up the generated function rather than compiling the + * expression, and a driver of the client side that recognizes the call. + * + * @return the call, or null if the invocation is plain + * JavaScript scheduled with an expression + */ + public @Nullable JsCall getJsCall() { + return jsCall; + } } /** diff --git a/flow-server/src/main/java/com/vaadin/flow/dom/Element.java b/flow-server/src/main/java/com/vaadin/flow/dom/Element.java index 4d67e408041..b955ac0ebc4 100644 --- a/flow-server/src/main/java/com/vaadin/flow/dom/Element.java +++ b/flow-server/src/main/java/com/vaadin/flow/dom/Element.java @@ -64,6 +64,10 @@ import com.vaadin.flow.internal.StateNode; import com.vaadin.flow.internal.nodefeature.SignalBindingFeature; import com.vaadin.flow.internal.nodefeature.VirtualChildrenList; +import com.vaadin.flow.js.JsCall; +import com.vaadin.flow.js.JsDefinition; +import com.vaadin.flow.js.JsDefinitionProxy; +import com.vaadin.flow.js.JsExpression; import com.vaadin.flow.server.AbstractStreamResource; import com.vaadin.flow.server.Command; import com.vaadin.flow.server.StreamResource; @@ -1822,6 +1826,11 @@ public T as(Class componentType) { *

* If the element is not attached or not visible, the function call will be * deferred until the element is attached and visible. + *

+ * The call is sent to the browser as an expression and compiled there, + * which a content security policy without unsafe-eval does not + * allow. {@link #executeJs(Class)} runs JavaScript that is declared in Java + * and collected into the bundle instead, and sends no expression. * * @param functionName * the name of the function to call, may contain dots to indicate @@ -1834,6 +1843,7 @@ public T as(Class componentType) { * null if not attached). * @return a pending result that can be used to get a return value from the * execution + * @see #executeJs(Class) * @since 25.0 */ public PendingJavaScriptResult callJsFunction(String functionName, @@ -1855,8 +1865,8 @@ public PendingJavaScriptResult callJsFunction(String functionName, System.arraycopy(arguments, 0, jsParameters, 1, arguments.length); } - return scheduleJavaScriptInvocation("return $0." + functionName + "(" - + paramPlaceholderString + ")", jsParameters); + return scheduleJavaScriptInvocation(null, "return $0." + functionName + + "(" + paramPlaceholderString + ")", jsParameters); } /** @@ -1918,6 +1928,11 @@ public PendingJavaScriptResult callJsFunction(String functionName, *

* If the element is not attached or not visible, the function call will be * deferred until the element is attached and visible. + *

+ * The expression is sent to the browser and compiled there, which a content + * security policy without unsafe-eval does not allow. + * {@link #executeJs(Class)} runs JavaScript that is declared in Java and + * collected into the bundle instead, and sends no expression. * * @param expression * the JavaScript expression to invoke @@ -1925,27 +1940,109 @@ public PendingJavaScriptResult callJsFunction(String functionName, * parameters to pass to the expression * @return a pending result that can be used to get a value returned from * the expression + * @see #executeJs(Class) * @since 25.0 */ public PendingJavaScriptResult executeJs(String expression, Object... parameters) { + return scheduleExecuteJs(expression, parameters); + } - // Add "this" as the last parameter - Object[] wrappedParameters; - if (parameters.length == 0) { - wrappedParameters = new Object[] { this }; - } else { - wrappedParameters = Arrays.copyOf(parameters, - parameters.length + 1); - wrappedParameters[parameters.length] = this; - } + /** + * Asynchronously runs the JavaScript that the given interface declares in + * the browser in the context of this element, through an implementation of + * the interface that this method answers with: calling a method of the + * implementation runs the JavaScript that the method declares, with the + * arguments of the call as its parameters. + *

+ * The interface is annotated with {@link JsDefinition}, and each of its + * methods declares the JavaScript it runs with {@link JsExpression}: + * + * 

+     * @JsDefinition
+     * public interface GreeterJs extends Serializable {
+     *     @JsExpression("window.alert($0)")
+     *     void showGreeting(String greeting);
+     * }
+     *
+     * element.executeJs(GreeterJs.class).showGreeting("Hello");
+     *
+ * + * The declared JavaScript runs the way an expression given to + * {@link #executeJs(String, Object...)} does: in an async + * JavaScript method, with this element available as this and + * the arguments of the call as $0, $1, and so on, + * after pending DOM updates, and deferred while the element is not attached + * or not visible. A method that returns {@link PendingJavaScriptResult} can + * be used to retrieve the return value the same way. + *

+ * What differs is that nothing about the JavaScript is decided at the call + * site: the build collects the declarations of every JavaScript definition + * into the bundle, and the client runs the collected function after looking + * it up by an identifier of the JavaScript itself. No expression is sent + * and none is compiled in the browser, so the call works under a content + * security policy without unsafe-eval, and what declared the + * JavaScript in Java is not sent to a production browser either. + *

+ * The scheduled invocation carries the call as a {@link JsCall}, so a + * driver of the client side that can not run JavaScript can recognize it, + * or run it on its own implementation of the same interface. + *

+ * Every method of the interface declares JavaScript and returns either + * void or {@link PendingJavaScriptResult}. One that is + * implemented in Java instead - a default or a + * static method - is not what such an interface is for, so the + * interface is refused rather than partly run in the browser. + *

+ * The interface is checked when the implementation is handed out, so one + * that can not work says so here rather than at the first call. + * + * @param + * the JavaScript definition type + * @param definitionType + * the JavaScript definition, not null + * @return an implementation of the interface, to call the declared + * JavaScript through, not null + * @throws IllegalArgumentException + * if the type is not an interface, is not annotated with + * {@link JsDefinition}, or has a method that can not be + * answered + */ + public T executeJs(Class definitionType) { + return JsDefinitionProxy.create(definitionType, this::scheduleJsCall); + } + + private PendingJavaScriptResult scheduleExecuteJs(String expression, + Object[] parameters) { // Wrap in a function that is applied with last parameter as "this" String wrappedExpression = "return (async function() { " + expression + "}).apply($" + parameters.length + ")"; - return scheduleJavaScriptInvocation(wrappedExpression, - wrappedParameters); + return scheduleJavaScriptInvocation(null, wrappedExpression, + withElementAsLastParameter(parameters)); + } + + /** + * Schedules a call made through a JavaScript definition. The parameters are + * the arguments of the call followed by this element, which the client + * applies the generated function to, so there is no expression to wrap: the + * function that the build generated is already the equivalent of the + * wrapping that {@link #scheduleExecuteJs(String, Object[])} does around an + * expression. + */ + private PendingJavaScriptResult scheduleJsCall(JsCall call) { + return scheduleJavaScriptInvocation(call, call.getExpression(), + withElementAsLastParameter(call.arguments().toArray())); + } + + private Object[] withElementAsLastParameter(Object[] parameters) { + if (parameters.length == 0) { + return new Object[] { this }; + } + Object[] withElement = Arrays.copyOf(parameters, parameters.length + 1); + withElement[parameters.length] = this; + return withElement; } /** @@ -2010,11 +2107,11 @@ public Registration addJsInitializer(String expression, } private PendingJavaScriptResult scheduleJavaScriptInvocation( - String expression, Object[] parameters) { + @Nullable JsCall jsCall, String expression, Object[] parameters) { StateNode node = getNode(); - JavaScriptInvocation invocation = new JavaScriptInvocation(expression, - parameters); + JavaScriptInvocation invocation = new JavaScriptInvocation(jsCall, + expression, parameters); PendingJavaScriptInvocation pending = new PendingJavaScriptInvocation( node, invocation); diff --git a/flow-server/src/main/java/com/vaadin/flow/internal/FrontendUtils.java b/flow-server/src/main/java/com/vaadin/flow/internal/FrontendUtils.java index 9d53deb65ab..f38042fdd5d 100644 --- a/flow-server/src/main/java/com/vaadin/flow/internal/FrontendUtils.java +++ b/flow-server/src/main/java/com/vaadin/flow/internal/FrontendUtils.java @@ -214,6 +214,13 @@ public class FrontendUtils { */ public static final String FEATURE_FLAGS_FILE_NAME = "vaadin-featureflags.js"; + /** + * File name of the generated file that registers the JavaScript of the + * {@code @JsDefinition} interfaces on the class path, so that the client + * can run a server-initiated call without compiling an expression. + */ + public static final String JS_DEFINITIONS_FILE_NAME = "vaadin-js-definitions.js"; + /** * File name of the index.html in client side. */ diff --git a/flow-server/src/main/java/com/vaadin/flow/internal/ReflectTools.java b/flow-server/src/main/java/com/vaadin/flow/internal/ReflectTools.java index e3c69ee4d4a..a80b5df17b4 100644 --- a/flow-server/src/main/java/com/vaadin/flow/internal/ReflectTools.java +++ b/flow-server/src/main/java/com/vaadin/flow/internal/ReflectTools.java @@ -166,6 +166,32 @@ public static Optional findDeclaredMethod(Class cls, return Optional.empty(); } + /** + * Locates the public methods of the given type that have the given name and + * take the given number of parameters, which is the lookup available to a + * caller that has the arguments of a call rather than the parameter types + * of the method - values, whose classes are not the declarations. + *

+ * More than one is returned when the type overloads the name with the same + * number of parameters, which such a caller cannot tell apart. + * + * @param cls + * the type to look the methods up in + * @param methodName + * the name of the methods + * @param parameterCount + * the number of parameters the methods take + * @return the methods with that name and that number of parameters, empty + * if the type has none + */ + public static List getMethodsWithParameterCount(Class cls, + String methodName, int parameterCount) { + return Stream.of(cls.getMethods()) + .filter(method -> method.getName().equals(methodName) + && method.getParameterCount() == parameterCount) + .toList(); + } + /** * Returns the value of the java field. *

diff --git a/flow-server/src/main/java/com/vaadin/flow/js/JsCall.java b/flow-server/src/main/java/com/vaadin/flow/js/JsCall.java new file mode 100644 index 00000000000..0da75e0f97e --- /dev/null +++ b/flow-server/src/main/java/com/vaadin/flow/js/JsCall.java @@ -0,0 +1,176 @@ +/* + * Copyright 2000-2026 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.flow.js; + +import java.io.Serializable; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Objects; + +import com.vaadin.flow.dom.Element; +import com.vaadin.flow.internal.ReflectTools; +import com.vaadin.flow.internal.StringUtil; + +/** + * A call made through {@link Element#executeJs(Class)}: which definition + * interface, which method of it, and the arguments that were passed. + *

+ * The call is what is scheduled, and what a driver of the client side that can + * not run JavaScript sees in the pending invocation queue. A browser is sent + * less than this: the identifier of the function to run and the arguments, + * never the JavaScript itself, which it looks up in the bundle, and never the + * interface or the method, which stay on the server. Such a driver can dispatch + * on the interface and the method, or hand the call to an implementation of the + * same interface with {@link #invokeOn(Object)} and let Java dispatch it: + * + * 

+ * if (call.definitionType() == FocusJs.class) {
+ *     call.invokeOn(new FocusSimulation(Element.get(pending.getOwner())));
+ * }
+ *
+ * + * @param definitionType + * the JavaScript definition the call was made on + * @param methodName + * the name of the called method + * @param arguments + * the arguments of the call, in declaration order, any of which may + * be null + */ +public record JsCall(Class definitionType, String methodName, + List arguments) implements Serializable { + + /** + * Creates a call of the given method of the given JavaScript definition. + * + * @param definitionType + * the JavaScript definition, not null + * @param methodName + * the name of the called method, not null + * @param arguments + * the arguments of the call, not null + */ + public JsCall { + Objects.requireNonNull(definitionType, + "Definition type cannot be null"); + Objects.requireNonNull(methodName, "Method name cannot be null"); + // Copied rather than List.copyOf, which rejects a null element: an + // argument may be null, and the client gets it as null + arguments = Collections.unmodifiableList(new ArrayList<>(arguments)); + } + + /** + * Gets the identifier of the function that runs the given JavaScript with + * the given number of arguments, which is the key the generated bundle + * registers that function under and the only thing the client is told about + * a call. + *

+ * A hash of the JavaScript, so that the name of the Java that declared it + * stays on the server. The number of arguments is hashed with it, since it + * is what the parameters of the generated function are made of, and two + * methods that declare the same JavaScript for a different number of + * arguments are two functions. + * + * @param expression + * the declared JavaScript, not null + * @param argumentCount + * the number of arguments + * @return the function identifier, not null + */ + public static String functionId(String expression, int argumentCount) { + return StringUtil.getHash(argumentCount + ":" + expression, + StandardCharsets.UTF_8); + } + + /** + * Gets the JavaScript that this call runs in a browser, as declared by + * {@link JsExpression} on the called method. + *

+ * The expression is not sent to the client, which runs the function that + * the build generated from the same declaration. It is available here for + * the server side, for instance for a test that asserts what a browser + * would run. + * + * @return the JavaScript expression, not null + */ + public String getExpression() { + JsExpression annotation = resolveMethod() + .getAnnotation(JsExpression.class); + if (annotation == null) { + throw new IllegalStateException( + "Method " + methodName + " of " + definitionType.getName() + + " is not annotated with @JsExpression"); + } + return annotation.value(); + } + + /** + * Runs this call on an implementation of the JavaScript definition, which + * is how a driver of the client side reproduces it without running the + * JavaScript. + * + * @param implementation + * an implementation of {@link #definitionType()}, not + * null + * @return the value returned by the implementation, or null + * for a void method + * @throws IllegalArgumentException + * if the implementation does not implement + * {@link #definitionType()} + */ + public Object invokeOn(Object implementation) { + if (!definitionType.isInstance(implementation)) { + throw new IllegalArgumentException( + implementation.getClass().getName() + " does not implement " + + definitionType.getName()); + } + try { + return resolveMethod().invoke(implementation, arguments.toArray()); + } catch (IllegalAccessException | InvocationTargetException e) { + Throwable cause = e instanceof InvocationTargetException + ? e.getCause() + : e; + // What the implementation threw is what the caller wants to see + if (cause instanceof RuntimeException runtimeException) { + throw runtimeException; + } + throw new IllegalStateException( + "Could not run " + methodName + " on " + implementation, + cause); + } + } + + /** + * Finds the called method by name and argument count. Overloads that differ + * only in parameter types are not distinguishable this way, which is a + * limitation of the prototype rather than of the idea. + */ + private Method resolveMethod() { + List candidates = ReflectTools.getMethodsWithParameterCount( + definitionType, methodName, arguments.size()); + if (candidates.size() != 1) { + throw new IllegalStateException("Expected exactly one method named " + + methodName + " with " + arguments.size() + + " parameters in " + definitionType.getName() + ", found " + + candidates.size()); + } + return candidates.get(0); + } +} diff --git a/flow-server/src/main/java/com/vaadin/flow/js/JsDefinition.java b/flow-server/src/main/java/com/vaadin/flow/js/JsDefinition.java new file mode 100644 index 00000000000..05fa79f926a --- /dev/null +++ b/flow-server/src/main/java/com/vaadin/flow/js/JsDefinition.java @@ -0,0 +1,44 @@ +/* + * Copyright 2000-2026 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.flow.js; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import com.vaadin.flow.dom.Element; + +/** + * Marks an interface whose methods declare the JavaScript they run with + * {@link JsExpression}, to be called through {@link Element#executeJs(Class)}. + *

+ * The annotation is what makes the interface findable during the build: every + * annotated interface is collected into the generated bundle as a function per + * method, so the JavaScript an application can invoke from the server is known + * before it runs and the client never has to build a function from a string. + * That is what keeps a server-initiated call compatible with a content security + * policy that does not allow unsafe-eval. + * + * @see JsExpression + * @see Element#executeJs(Class) + */ +@Documented +@Target(ElementType.TYPE) +@Retention(RetentionPolicy.RUNTIME) +public @interface JsDefinition { +} diff --git a/flow-server/src/main/java/com/vaadin/flow/js/JsDefinitionProxy.java b/flow-server/src/main/java/com/vaadin/flow/js/JsDefinitionProxy.java new file mode 100644 index 00000000000..23c3caa48a6 --- /dev/null +++ b/flow-server/src/main/java/com/vaadin/flow/js/JsDefinitionProxy.java @@ -0,0 +1,166 @@ +/* + * Copyright 2000-2026 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.flow.js; + +import java.io.Serializable; +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.lang.reflect.Proxy; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Objects; + +import com.vaadin.flow.component.page.PendingJavaScriptResult; +import com.vaadin.flow.dom.Element; +import com.vaadin.flow.function.SerializableFunction; + +/** + * Hands out implementations of {@link JsDefinition} interfaces, which turn a + * call of a declared method into a {@link JsCall} and give it to whoever runs + * it. + *

+ * The interface is checked here rather than when a method is called, so one + * that can not work says so when the implementation is handed out. + *

+ * For internal use only. Call {@link Element#executeJs(Class)}, which runs the + * call on the element the implementation was obtained from. + */ +public final class JsDefinitionProxy { + + private JsDefinitionProxy() { + // Only static members + } + + /** + * Creates an implementation of the given JavaScript definition, which hands + * every call of it to the given function and answers with what the function + * answers, for a method that declares a result. + * + * @param + * the JavaScript definition type + * @param definitionType + * the JavaScript definition, not null + * @param runner + * what runs a call of the interface, not null + * @return an implementation of the interface, not null + * @throws IllegalArgumentException + * if the type is not an interface, is not annotated with + * {@link JsDefinition}, or has a method that can not be + * answered + */ + @SuppressWarnings("unchecked") + public static T create(Class definitionType, + SerializableFunction runner) { + Objects.requireNonNull(definitionType, + "Definition type cannot be null"); + Objects.requireNonNull(runner, "Runner cannot be null"); + if (!definitionType.isInterface()) { + throw new IllegalArgumentException( + definitionType.getName() + " is not an interface"); + } + if (!definitionType.isAnnotationPresent(JsDefinition.class)) { + throw new IllegalArgumentException(definitionType.getName() + + " is not annotated with @JsDefinition, so the build does not" + + " collect its JavaScript into the bundle"); + } + checkMethods(definitionType); + return (T) Proxy.newProxyInstance(definitionType.getClassLoader(), + new Class[] { definitionType }, + new JsDefinitionHandler(runner, definitionType)); + } + + /** + * Checks the methods of a JavaScript definition: every one of them declares + * the JavaScript it runs, and returns either nothing or the pending result + * of running it. An interface that declares JavaScript declares nothing + * else, so a method that is implemented in Java - a default or a static one + * - is refused rather than left aside. + */ + private static void checkMethods(Class definitionType) { + List undeclared = new ArrayList<>(); + List unanswerable = new ArrayList<>(); + List inJava = new ArrayList<>(); + for (Method method : definitionType.getMethods()) { + if (method.isDefault() + || Modifier.isStatic(method.getModifiers())) { + inJava.add(method.getName()); + continue; + } + if (!method.isAnnotationPresent(JsExpression.class)) { + // What it returns is beside the point until it declares + // something to return it from, and the first list that has + // anything in it is the one that is reported, so this changes + // what the lists hold rather than what is said + undeclared.add(method.getName()); + continue; + } + Class returnType = method.getReturnType(); + if (returnType != void.class && !returnType + .isAssignableFrom(PendingJavaScriptResult.class)) { + unanswerable.add(method.getName()); + } + } + if (!undeclared.isEmpty()) { + throw new IllegalArgumentException(definitionType.getName() + + " declares no JavaScript to run for " + + String.join(", ", undeclared) + + ". Annotate every method with @JsExpression"); + } + if (!unanswerable.isEmpty()) { + throw new IllegalArgumentException(definitionType.getName() + + " has " + String.join(", ", unanswerable) + + " returning something that can not be answered with." + + " A method returns void or PendingJavaScriptResult"); + } + if (!inJava.isEmpty()) { + throw new IllegalArgumentException(definitionType.getName() + + " has " + String.join(", ", inJava) + + " implemented in Java, which is not what an interface" + + " that declares JavaScript is for. Annotate every method" + + " with @JsExpression, and compose the calls in the class" + + " that makes them"); + } + } + + /** + * Turns a call of a JavaScript definition into a {@link JsCall} and runs it + * through the function it was created with. + */ + private record JsDefinitionHandler( + SerializableFunction runner, + Class definitionType) + implements + InvocationHandler, + Serializable { + + @Override + public Object invoke(Object proxy, Method method, Object[] args) + throws Throwable { + if (method.getDeclaringClass() == Object.class) { + return method.invoke(this, args); + } + boolean returnsResult = method.getReturnType() + .isAssignableFrom(PendingJavaScriptResult.class); + List arguments = args == null ? List.of() + : Arrays.asList(args); + PendingJavaScriptResult result = runner.apply( + new JsCall(definitionType, method.getName(), arguments)); + return returnsResult ? result : null; + } + } +} diff --git a/flow-server/src/main/java/com/vaadin/flow/js/JsExpression.java b/flow-server/src/main/java/com/vaadin/flow/js/JsExpression.java new file mode 100644 index 00000000000..a67a11ae599 --- /dev/null +++ b/flow-server/src/main/java/com/vaadin/flow/js/JsExpression.java @@ -0,0 +1,50 @@ +/* + * Copyright 2000-2026 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.flow.js; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import com.vaadin.flow.dom.Element; + +/** + * The JavaScript that a method of a JavaScript definition runs, as a constant + * expression. + *

+ * The annotated method is called through {@link Element#executeJs(Class)}. Its + * arguments are the parameters of the expression, referenced positionally as + * $0, $1, …, and the element the definition + * was obtained from is this — the same contract as + * {@link Element#executeJs(String, Object...)}, except that the expression is a + * constant of the interface instead of a string built at the call site. + * + * @see Element#executeJs(Class) + */ +@Documented +@Target(ElementType.METHOD) +@Retention(RetentionPolicy.RUNTIME) +public @interface JsExpression { + + /** + * The JavaScript expression to run. + * + * @return the expression + */ + String value(); +} diff --git a/flow-server/src/main/java/com/vaadin/flow/server/communication/UidlRequestHandler.java b/flow-server/src/main/java/com/vaadin/flow/server/communication/UidlRequestHandler.java index 9c122728211..b298b168ac4 100644 --- a/flow-server/src/main/java/com/vaadin/flow/server/communication/UidlRequestHandler.java +++ b/flow-server/src/main/java/com/vaadin/flow/server/communication/UidlRequestHandler.java @@ -26,11 +26,14 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import tools.jackson.databind.JsonNode; import tools.jackson.databind.node.ArrayNode; import tools.jackson.databind.node.JsonNodeType; import tools.jackson.databind.node.ObjectNode; import com.vaadin.flow.component.UI; +import com.vaadin.flow.internal.ConstantPool; +import com.vaadin.flow.internal.ConstantPoolKey; import com.vaadin.flow.internal.JacksonUtils; import com.vaadin.flow.internal.JsonDecodingException; import com.vaadin.flow.server.HandlerHelper; @@ -85,6 +88,8 @@ public class UidlRequestHandler extends SynchronizedRequestHandler private static final String RPC = RPC_INVOCATIONS; private static final String LOCATION = RPC_NAVIGATION_LOCATION; private static final String CHANGES = "changes"; + + private static final String CONSTANTS = "constants"; private static final String EXECUTE = UIDL_KEY_EXECUTE; @Override @@ -208,7 +213,7 @@ private void writeSyncError(SystemMessages systemMessages, void writeUidl(UI ui, Writer writer, boolean resync) throws IOException { ObjectNode uidl = createUidl(ui, resync); - removeOffendingMprHashFragment(uidl); + removeOffendingMprHashFragment(ui, uidl); String responseString = uidl.toString(); ui.getInternals().setLastRequestResponse(responseString); @@ -282,7 +287,7 @@ public static void commitJsonResponse(VaadinResponse response, String json) outputStream.flush(); } - private void removeOffendingMprHashFragment(ObjectNode uidl) { + private void removeOffendingMprHashFragment(UI ui, ObjectNode uidl) { if (!uidl.has(EXECUTE)) { return; } @@ -292,15 +297,18 @@ private void removeOffendingMprHashFragment(ObjectNode uidl) { int idx = -1; for (int i = 0; i < exec.size(); i++) { ArrayNode arr = (ArrayNode) exec.get(i); - for (int j = 0; j < arr.size(); j++) { + String runs = whatRuns(arr, uidl); + if (runs != null && runs.contains("history.pushState")) { + idx = i; + } + // Everything but the last element is a parameter, and the v7 UIDL + // this reaches into is one of them. The last one names what the + // invocation runs rather than being it. + for (int j = 0; j < arr.size() - 1; j++) { if (!arr.get(j).getNodeType().equals(JsonNodeType.STRING)) { continue; } String script = arr.get(j).asString(); - if (script.contains("history.pushState")) { - idx = i; - continue; - } if (!script.startsWith(SYNC_ID)) { continue; } @@ -319,9 +327,11 @@ private void removeOffendingMprHashFragment(ObjectNode uidl) { if (location != null) { ArrayNode arr = JacksonUtils.createArrayNode(); arr.add(""); - arr.add(String - .format(location.startsWith("http") ? PUSH_STATE_LOCATION - : PUSH_STATE_HASH, location)); + arr.add(asConstant(ui, uidl, + String.format( + location.startsWith("http") ? PUSH_STATE_LOCATION + : PUSH_STATE_HASH, + location))); if (idx >= 0) { exec.set(idx, arr); } else { @@ -331,6 +341,52 @@ private void removeOffendingMprHashFragment(ObjectNode uidl) { } } + /** + * What the given invocation runs, as this response carries it. + *

+ * An invocation names what it runs among the constants of the response that + * sends it, so this answers for one that runs something the client has not + * been sent before, which is what an invocation of a location that is being + * navigated to is. One that runs something the client already has is + * answered for with null, and the push state of the corrected + * location is then added to the response rather than replacing it. + */ + private static String whatRuns(ArrayNode invocation, ObjectNode uidl) { + if (invocation.isEmpty() || !uidl.has(CONSTANTS)) { + return null; + } + JsonNode name = invocation.get(invocation.size() - 1); + if (!name.getNodeType().equals(JsonNodeType.STRING)) { + return null; + } + JsonNode constant = uidl.get(CONSTANTS).get(name.asString()); + return constant != null + && constant.getNodeType().equals(JsonNodeType.STRING) + ? constant.asString() + : null; + } + + /** + * Registers the given script with the constant pool of the given UI, puts + * it among the constants of the given response when the client does not + * have it yet, and answers with what names it - which is what an invocation + * carries instead of the script. + */ + private static String asConstant(UI ui, ObjectNode uidl, String script) { + ConstantPool constantPool = ui.getInternals().getConstantPool(); + String name = constantPool.getConstantId( + new ConstantPoolKey(JacksonUtils.createNode(script))); + if (constantPool.hasNewConstants()) { + ObjectNode constants = uidl.has(CONSTANTS) + ? (ObjectNode) uidl.get(CONSTANTS) + : uidl.putObject(CONSTANTS); + constantPool.dumpConstants().properties() + .forEach(constant -> constants.set(constant.getKey(), + constant.getValue())); + } + return name; + } + private String removeHashInV7Uidl(ObjectNode json) { String removed = null; ArrayNode changes = (ArrayNode) json.get(CHANGES); diff --git a/flow-server/src/main/java/com/vaadin/flow/server/communication/UidlWriter.java b/flow-server/src/main/java/com/vaadin/flow/server/communication/UidlWriter.java index b3273cc132e..e7e1a02c9ae 100644 --- a/flow-server/src/main/java/com/vaadin/flow/server/communication/UidlWriter.java +++ b/flow-server/src/main/java/com/vaadin/flow/server/communication/UidlWriter.java @@ -44,6 +44,8 @@ import com.vaadin.flow.component.internal.PendingJavaScriptInvocation; import com.vaadin.flow.component.internal.UIInternals; import com.vaadin.flow.function.SerializableConsumer; +import com.vaadin.flow.internal.ConstantPool; +import com.vaadin.flow.internal.ConstantPoolKey; import com.vaadin.flow.internal.JacksonCodec; import com.vaadin.flow.internal.JacksonUtils; import com.vaadin.flow.internal.ResourceContentHash; @@ -56,6 +58,7 @@ import com.vaadin.flow.internal.nodefeature.ComponentMapping; import com.vaadin.flow.internal.nodefeature.ReturnChannelMap; import com.vaadin.flow.internal.nodefeature.ReturnChannelRegistration; +import com.vaadin.flow.js.JsCall; import com.vaadin.flow.server.DependencyFilter; import com.vaadin.flow.server.SystemMessages; import com.vaadin.flow.server.VaadinService; @@ -168,10 +171,6 @@ public ObjectNode createUidl(UI ui, boolean async, boolean resync) { uiInternals.clearPendingStyleSheetRemovals(); } - if (uiInternals.getConstantPool().hasNewConstants()) { - response.set("constants", - uiInternals.getConstantPool().dumpConstants()); - } if (!stateChanges.isEmpty()) { response.set("changes", stateChanges); } @@ -180,7 +179,14 @@ public ObjectNode createUidl(UI ui, boolean async, boolean resync) { .dumpPendingJavaScriptInvocations(); if (!executeJavaScriptList.isEmpty()) { response.set(JsonConstants.UIDL_KEY_EXECUTE, - encodeExecuteJavaScriptList(executeJavaScriptList)); + encodeExecuteJavaScriptList(executeJavaScriptList, + uiInternals.getConstantPool())); + } + // Dumped after the invocations are encoded, since what each of them + // runs is a constant of this response + if (uiInternals.getConstantPool().hasNewConstants()) { + response.set("constants", + uiInternals.getConstantPool().dumpConstants()); } if (service.getDeploymentConfiguration().isRequestTiming()) { response.set("timings", createPerformanceData(ui)); @@ -306,9 +312,10 @@ private static InputStream getInlineResourceStream(String url, // non-private for testing purposes static ArrayNode encodeExecuteJavaScriptList( - List executeJavaScriptList) { - return executeJavaScriptList.stream() - .map(UidlWriter::encodeExecuteJavaScript) + List executeJavaScriptList, + ConstantPool constantPool) { + return executeJavaScriptList.stream().map( + invocation -> encodeExecuteJavaScript(invocation, constantPool)) .collect(JacksonUtils.asArray()); } @@ -329,7 +336,12 @@ private static ReturnChannelRegistration createReturnValueChannel( } private static ArrayNode encodeExecuteJavaScript( - PendingJavaScriptInvocation invocation) { + PendingJavaScriptInvocation invocation, ConstantPool constantPool) { + JsCall jsCall = invocation.getInvocation().getJsCall(); + if (jsCall != null) { + return encodeJsCall(invocation, jsCall, constantPool); + } + List parametersList = invocation.getInvocation() .getParameters(); @@ -371,10 +383,74 @@ private static ArrayNode encodeExecuteJavaScript( //@formatter:on } - // [argument1, argument2, ..., script] + // [argument1, argument2, ..., what to run] + return Stream + .concat(parameters.map(JacksonCodec::encodeWithTypeInfo), + Stream.of( + constantOf(JacksonUtils.createNode(expression), + constantPool))) + .collect(JacksonUtils.asArray()); + } + + /** + * Registers what an invocation runs with the constant pool and answers with + * what names it in the invocation. + *

+ * An expression is then sent once per session rather than with every + * invocation that runs it, and the target of an invocation of declared + * JavaScript is a constant like any other. The two kinds of invocation look + * the same on the wire, and the client reads what to run out of the pool + * either way. + */ + private static JsonNode constantOf(JsonNode whatToRun, + ConstantPool constantPool) { + return JacksonUtils.createNode( + constantPool.getConstantId(new ConstantPoolKey(whatToRun))); + } + + /** + * Encodes a call made through a JavaScript definition as + * [argument1, ..., element, successChannel, errorChannel, function], + * where the trailing constant names the function to run rather than + * carrying JavaScript. The constant is an object naming the function, which + * is what tells it apart from the constant of an invocation that runs an + * expression, a string. The function is named by a hash of the JavaScript + * it runs, so no expression is sent, nothing is compiled in the browser, + * and what declared the JavaScript in Java stays on the server. + *

+ * The parameters are the arguments of the call, then the element to apply + * the function to, and the two return value channels when the call is + * subscribed to. The client reads them the other way around: the function + * it looks up takes the arguments, so what follows them is the element and + * the channels, if any. + */ + private static ArrayNode encodeJsCall( + PendingJavaScriptInvocation invocation, JsCall call, + ConstantPool constantPool) { + Stream parameters = invocation.getInvocation().getParameters() + .stream(); + + if (invocation.isSubscribed()) { + StateNode owner = invocation.getOwner(); + List channels = new ArrayList<>(); + + ReturnChannelRegistration successChannel = createReturnValueChannel( + owner, channels, invocation::complete); + ReturnChannelRegistration errorChannel = createReturnValueChannel( + owner, channels, invocation::completeExceptionally); + + parameters = Stream.concat(parameters, + Stream.of(successChannel, errorChannel)); + } + + ObjectNode function = JacksonUtils.createObjectNode(); + function.put(JsonConstants.UIDL_KEY_JS_FUNCTION, + JsCall.functionId(invocation.getInvocation().getExpression(), + call.arguments().size())); + return Stream .concat(parameters.map(JacksonCodec::encodeWithTypeInfo), - Stream.of(JacksonUtils.createNode(expression))) + Stream.of(constantOf(function, constantPool))) .collect(JacksonUtils.asArray()); } diff --git a/flow-server/src/main/java/com/vaadin/flow/shared/JsonConstants.java b/flow-server/src/main/java/com/vaadin/flow/shared/JsonConstants.java index 599e69317d6..90b7e677ce5 100644 --- a/flow-server/src/main/java/com/vaadin/flow/shared/JsonConstants.java +++ b/flow-server/src/main/java/com/vaadin/flow/shared/JsonConstants.java @@ -165,6 +165,14 @@ public class JsonConstants implements Serializable { */ public static final String UIDL_KEY_EXECUTE = "execute"; + /** + * Key of the function to run in the constant that an invocation of declared + * JavaScript names, in place of the expression that an invocation of an + * expression names. The value identifies the JavaScript that the bundle + * holds, and says nothing about the Java that declared it. + */ + public static final String UIDL_KEY_JS_FUNCTION = "f"; + /** * Key used to hold the feature id when synchronizing node values. */ diff --git a/flow-server/src/main/resources/vite.generated.ts b/flow-server/src/main/resources/vite.generated.ts index df3a234504b..f15dece4a7a 100644 --- a/flow-server/src/main/resources/vite.generated.ts +++ b/flow-server/src/main/resources/vite.generated.ts @@ -137,6 +137,12 @@ const themeOptions = { const hasExportedWebComponents = existsSync(path.resolve(frontendFolder, 'web-component.html')); const commercialBannerComponent = path.resolve(frontendFolder, settings.generatedFolder, 'commercial-banner.js'); const hasCommercialBanner = existsSync(commercialBannerComponent); +// The JavaScript declared by the @JsDefinition interfaces, generated before the +// build. Hashed into the stats like the banner above, so that a bundle whose +// definitions changed is rebuilt instead of running with the functions it was +// built with. +const jsDefinitionsFile = path.resolve(frontendFolder, settings.generatedFolder, 'vaadin-js-definitions.js'); +const hasJsDefinitions = existsSync(jsDefinitionsFile); // The browsers that Vaadin supports: Chrome, Edge and Firefox evergreen at the // versions current today, Firefox ESR, and Safari 17 in its latest minor @@ -328,6 +334,12 @@ function statsExtracterPlugin(): PluginOption { const fileBuffer = readFileSync(commercialBannerComponent, { encoding: 'utf-8' }).replace(/\r\n/g, '\n'); frontendFiles[settings.generatedFolder + '/commercial-banner.js'] = createHash('sha256').update(fileBuffer, 'utf8').digest('hex'); } + if (hasJsDefinitions) { + const fileBuffer = readFileSync(jsDefinitionsFile, { encoding: 'utf-8' }).replace(/\r\n/g, '\n'); + frontendFiles[settings.generatedFolder + '/vaadin-js-definitions.js'] = createHash('sha256') + .update(fileBuffer, 'utf8') + .digest('hex'); + } const themeJsonContents: Record = {}; const themesFolder = path.resolve(jarResourcesFolder, 'themes'); diff --git a/flow-server/src/test/java/com/vaadin/flow/component/FocusableTest.java b/flow-server/src/test/java/com/vaadin/flow/component/FocusableTest.java index 59f22075957..386f9544b03 100644 --- a/flow-server/src/test/java/com/vaadin/flow/component/FocusableTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/component/FocusableTest.java @@ -15,17 +15,22 @@ */ package com.vaadin.flow.component; +import java.util.ArrayList; import java.util.List; import org.junit.jupiter.api.Test; +import tools.jackson.databind.node.ObjectNode; import com.vaadin.flow.component.FocusOption.FocusVisible; import com.vaadin.flow.component.FocusOption.PreventScroll; import com.vaadin.flow.component.internal.PendingJavaScriptInvocation; +import com.vaadin.flow.dom.Element; +import com.vaadin.flow.js.JsCall; import com.vaadin.tests.util.MockUI; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; class FocusableTest { @@ -255,15 +260,69 @@ void focus_withoutOptions_generatesCorrectJS() { .getExpression(); assertTrue(expression.contains("setTimeout"), "Should contain setTimeout wrapper"); - assertTrue(expression.contains(".focus()"), - "Should contain focus call without parameters"); - assertFalse(expression.contains(".focus($0)"), - "Should not contain focus call with parameter"); + assertTrue(expression.contains(".focus($0)"), + "Should contain focus call with the options parameter"); - // Check the parameters + // Check the parameters: the options are null, which the browser makes + // the same as calling focus() with none List params = invocations.getFirst().getInvocation() .getParameters(); - assertEquals(1, params.size(), - "Should have exactly 1 wrapped parameter (no user-provided parameters)"); + assertEquals(2, params.size(), + "Should have the options and the element the function runs on"); + assertNull(params.getFirst(), "Should pass no options"); + } + + @Test + void pendingInvocations_runOnAnImplementationOfTheDefinition_plainJavaScriptLeftIntact() { + ui.add(component); + component.focus(PreventScroll.ENABLED); + component.getElement().executeJs("this.scrollTop = 0"); + component.blur(); + + // What a driver of the client side that cannot run JavaScript does: + // take the queue once, in order, and let Java dispatch the calls it + // recognizes onto its own implementation of the JavaScript definition + List log = new ArrayList<>(); + List unhandledJs = new ArrayList<>(); + for (PendingJavaScriptInvocation pending : ui + .dumpPendingJsInvocations()) { + JsCall call = pending.getInvocation().getJsCall(); + if (call != null + && call.definitionType() == Focusable.FocusJs.class) { + call.invokeOn(new FocusSimulation( + Element.get(pending.getOwner()), log)); + } else { + log.add("unhandled"); + unhandledJs.add(pending.getInvocation().getExpression()); + } + } + + assertEquals( + List.of("focus div {\"preventScroll\":true}", "unhandled", + "blur div"), + log, + "calls should be dispatched onto the implementation, in order"); + assertEquals(1, unhandledJs.size(), + "the application JavaScript should be left for the driver to report"); + assertTrue(unhandledJs.get(0).contains("this.scrollTop = 0"), + "the unhandled invocation should be the application JavaScript"); + } + + /** + * What a browserless driver would register for {@link Focusable.FocusJs}: + * the server-side effect of the operations, with no JavaScript involved. + */ + private record FocusSimulation(Element target, + List log) implements Focusable.FocusJs { + + @Override + public void focus(ObjectNode options) { + log.add("focus " + target.getTag() + " " + options); + } + + @Override + public void blur() { + log.add("blur " + target.getTag()); + } } } diff --git a/flow-server/src/test/java/com/vaadin/flow/dom/ElementTest.java b/flow-server/src/test/java/com/vaadin/flow/dom/ElementTest.java index 346913d9e13..4e47e44c20a 100644 --- a/flow-server/src/test/java/com/vaadin/flow/dom/ElementTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/dom/ElementTest.java @@ -77,6 +77,9 @@ import com.vaadin.flow.internal.nodefeature.ReturnChannelMap; import com.vaadin.flow.internal.nodefeature.ReturnChannelRegistration; import com.vaadin.flow.internal.nodefeature.VirtualChildrenList; +import com.vaadin.flow.js.JsCall; +import com.vaadin.flow.js.JsDefinition; +import com.vaadin.flow.js.JsExpression; import com.vaadin.flow.server.ErrorEvent; import com.vaadin.flow.server.MockVaadinServletService; import com.vaadin.flow.server.StreamResource; @@ -2662,6 +2665,57 @@ void callFunctionOnSubProperty() { assertPendingJs(ui, "return $0.property.other.method()", element); } + @Test + void executeJsWithDefinition_schedulesTheDeclaredExpressionAndCarriesTheCall() { + UI ui = new MockUI(); + Element element = ElementFactory.createDiv(); + ui.getElement().appendChild(element); + + element.executeJs(TestJs.class).method("foo"); + ui.getInternals().getStateTree().runExecutionsBeforeClientResponse(); + + List pendingJs = ui.getInternals() + .dumpPendingJavaScriptInvocations(); + assertEquals(1, pendingJs.size()); + JavaScriptInvocation invocation = pendingJs.get(0).getInvocation(); + + assertEquals("this.method($0)", invocation.getExpression(), + "the declared expression should not be wrapped, since the generated function is what runs"); + assertEquals(List.of("foo", element), invocation.getParameters(), + "the arguments should be followed by the element to apply the function to"); + assertEquals(new JsCall(TestJs.class, "method", List.of("foo")), + invocation.getJsCall()); + } + + @Test + void executeJsWithDefinition_methodReturningAResult_schedulesAndReturnsIt() { + UI ui = new MockUI(); + Element element = ElementFactory.createDiv(); + ui.getElement().appendChild(element); + + PendingJavaScriptResult result = element.executeJs(ResultJs.class) + .readValue(); + ui.getInternals().getStateTree().runExecutionsBeforeClientResponse(); + + List pendingJs = ui.getInternals() + .dumpPendingJavaScriptInvocations(); + assertEquals(1, pendingJs.size()); + assertSame(pendingJs.get(0), result, + "the result of the call should be the invocation the element scheduled"); + } + + @JsDefinition + interface ResultJs extends Serializable { + @JsExpression("return this.value;") + PendingJavaScriptResult readValue(); + } + + @JsDefinition + interface TestJs extends Serializable { + @JsExpression("this.method($0)") + void method(String value); + } + @Test void addJsInitializer_nullExpression_throws() { Element element = ElementFactory.createDiv(); diff --git a/flow-server/src/test/java/com/vaadin/flow/internal/ReflectToolsTest.java b/flow-server/src/test/java/com/vaadin/flow/internal/ReflectToolsTest.java index ffb372337b3..9f44a787b05 100644 --- a/flow-server/src/test/java/com/vaadin/flow/internal/ReflectToolsTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/internal/ReflectToolsTest.java @@ -617,6 +617,23 @@ void findDeclaredMethod_otherParameterTypes_empty() { "superMethod", Integer.class).isEmpty()); } + @Test + void getMethodsWithParameterCount_nameAndArity_allTheCallerCannotTellApart() { + assertEquals( + List.of(ReflectTools.findMethod(Overloading.class, "once", + String.class)), + ReflectTools.getMethodsWithParameterCount(Overloading.class, + "once", 1), + "a method should be found without knowing the parameter types"); + assertEquals(2, + ReflectTools.getMethodsWithParameterCount(Overloading.class, + "overloaded", 1).size(), + "overloads with the same arity cannot be told apart this way"); + assertTrue(ReflectTools + .getMethodsWithParameterCount(Overloading.class, "once", 3) + .isEmpty()); + } + @Test void findDeclaredMethod_declaredOnObject_empty() { assertTrue(ReflectTools @@ -624,6 +641,19 @@ void findDeclaredMethod_declaredOnObject_empty() { .isEmpty()); } + // S1172: the parameters are what the lookup by arity tells apart + @SuppressWarnings("java:S1172") + public static class Overloading { + public void once(String value) { + } + + public void overloaded(String value) { + } + + public void overloaded(int value) { + } + } + // S1068 and S1144: the members are looked up and used reflectively @SuppressWarnings({ "java:S1068", "java:S1144" }) private static class FieldsAndMethodsSuperclass { diff --git a/flow-server/src/test/java/com/vaadin/flow/js/JsCallTest.java b/flow-server/src/test/java/com/vaadin/flow/js/JsCallTest.java new file mode 100644 index 00000000000..8368f3726f1 --- /dev/null +++ b/flow-server/src/test/java/com/vaadin/flow/js/JsCallTest.java @@ -0,0 +1,135 @@ +/* + * Copyright 2000-2026 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.flow.js; + +import java.io.Serializable; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class JsCallTest { + + @JsDefinition + interface GreeterJs extends Serializable { + @JsExpression("window.alert($0)") + void showGreeting(String greeting); + + void undeclared(); + + @JsExpression("this.ambiguous($0)") + void ambiguous(String value); + + @JsExpression("this.ambiguous($0)") + void ambiguous(int value); + } + + private static class Greeter implements GreeterJs { + private final List greetings = new java.util.ArrayList<>(); + + @Override + public void showGreeting(String greeting) { + greetings.add(greeting); + } + + @Override + public void undeclared() { + throw new UnsupportedOperationException("nothing to do here"); + } + + @Override + public void ambiguous(String value) { + } + + @Override + public void ambiguous(int value) { + } + } + + private static JsCall call(String methodName, Object... arguments) { + return new JsCall(GreeterJs.class, methodName, + Arrays.asList(arguments)); + } + + @Test + void getExpression_methodWithoutDeclaredJavaScript_throws() { + IllegalStateException exception = assertThrows( + IllegalStateException.class, + () -> call("undeclared").getExpression()); + + assertTrue(exception.getMessage().contains("@JsExpression"), + "the message should name what the method is missing: " + + exception.getMessage()); + } + + @Test + void getExpression_overloadsWithTheSameArity_throws() { + // Overloads are resolved by name and argument count, so two of them + // with the same arity can not be told apart + assertThrows(IllegalStateException.class, + () -> call("ambiguous", "value").getExpression()); + } + + @Test + void invokeOn_implementation_runsTheMethod() { + Greeter greeter = new Greeter(); + + call("showGreeting", "Hello").invokeOn(greeter); + + assertEquals(List.of("Hello"), greeter.greetings); + } + + @Test + void nullArgument_keptAndPassedToTheImplementation() { + // What a method whose value is optional is called with - the focus + // options of a browser, for one - so it has to survive the call and + // reach the implementation as it was. + JsCall call = call("showGreeting", (Object) null); + Greeter greeter = new Greeter(); + + assertEquals(Collections.singletonList(null), call.arguments()); + + call.invokeOn(greeter); + + assertEquals(Collections.singletonList(null), greeter.greetings); + } + + @Test + void invokeOn_somethingElse_throws() { + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, + () -> call("showGreeting", "Hello").invokeOn("not a greeter")); + + assertTrue(exception.getMessage().contains(GreeterJs.class.getName()), + "the message should name the interface that was expected: " + + exception.getMessage()); + } + + @Test + void invokeOn_implementationThrows_theFailureReachesTheCaller() { + UnsupportedOperationException exception = assertThrows( + UnsupportedOperationException.class, + () -> call("undeclared").invokeOn(new Greeter())); + + assertEquals("nothing to do here", exception.getMessage(), + "what the implementation threw should not be wrapped"); + } +} diff --git a/flow-server/src/test/java/com/vaadin/flow/js/JsDefinitionProxyTest.java b/flow-server/src/test/java/com/vaadin/flow/js/JsDefinitionProxyTest.java new file mode 100644 index 00000000000..524d502e1a3 --- /dev/null +++ b/flow-server/src/test/java/com/vaadin/flow/js/JsDefinitionProxyTest.java @@ -0,0 +1,151 @@ +/* + * Copyright 2000-2026 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.flow.js; + +import java.io.Serializable; +import java.util.ArrayList; +import java.util.List; + +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +import com.vaadin.flow.component.page.PendingJavaScriptResult; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class JsDefinitionProxyTest { + + @JsDefinition + interface GreeterJs extends Serializable { + @JsExpression("window.alert($0)") + void showGreeting(String greeting); + + @JsExpression("return this.value;") + PendingJavaScriptResult readValue(); + } + + @JsDefinition + interface UndeclaredJs extends Serializable { + void undeclared(); + } + + @JsDefinition + interface UnsupportedJs extends Serializable { + @JsExpression("return this.value;") + String readValue(); + } + + @JsDefinition + interface ComposingJs extends Serializable { + @JsExpression("this.method()") + void method(); + + default void twice() { + method(); + method(); + } + } + + @JsDefinition + interface HelpingJs extends Serializable { + @JsExpression("this.method()") + void method(); + + static String name() { + return "HelpingJs"; + } + } + + private final List calls = new ArrayList<>(); + + private final PendingJavaScriptResult result = Mockito + .mock(PendingJavaScriptResult.class); + + private T proxy(Class definitionType) { + return JsDefinitionProxy.create(definitionType, call -> { + calls.add(call); + return result; + }); + } + + @Test + void create_callOfADeclaredMethod_runsItAsACall() { + GreeterJs greeter = proxy(GreeterJs.class); + + greeter.showGreeting("Hello"); + + assertEquals(List.of( + new JsCall(GreeterJs.class, "showGreeting", List.of("Hello"))), + calls); + assertNotNull(greeter.toString(), + "the implementation should answer the methods of Object"); + } + + @Test + void create_methodDeclaringAResult_answersWithWhatRanIt() { + assertEquals(result, proxy(GreeterJs.class).readValue(), + "a method that declares a result answers with the pending one"); + } + + @Test + void create_notAnInterface_throws() { + assertThrows(IllegalArgumentException.class, + () -> proxy(JsDefinitionProxyTest.class), + "only an interface can declare JavaScript methods"); + } + + @Test + void create_interfaceWithoutAnnotation_throws() { + assertThrows(IllegalArgumentException.class, + () -> proxy(Serializable.class), + "an interface the build does not collect should be refused"); + } + + @Test + void create_methodWithoutDeclaredJavaScript_throws() { + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, + () -> proxy(UndeclaredJs.class)); + + assertTrue(exception.getMessage().contains("undeclared"), + "the message should name the method that declares nothing: " + + exception.getMessage()); + } + + @Test + void create_methodWithAnotherReturnType_throws() { + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, + () -> proxy(UnsupportedJs.class)); + + assertTrue(exception.getMessage().contains("readValue"), + "the message should name the method: " + + exception.getMessage()); + } + + @Test + void create_methodImplementedInJava_throws() { + // A default method and a static one are both Java on an interface that + // is about JavaScript, so neither is quietly left aside + assertTrue(assertThrows(IllegalArgumentException.class, + () -> proxy(ComposingJs.class)).getMessage().contains("twice")); + assertTrue(assertThrows(IllegalArgumentException.class, + () -> proxy(HelpingJs.class)).getMessage().contains("name")); + } +} diff --git a/flow-server/src/test/java/com/vaadin/flow/server/communication/UidlRequestHandlerTest.java b/flow-server/src/test/java/com/vaadin/flow/server/communication/UidlRequestHandlerTest.java index 7c504e222c5..46b9a950955 100644 --- a/flow-server/src/test/java/com/vaadin/flow/server/communication/UidlRequestHandlerTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/server/communication/UidlRequestHandlerTest.java @@ -288,7 +288,9 @@ void should_changeURL_when_v7LocationProvided() throws Exception { assertEquals( "setTimeout(() => history.pushState(null, null, 'http://localhost:9998/#!away'));", - uidl.get("execute").get(1).get(1).textValue()); + whatRuns(uidl, 1), + "the push state of the corrected location should replace the one the response carried: " + + uidl); } @Test @@ -308,7 +310,9 @@ void should_updateHash_when_v7LocationNotProvided() throws Exception { assertEquals( "setTimeout(() => history.pushState(null, null, location.pathname + location.search + '#!away'));", - uidl.get("execute").get(1).get(1).textValue()); + whatRuns(uidl, 1), + "the push state of the corrected hash should replace the one the response carried: " + + uidl); } @Test @@ -524,6 +528,16 @@ protected ServerRpcHandler createRpcHandler() { "Response should have null message"); } + /** + * What the invocation at the given index of the given response runs, which + * the invocation names among the constants of the response. + */ + private static String whatRuns(ObjectNode uidl, int index) { + ArrayNode invocation = (ArrayNode) uidl.get("execute").get(index); + String name = invocation.get(invocation.size() - 1).asString(); + return uidl.get("constants").get(name).asString(); + } + private ObjectNode generateUidl(boolean withLocation, boolean withHash) { // @formatter:off @@ -533,11 +547,17 @@ private ObjectNode generateUidl(boolean withLocation, boolean withHash) { " \"clientId\": 3," + " \"changes\": []," + " \"execute\": [" + - " [\"\", \"document.title = $0\"]," + - " [\"\", \"setTimeout(() => window.history.pushState(null, '', $0))\"]," + - " [[0, 16], \"___PLACE_FOR_V7_UIDL___\", \"$0.firstElementChild.setResponse($1)\"]," + - " [1,null,[0, 16], \"return (function() { this.$server['}p']($0, true, $1)}).apply($2)\"]" + + " [\"\", \"title\"]," + + " [\"\", \"pushState\"]," + + " [[0, 16], \"___PLACE_FOR_V7_UIDL___\", \"setResponse\"]," + + " [1,null,[0, 16], \"callServer\"]" + " ]," + + " \"constants\": {" + + " \"title\": \"document.title = $0\"," + + " \"pushState\": \"setTimeout(() => window.history.pushState(null, '', $0))\"," + + " \"setResponse\": \"$0.firstElementChild.setResponse($1)\"," + + " \"callServer\": \"return (function() { this.$server['}p']($0, true, $1)}).apply($2)\"" + + " }," + " \"timings\": []" + "}"); diff --git a/flow-server/src/test/java/com/vaadin/flow/server/communication/UidlWriterTest.java b/flow-server/src/test/java/com/vaadin/flow/server/communication/UidlWriterTest.java index ae613af7c26..18d252e3fdc 100644 --- a/flow-server/src/test/java/com/vaadin/flow/server/communication/UidlWriterTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/server/communication/UidlWriterTest.java @@ -17,6 +17,7 @@ import jakarta.servlet.http.HttpServletRequest; +import java.io.Serializable; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -30,6 +31,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.parallel.Isolated; import org.mockito.Mockito; +import tools.jackson.databind.JsonNode; import tools.jackson.databind.node.ArrayNode; import tools.jackson.databind.node.ObjectNode; @@ -48,8 +50,12 @@ import com.vaadin.flow.dom.Element; import com.vaadin.flow.dom.ElementFactory; import com.vaadin.flow.internal.BundleUtils; +import com.vaadin.flow.internal.ConstantPool; import com.vaadin.flow.internal.JacksonUtils; import com.vaadin.flow.internal.StateTree; +import com.vaadin.flow.js.JsCall; +import com.vaadin.flow.js.JsDefinition; +import com.vaadin.flow.js.JsExpression; import com.vaadin.flow.router.ParentLayout; import com.vaadin.flow.router.Route; import com.vaadin.flow.router.RouteConfiguration; @@ -185,20 +191,146 @@ void testEncodeExecuteJavaScript_npmMode() { element.getNode(), invocation)) .collect(Collectors.toList()); - ArrayNode json = UidlWriter - .encodeExecuteJavaScriptList(executeJavaScriptList); + ConstantPool constantPool = new ConstantPool(); + ArrayNode json = UidlWriter.encodeExecuteJavaScriptList( + executeJavaScriptList, constantPool); + ObjectNode constants = constantPool.dumpConstants(); ArrayNode expectedJson = JacksonUtils.createArray( JacksonUtils.createArray( // Null since element is not attached JacksonUtils.nullNode(), - JacksonUtils.createNode("$0.focus()")), + JacksonUtils.createNode(nameOfWhatRuns( + JacksonUtils.createNode("$0.focus()"), + constants))), JacksonUtils.createArray( JacksonUtils.createNode("Lives remaining:"), JacksonUtils.createNode(3), - JacksonUtils.createNode("console.log($0, $1)"))); + JacksonUtils.createNode(nameOfWhatRuns( + JacksonUtils.createNode("console.log($0, $1)"), + constants)))); + + assertTrue(JacksonUtils.jsonEquals(expectedJson, json), + "an invocation should name what it runs among the constants of the message: " + + json + " " + constants); + } + + /** + * What names the given constant among the given ones, which is what an + * invocation that runs it carries instead of the constant itself. + */ + private static String nameOfWhatRuns(JsonNode whatRuns, + ObjectNode constants) { + return JacksonUtils.getKeys(constants).stream() + .filter(key -> JacksonUtils.jsonEquals(whatRuns, + constants.get(key))) + .findFirst() + .orElseThrow(() -> new AssertionError("The constants " + + constants + " should carry " + whatRuns)); + } + + /** + * The constant that names the function of the JavaScript declared for the + * given number of arguments, which is what a call of it runs. + */ + private static ObjectNode functionConstant(String expression, + int argumentCount) { + ObjectNode constant = JacksonUtils.createObjectNode(); + constant.put("f", JsCall.functionId(expression, argumentCount)); + return constant; + } + + @Test + void encodeExecuteJavaScript_sameScriptTwice_sentOnceAndNamedTwice() { + Element element = ElementFactory.createDiv(); + ConstantPool constantPool = new ConstantPool(); + + ArrayNode first = UidlWriter.encodeExecuteJavaScriptList( + List.of(new PendingJavaScriptInvocation(element.getNode(), + new JavaScriptInvocation("$0.focus()", element))), + constantPool); + constantPool.dumpConstants(); + ArrayNode second = UidlWriter.encodeExecuteJavaScriptList( + List.of(new PendingJavaScriptInvocation(element.getNode(), + new JavaScriptInvocation("$0.focus()", element))), + constantPool); + + assertEquals(nameOfWhatRuns(first), nameOfWhatRuns(second), + "the same script should be named the same way"); + assertFalse(constantPool.hasNewConstants(), + "and sent once, not with every invocation that runs it"); + } + + /** + * What the first invocation of the given list names as the thing it runs. + */ + private static String nameOfWhatRuns(ArrayNode invocations) { + ArrayNode invocation = (ArrayNode) invocations.get(0); + return invocation.get(invocation.size() - 1).asString(); + } + + @Test + void encodeExecuteJavaScript_jsCall_sendsTheTargetInsteadOfTheScript() { + Element element = ElementFactory.createDiv(); + + JsCall call = new JsCall(TestJs.class, "method", List.of("foo")); + JavaScriptInvocation invocation = new JavaScriptInvocation(call, + call.getExpression(), "foo", element); + + ConstantPool constantPool = new ConstantPool(); + ArrayNode json = UidlWriter.encodeExecuteJavaScriptList(List.of( + new PendingJavaScriptInvocation(element.getNode(), invocation)), + constantPool); + ObjectNode constants = constantPool.dumpConstants(); + + ArrayNode expectedJson = JacksonUtils.createArray( + JacksonUtils.createArray(JacksonUtils.createNode("foo"), + // Null since element is not attached + JacksonUtils.nullNode(), + JacksonUtils.createNode(nameOfWhatRuns( + functionConstant("this.method($0)", 1), + constants)))); + + assertTrue(JacksonUtils.jsonEquals(expectedJson, json), + "a call of declared JavaScript should name a function, the same way an expression names a script: " + + json + " " + constants); + assertFalse(constants.toString().contains(TestJs.class.getName()), + "and the constant should carry neither JavaScript nor what declared it: " + + constants); + } + + @Test + void encodeExecuteJavaScript_subscribedDefinitionCall_addsTheReturnChannels() { + Element element = ElementFactory.createDiv(); + + JsCall call = new JsCall(TestJs.class, "method", List.of("foo")); + JavaScriptInvocation invocation = new JavaScriptInvocation(call, + call.getExpression(), "foo", element); + PendingJavaScriptInvocation pending = new PendingJavaScriptInvocation( + element.getNode(), invocation); + pending.then(value -> { + }); + + ConstantPool constantPool = new ConstantPool(); + ArrayNode json = UidlWriter + .encodeExecuteJavaScriptList(List.of(pending), constantPool); + + ArrayNode encoded = (ArrayNode) json.get(0); + assertEquals(5, encoded.size(), + "the argument and the element should be followed by the two channels and the function to run: " + + encoded); + assertEquals( + nameOfWhatRuns(functionConstant("this.method($0)", 1), + constantPool.dumpConstants()), + encoded.get(4).asString(), + "and the function should be the same one as for a call that is not subscribed to: " + + encoded); + } - assertTrue(JacksonUtils.jsonEquals(expectedJson, json)); + @JsDefinition + interface TestJs extends Serializable { + @JsExpression("this.method($0)") + void method(String value); } @Test diff --git a/flow-test-generic/src/main/java/com/vaadin/flow/testutil/ClassesSerializableTest.java b/flow-test-generic/src/main/java/com/vaadin/flow/testutil/ClassesSerializableTest.java index a38af3d0102..35e5fd22fa7 100644 --- a/flow-test-generic/src/main/java/com/vaadin/flow/testutil/ClassesSerializableTest.java +++ b/flow-test-generic/src/main/java/com/vaadin/flow/testutil/ClassesSerializableTest.java @@ -268,6 +268,7 @@ protected Stream getExcludedPatterns() { // Static Utilities "com\\.vaadin\\.flow\\.component\\.wakelock\\.WakeLock", + "com\\.vaadin\\.flow\\.js\\.JsDefinitionProxy", // Flow client classes "com\\.vaadin\\.client\\..*", diff --git a/vaadin-dev-server/src/main/java/com/vaadin/base/devserver/devloop/DevLoopRedefiner.java b/vaadin-dev-server/src/main/java/com/vaadin/base/devserver/devloop/DevLoopRedefiner.java index e8a6c555bcd..4a77951ec5a 100644 --- a/vaadin-dev-server/src/main/java/com/vaadin/base/devserver/devloop/DevLoopRedefiner.java +++ b/vaadin-dev-server/src/main/java/com/vaadin/base/devserver/devloop/DevLoopRedefiner.java @@ -63,6 +63,9 @@ import com.vaadin.flow.internal.DevModeHandler; import com.vaadin.flow.internal.DevModeHandlerManager; import com.vaadin.flow.internal.ThemeUtils; +import com.vaadin.flow.js.JsCall; +import com.vaadin.flow.js.JsDefinition; +import com.vaadin.flow.js.JsExpression; import com.vaadin.flow.server.VaadinService; import com.vaadin.flow.theme.Theme; @@ -1374,6 +1377,23 @@ static String frontendDependencies(Class type) { + ":" + annotation.themeFor()); } } + // The JavaScript a JavaScript definition declares is generated into + // the bundle by the build, exactly like the imports above, so an + // edited expression or a method added or removed only reaches the + // browser through a restart that regenerates the file and rebuilds the + // bundle. What identifies a function is what it runs, so that is what + // the fingerprint is made of: renaming a method changes nothing the + // browser has, and editing what it declares changes everything. + if (type.isAnnotationPresent(JsDefinition.class)) { + for (Method method : type.getMethods()) { + JsExpression expression = method + .getAnnotation(JsExpression.class); + if (expression != null) { + imports.add("jsdefinition:" + JsCall.functionId( + expression.value(), method.getParameterCount())); + } + } + } // These two are read off the class whatever it is. @Theme in particular // has to sit on the AppShellConfigurator, which is never a Component - // so reading it only from Components meant a theme could be added, diff --git a/vaadin-dev-server/src/main/java/com/vaadin/base/devserver/hotswap/impl/JsDefinitionHotswapper.java b/vaadin-dev-server/src/main/java/com/vaadin/base/devserver/hotswap/impl/JsDefinitionHotswapper.java new file mode 100644 index 00000000000..bfd4f6080b8 --- /dev/null +++ b/vaadin-dev-server/src/main/java/com/vaadin/base/devserver/hotswap/impl/JsDefinitionHotswapper.java @@ -0,0 +1,135 @@ +/* + * Copyright 2000-2026 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.base.devserver.hotswap.impl; + +import java.util.List; +import java.util.stream.Collectors; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.vaadin.base.devserver.hotswap.HotswapClassEvent; +import com.vaadin.base.devserver.hotswap.VaadinHotswapper; +import com.vaadin.flow.di.Lookup; +import com.vaadin.flow.internal.FrontendUtils; +import com.vaadin.flow.js.JsDefinition; +import com.vaadin.flow.js.JsExpression; +import com.vaadin.flow.server.AbstractConfiguration; +import com.vaadin.flow.server.Mode; +import com.vaadin.flow.server.VaadinService; +import com.vaadin.flow.server.frontend.Options; +import com.vaadin.flow.server.frontend.TaskGenerateJsDefinitions; +import com.vaadin.flow.server.startup.ApplicationConfiguration; + +/** + * Reports a {@link JsDefinition} interface whose JavaScript the frontend bundle + * does not carry. + *

+ * The JavaScript a definition method declares with {@link JsExpression} is + * collected into the bundle when the frontend is built. Redefining the + * interface therefore does not change what the browser can run: a call made + * after the change either runs the JavaScript the bundle was built with, or + * finds no function at all when the redefinition renamed or added a method. + *

+ * With the frontend dev server running, the file the functions are generated + * into is written again from what the interfaces now declare. The dev server + * replaces the module in every browser that has it, the file registers the new + * functions, and a call made afterwards runs them - no restart, and nothing is + * compiled from a string in the browser, since the dev server serves the file + * it just read. Without the dev server, a bundle is what the browser runs and + * only a build produces a new one, so the change is reported instead. + *

+ * The comparison is against the generated file the bundle was built from, which + * is what the browser can run, and it uses the same rendering the build wrote, + * so the declared JavaScript of every method and the number of arguments it + * takes have to match for an interface to pass silently. + *

+ * For internal use only. May be renamed or removed in a future release. + */ +public class JsDefinitionHotswapper implements VaadinHotswapper { + + @Override + public void onClassesChange(HotswapClassEvent event) { + List> definitions = event.getChangedClasses().stream() + .filter(type -> type.isAnnotationPresent(JsDefinition.class)) + .toList(); + if (definitions.isEmpty()) { + return; + } + + VaadinService service = event.getVaadinService(); + Options options = buildOptions(service); + + List> missing; + if (canReplaceInTheBrowser(service)) { + // Writing the file again is what the browser runs afterwards, and + // the write leaves the file alone when nothing it holds changed, + // so what comes back is what could not be applied + missing = TaskGenerateJsDefinitions.updateJsDefinitions(options, + definitions); + } else { + // What a browser has without the dev server is a bundle, which + // only a build produces, so a change can only be reported + missing = TaskGenerateJsDefinitions + .findMissingFromGeneratedFile(options, definitions); + } + + if (!missing.isEmpty()) { + warnAboutMissingDefinitions(missing); + } + } + + private static boolean canReplaceInTheBrowser(VaadinService service) { + ApplicationConfiguration configuration = ApplicationConfiguration + .get(service.getContext()); + return configuration != null && configuration + .getMode() == Mode.DEVELOPMENT_FRONTEND_LIVERELOAD; + } + + /** + * The least the generated file needs to be read and written: where the + * project is and where its frontend folder is. No class finder, since what + * the file has to hold is passed in rather than scanned for. + */ + private static Options buildOptions(VaadinService service) { + AbstractConfiguration configuration = service + .getDeploymentConfiguration(); + return new Options(service.getContext().getAttribute(Lookup.class), + null, configuration.getProjectFolder()).withFrontendDirectory( + FrontendUtils.getProjectFrontendDir(configuration)); + } + + /** + * Warns that the bundle does not carry what the given interfaces declare. + *

+ * Package-private so that what a change is warned about can be asserted. + * + * @param definitions + * the JavaScript definitions the bundle does not carry, never + * empty + */ + void warnAboutMissingDefinitions(List> definitions) { + getLogger().warn( + "The JavaScript declared by {} is not the JavaScript the frontend bundle carries. " + + "It is collected into the bundle when the frontend is built, so a call made through the definition keeps running the previous version, or finds no function at all, until the application is restarted.", + definitions.stream().map(Class::getName) + .collect(Collectors.joining(", "))); + } + + private static Logger getLogger() { + return LoggerFactory.getLogger(JsDefinitionHotswapper.class); + } +} diff --git a/vaadin-dev-server/src/main/java/com/vaadin/base/devserver/startup/DevModeStartupListener.java b/vaadin-dev-server/src/main/java/com/vaadin/base/devserver/startup/DevModeStartupListener.java index 93db16c3b47..f4f1cc49725 100644 --- a/vaadin-dev-server/src/main/java/com/vaadin/base/devserver/startup/DevModeStartupListener.java +++ b/vaadin-dev-server/src/main/java/com/vaadin/base/devserver/startup/DevModeStartupListener.java @@ -40,6 +40,7 @@ import com.vaadin.flow.di.Lookup; import com.vaadin.flow.internal.DevModeHandlerManager; import com.vaadin.flow.internal.Template; +import com.vaadin.flow.js.JsDefinition; import com.vaadin.flow.router.HasErrorParameter; import com.vaadin.flow.router.Layout; import com.vaadin.flow.router.Route; @@ -73,7 +74,7 @@ Template.class, LoadDependenciesOnStartup.class, TypeScriptBootstrapModifier.class, DevToolsMessageHandler.class, Component.class, Layout.class, StyleSheet.class, - StyleSheet.Container.class }) + StyleSheet.Container.class, JsDefinition.class }) @WebListener public class DevModeStartupListener implements VaadinServletContextStartupInitializer, Serializable, diff --git a/vaadin-dev-server/src/main/resources/META-INF/services/com.vaadin.base.devserver.hotswap.VaadinHotswapper b/vaadin-dev-server/src/main/resources/META-INF/services/com.vaadin.base.devserver.hotswap.VaadinHotswapper index b259e50e312..bfbb214854a 100644 --- a/vaadin-dev-server/src/main/resources/META-INF/services/com.vaadin.base.devserver.hotswap.VaadinHotswapper +++ b/vaadin-dev-server/src/main/resources/META-INF/services/com.vaadin.base.devserver.hotswap.VaadinHotswapper @@ -20,3 +20,4 @@ com.vaadin.base.devserver.hotswap.impl.DefaultTranslationsHotswapper com.vaadin.base.devserver.hotswap.impl.RouteRegistryHotswapper com.vaadin.base.devserver.hotswap.impl.ErrorViewHotswapper com.vaadin.base.devserver.devloop.DevLoopHotswapper +com.vaadin.base.devserver.hotswap.impl.JsDefinitionHotswapper diff --git a/vaadin-dev-server/src/test/java/com/vaadin/base/devserver/devloop/DevLoopRedefinerTest.java b/vaadin-dev-server/src/test/java/com/vaadin/base/devserver/devloop/DevLoopRedefinerTest.java index 6be8aead3a7..6c72ecfd754 100644 --- a/vaadin-dev-server/src/test/java/com/vaadin/base/devserver/devloop/DevLoopRedefinerTest.java +++ b/vaadin-dev-server/src/test/java/com/vaadin/base/devserver/devloop/DevLoopRedefinerTest.java @@ -18,6 +18,7 @@ import java.io.File; import java.io.IOException; import java.io.InputStream; +import java.io.Serializable; import java.net.URISyntaxException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; @@ -36,10 +37,14 @@ import com.vaadin.flow.component.dependency.JsModule; import com.vaadin.flow.component.dependency.NpmPackage; import com.vaadin.flow.component.page.AppShellConfigurator; +import com.vaadin.flow.js.JsCall; +import com.vaadin.flow.js.JsDefinition; +import com.vaadin.flow.js.JsExpression; import com.vaadin.flow.theme.Theme; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -159,6 +164,18 @@ static class SomeView extends Component { static class NothingDeclared { } + @JsDefinition + interface GreeterJs extends Serializable { + @JsExpression("window.alert($0)") + void showGreeting(String greeting); + } + + @JsDefinition + interface EditedGreeterJs extends Serializable { + @JsExpression("window.alert('edited ' + $0)") + void showGreeting(String greeting); + } + @Test void frontendDependencies_seesTheThemeOnAnAppShellThatIsNoComponent() { // @Theme belongs on the AppShellConfigurator, which is never a @@ -175,6 +192,22 @@ void frontendDependencies_seesTheThemeOnAnAppShellThatIsNoComponent() { assertTrue(imports.contains("dark"), imports); } + @Test + void frontendDependencies_seesTheJavaScriptADefinitionDeclares() { + // The declared JavaScript is generated into the bundle by the build, so + // a redefined interface leaves the browser running the JavaScript the + // bundle was built with until a restart regenerates it. + String imports = DevLoopRedefiner.frontendDependencies(GreeterJs.class); + + assertTrue(imports.contains( + "jsdefinition:" + JsCall.functionId("window.alert($0)", 1)), + imports); + // What the browser has of a method is the function of what it + // declares, so an edited expression is a different one. + assertNotEquals(imports, + DevLoopRedefiner.frontendDependencies(EditedGreeterJs.class)); + } + @Test void frontendDependencies_seesBuildTimeImportsOnANonComponent() { String imports = DevLoopRedefiner diff --git a/vaadin-dev-server/src/test/java/com/vaadin/base/devserver/hotswap/impl/JsDefinitionHotswapperTest.java b/vaadin-dev-server/src/test/java/com/vaadin/base/devserver/hotswap/impl/JsDefinitionHotswapperTest.java new file mode 100644 index 00000000000..6d9a8eec6b5 --- /dev/null +++ b/vaadin-dev-server/src/test/java/com/vaadin/base/devserver/hotswap/impl/JsDefinitionHotswapperTest.java @@ -0,0 +1,226 @@ +/* + * Copyright 2000-2026 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.base.devserver.hotswap.impl; + +import java.io.File; +import java.io.IOException; +import java.io.Serializable; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.mockito.Mockito; + +import com.vaadin.base.devserver.hotswap.HotswapClassEvent; +import com.vaadin.flow.di.Lookup; +import com.vaadin.flow.internal.FrontendUtils; +import com.vaadin.flow.js.JsDefinition; +import com.vaadin.flow.js.JsExpression; +import com.vaadin.flow.server.MockVaadinServletService; +import com.vaadin.flow.server.Mode; +import com.vaadin.flow.server.frontend.Options; +import com.vaadin.flow.server.frontend.TaskGenerateJsDefinitions; +import com.vaadin.flow.server.startup.ApplicationConfiguration; +import com.vaadin.flow.server.startup.ApplicationConfigurationFactory; +import com.vaadin.tests.util.MockDeploymentConfiguration; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class JsDefinitionHotswapperTest { + + @JsDefinition + interface GreeterJs extends Serializable { + @JsExpression("window.alert($0); this.focus()") + void showGreeting(String greeting); + } + + @JsDefinition + interface CounterJs extends Serializable { + @JsExpression("this.count = ($0 || 0) + 1") + void count(Integer from); + } + + static class NotADefinition { + } + + // Records what a change is warned about instead of logging it. + private static class TestHotswapper extends JsDefinitionHotswapper { + private final List reported = new ArrayList<>(); + + @Override + void warnAboutMissingDefinitions(List> definitions) { + definitions.stream().map(Class::getName).forEach(reported::add); + } + } + + @TempDir + File projectFolder; + + private TestHotswapper hotswapper; + private MockVaadinServletService service; + private File frontendFolder; + private ApplicationConfiguration configuration; + + @BeforeEach + void setUp() { + hotswapper = new TestHotswapper(); + + MockDeploymentConfiguration deploymentConfiguration = new MockDeploymentConfiguration(); + deploymentConfiguration.setProjectFolder(projectFolder); + service = new MockVaadinServletService(deploymentConfiguration); + // Resolved the way the production code resolves it, so a case writes + // the file where the hotswapper looks for it + frontendFolder = FrontendUtils + .getProjectFrontendDir(deploymentConfiguration); + + configuration = Mockito.mock(ApplicationConfiguration.class); + // What a browser runs without the frontend dev server is a bundle, + // which a case has to opt out of to get the file written again. + Mockito.when(configuration.getMode()) + .thenReturn(Mode.DEVELOPMENT_BUNDLE); + Mockito.when(service.getLookup() + .lookup(ApplicationConfigurationFactory.class)) + .thenReturn(context -> configuration); + } + + /** + * Puts the frontend dev server in play, which is what can replace the + * generated file in a running browser. + */ + private void withFrontendDevServer() { + Mockito.when(configuration.getMode()) + .thenReturn(Mode.DEVELOPMENT_FRONTEND_LIVERELOAD); + } + + private File generatedFile() { + return new File( + FrontendUtils.getFrontendGeneratedFolder(frontendFolder), + FrontendUtils.JS_DEFINITIONS_FILE_NAME); + } + + private String readGeneratedDefinitions() throws IOException { + return Files.readString(generatedFile().toPath(), + StandardCharsets.UTF_8); + } + + /** + * Writes the file as a build writes it for the given interface, where the + * hotswapper looks for it, which is what a browser would be running. + */ + private void writeGeneratedDefinitionsFor(Class definition) { + Options options = new Options(Mockito.mock(Lookup.class), null, null) + .withFrontendDirectory(frontendFolder); + TaskGenerateJsDefinitions.updateJsDefinitions(options, + List.of(definition)); + } + + /** + * Edits the written file, to make it the older version of the declarations + * that a browser would still be running. + */ + private void editGeneratedDefinitions(String declared, String previously) + throws IOException { + Files.writeString(generatedFile().toPath(), + readGeneratedDefinitions().replace(declared, previously), + StandardCharsets.UTF_8); + } + + private void classesChanged(Class... classes) { + hotswapper.onClassesChange( + new HotswapClassEvent(service, Set.of(classes), true)); + } + + @Test + void fileCarriesTheDeclarations_nothingReported() { + writeGeneratedDefinitionsFor(GreeterJs.class); + + classesChanged(GreeterJs.class); + + assertTrue(hotswapper.reported.isEmpty(), + "a browser running what the interface declares needs nothing said about it: " + + hotswapper.reported); + } + + @Test + void fileDoesNotCarryTheDeclarations_reported() { + classesChanged(GreeterJs.class); + + assertEquals(List.of(GreeterJs.class.getName()), hotswapper.reported, + "without the dev server only a build can put them there, so say so"); + } + + @Test + void frontendDevServerRunning_appliedInsteadOfReported() + throws IOException { + writeGeneratedDefinitionsFor(GreeterJs.class); + editGeneratedDefinitions("window.alert($0); this.focus()", + "window.alert($0)"); + withFrontendDevServer(); + + classesChanged(GreeterJs.class); + + assertTrue(hotswapper.reported.isEmpty(), + "with the dev server the change is applied, not reported: " + + hotswapper.reported); + assertTrue( + readGeneratedDefinitions() + .contains("window.alert($0); this.focus()"), + "and what the browser reloads is what the interface declares now"); + } + + @Test + void frontendDevServerRunningAndFileUpToDate_fileLeftAlone() { + writeGeneratedDefinitionsFor(GreeterJs.class); + // A moment in the past, so that a write of the same content shows + generatedFile().setLastModified(System.currentTimeMillis() - 60_000); + long untouched = generatedFile().lastModified(); + withFrontendDevServer(); + + classesChanged(GreeterJs.class); + + assertTrue(hotswapper.reported.isEmpty(), + "the browser is running what the interface declares: " + + hotswapper.reported); + assertEquals(untouched, generatedFile().lastModified(), + "a redefinition that changes no JavaScript should leave the file alone, or the dev server replaces the module in every browser for nothing"); + } + + @Test + void frontendDevServerRunningButNothingCanBeWritten_reported() { + // A directory where the file belongs: the change cannot be applied, so + // it is reported rather than passing as applied + generatedFile().mkdirs(); + withFrontendDevServer(); + + classesChanged(GreeterJs.class); + + assertEquals(List.of(GreeterJs.class.getName()), hotswapper.reported); + } + + @Test + void noDefinitionChanged_nothingReported() { + classesChanged(NotADefinition.class); + + assertTrue(hotswapper.reported.isEmpty(), + "a class that declares no JavaScript is not a frontend change"); + } +} diff --git a/vaadin-dev-server/src/test/java/com/vaadin/base/devserver/startup/DevModeClassFinderTest.java b/vaadin-dev-server/src/test/java/com/vaadin/base/devserver/startup/DevModeClassFinderTest.java index f0d3cb8b61b..985cb15e218 100644 --- a/vaadin-dev-server/src/test/java/com/vaadin/base/devserver/startup/DevModeClassFinderTest.java +++ b/vaadin-dev-server/src/test/java/com/vaadin/base/devserver/startup/DevModeClassFinderTest.java @@ -38,6 +38,7 @@ import com.vaadin.flow.component.dependency.StyleSheet; import com.vaadin.flow.component.page.AppShellConfigurator; import com.vaadin.flow.internal.Template; +import com.vaadin.flow.js.JsDefinition; import com.vaadin.flow.router.HasErrorParameter; import com.vaadin.flow.router.Layout; import com.vaadin.flow.router.Route; @@ -75,7 +76,7 @@ void applicableClasses_knownClasses() { Template.class, LoadDependenciesOnStartup.class, Component.class, TypeScriptBootstrapModifier.class, DevToolsMessageHandler.class, Layout.class, StyleSheet.class, - StyleSheet.Container.class); + StyleSheet.Container.class, JsDefinition.class); for (Class clz : classes) { assertTrue(knownClasses.contains(clz),