vaadin / flow

Vaadin Flow is a Java framework binding Vaadin web components to Java. This is part of Vaadin 10+.
Apache License 2.0
605 stars 166 forks source link

Make it possible for executeJavaScript snippets to reference values exported by a JS module #5094

Open Legioth opened 5 years ago

Legioth commented 5 years ago

When I write JavaScript into a JS module file, I can easily use stuff exported from another module:

import { bar } from "foo";

bar("Hello");

Things get messy if I want to pass a value from the server to bar. Without any framework support, I'd have to create a separate myModule.js module file that imports bar and publishes it somewhere in the global namespace.

import { bar } from "foo";

window.publishedBar = bar;

My Java logic would then need to have @JsModule("./myModule.js") and only then could I do executeJavaScript("publishedBar($0)", "Hello");. Even though this would work, it would require quite much boilerplate.

My assumption is that information about what to import from where would have to be included in the input to webpack (i.e. main.js), which in turn means that it would have to be expressed in a declarative way in the Java code so that the logic that generates main.js could find it.

Since generation of main.js is already based on scanning for @JsModue annotations, we could use additional information in those annotations to generate code in main.js that imports specific parts and stores them somewhere for future use. Based on something like @JsModule(value="foo", imports = {"bar", "baz"}), we could generate main.js like this:

import {bar as generated123, baz as generated321} from "foo";

window.Vaadin.Flow.imports["foo"] = {bar: generated123, baz: generated321};

Based on this, I could do executeJavaScript("Vaadin.Flow.imports['foo'].bar($0)", "Hello"). The Vaadin.Flow.imports syntax is still quite verbose and not very discoverable, but I would at the very least not have to create a separate myModule.js file. In this case, I could have written .foo instead of ['foo'], but that wouldn't be feasible if importing from e.g. @vaadin/vaadin-grid/someFile.js.

We could also consider always injecting an imports variable into the scope that we set up when evaluating JS expressions from the server, thus simplifying the expression to executeJavaScript("imports['foo'].bar($0), "Hello");.

Legioth commented 5 years ago

Based on a separate discussion with @manolo, there was an alternative idea of making specific imports directly available in the JS expression instead of using an indirect reference like imports['foo'].bar.

Which imports to make available would be based on a class literal passed to executeJavaScript - the available imports would be based on @JsModule annotations declared on that particular class. This means that the generated main.js file would populate a map from fully qualified Java class names to a map of values to import for that class.

import {bar as generated123} from "module1";
import {baz as generated321} from "module2";

window.Vaadin.Flow.imports["com.example.MyView"] = {bar: generated123, baz: generated321};

Using the modules from Java could then be expressed as executeJavaScript(MyView.class, "console.log(bar, baz)");

One big benefit of this approach is that if offers better discoverability of the feature - developers can find out about the feature by observing there's an additional overload of the method and thus looking at javadocs to discover the difference.

The big challenge is how to deal with duplicates in case a class imports multiple modules that have exports with the same name. In JS, this would be handled by renaming either import or by importing * with different names.

import { foo } from "module1";
import { foo as bar} from "module2";
console.log(foo, bar);

or

import * as m1 from "module1";
import * as m2 from "module2";
console.log(m1.foo, m2.foo);

There are two potential ideas for how to express this in an annotation in a format that enables generating code for main.js.

  1. Use exactly the same syntax as in JS files. This could be expressed as @JsModule(value="module2", imports="{foo as bar}") or alternatively the entire statement, e.g. @JsImport("import {foo as bar} from 'module2'"). Implementing this would require us to parse the string to be able to generate the code that exports the correct values into window.Vaadin.Flow.imports.
  2. Design an annotation API that allows expressing the same thing in a structured way. For an individual rename, this could be something like @JsModule(value="module2", name = "foo", as="bar"). Expressing a more complex import definition such as import foo, {foo2, foo3 as bar} from "module3" would require a relatively elaborate annotation API design.
bogdanudrescu commented 5 years ago

Using the modules from Java could then be expressed as executeJavaScript(MyView.class, "console.log(bar, baz)");

What about accessing in the same executeJavaScript literals generated from annotations defined in several classes, possibly with same name? Since they are stored in the global namespace they are accessible from everywhere. Or we limit the api to just one class?

Then several classes importing the same package may inject the same module exports again, with or without the same name. Though this may be a rare case.

Legioth commented 5 years ago

My idea was that the executeJavaScript method would only accept one class literal. In the rare case where you need to import some kind of combination, then you'd have to create a new class so that annotations on that class would define any renames necessary to avoid conflicts. This would basically mean that we could always check for conflicts at compile time.

Legioth commented 4 years ago

Yet another alternative would be to not expose the imports as top-level variables, but instead use them as regular $n parameters with executeJS, i.e. something like this: executeJS("console.log($0.foo, $0.bar)", Foo.class) where Foo would have annotations that define from where foo and bar are imported.

I was originally thinking that we would reuse @JsModule that are already present on Component subclasses. Passing a component class as the parameter to executeJS is semantically weird. It would be more logical to have a dedicated class that only represents the values that can be imported from a module but has no other meaning. We could still use the same @JsModule annotation that is supplemented with optional fields for defining imports, but we would remove the assumption that @JsModule can only be used on component classes.

Similarly to how attaching a component would ensure all its @JsModule definitions are actually loaded in the browser (only to ensure the fallback bundle is loaded if needed until we implement proper support for code splitting), there would also be similar logic that ensures bundles are loaded if necessary the first time a @JsModule value is encountered through executeJS.

This approach would also allow importing * from a module without declaring individual imports. A * import shouldn't be allowed in combination with any other import to avoid shadowing problems. This is a quite harmless limitation since you could access the imports from one module as $0 and the imports from another module as $1 rather than merging both into the same namespace.

The only user-facing API change would thus be to add two optional fields to @JsModule: String[] imports() default {}; for naming specific imports (or "default") to include and boolean importAll() default false; to import everything. In addition to this, there would be some logic changes to how generated-flow-imports.js is generated, support for a new parameter type to executeJS, and making that method trigger loading additional bundles if needed.

Legioth commented 4 years ago

With my latest proposal, you could thus use lit-html to render the contents of an element with the help of this import declaration:

@JsModule(value = "lit-html", imports = {"render", "html"})
public class LitImports {}

and this snippet to actually do the rendering:

element.executeJs("$0.render($0.html`<div>Hello ${$1}</div>`, this), LitImports.class, "Lit");

Behind the scenes, something like this would be generated in generated-flow-imports.js:

import { render as generated1, html as generated2 } from 'lit-html';

window.Vaadin.Flow.imports["com.example.LitImports"] = { render: generated1, html: generated2 };

And then the logic that sets up the context for evaluating the JS snippet in the browser would do something analogous to this:

let $0 = window.Vaadin.Flow.imports["com.example.LitImports"];

It would also be possible to use a wildcard import, i.e. @JsModule(value = "lit-html", importAll = true) which would otherwise work the same, except that the code generated in generated-flow-imports.js would be like

import * as generated1 from 'lit-html';

window.Vaadin.Flow.imports["com.example.LitImports"] = generated1;

If you prefer to have the lit-html part look like lit-html, then you can first destructure the imports that you will use:

element.executeJS("let {render, html} = $0; render(html`<div>Hello ${$1}</div>`, this)", LitImports.class, "Lit");

If you prefer to write JS in a JS file, then you could create a myScripts.js with the actual logic

import { html, render } from 'lit-html';

export function renderElement(element, text) {
  render(html`<div>Hello ${text}</div>`, element);
}

and just invoke it from Java:

element.executeJs($0.renderElement(this, $1), MyScripts.class, "Lit");

This also opens the idea of having a Java interface for the exported functions of a JS module so that you could do something like this:

JsModuleProxy.get(MyScripts.class).renderElement(element, "Lit");

But that's a separate discussion.

Legioth commented 3 years ago

Based on my latest thinking, we should not not have executeJs at all and thus also not have any problem with referencing JS module exports in such snippets. The reason for that no having executeJs at all is that it's unfriendly to CSP and has a possibility of causing XSS vulnerabilities if used in the wrong way.

Instead, I'm proposing a new mechanism in https://github.com/vaadin/flow/issues/10759 which would as an added benefit also give natural interop with JS modules.

The original example in this issue with passing a string from the server to a bar function exported from a foo module would then be expressed by defining a Java interface as an overlay over that module, acquiring a proxy instance of that interface from the framework and invoking a method on that proxy.

Interface definition:

@JsExpressionModule("foo")
public interface Foo {
  void bar(String greeting);
}

Usage:

page.getJsInvoker(Foo.class).bar("Hello");

To make more advanced integrations e.g. based on combining multiple modules, one can create an intermediate module with appropriate imports that would then export the combining logic. This intermediate module works in exactly the same way as any other JS module which means that it's free to make arbitrary imports and exports.

import { bar } from 'foo';
import { bar as anotherBar ) from 'baz';

export function doStuff(value) {
  bar(anotherBar(value));
}
samie commented 2 years ago

I used a workaround JS like file:

import { MyThing } from './mything.js';
window.MyThing = MyThing;

This works but is rather inconvenient.