vaadin / flow

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

JavaScript include works under v14.1.25, but not under 15.0.4 #8081

Closed markhm closed 4 years ago

markhm commented 4 years ago

My Mapbox wrapper component mapbox-flow works under Vaadin 14.1.25, but not under Vaadin 15.0.4. In lack of a migration guide, I am assuming this is a bug in v15.0.4, please correct me if I am wrong.

To reproduce:

Now change the pom.xml to v15.0.4 and run mvn clean jetty:run. (Note that package-lock.json ea are cleaned via a automatically via a maven plugin.) Now observe the map component is not loaded correctly in the demo page.

I have confirmed this problem both on Windows 10 and macOS Catalina using JDK 13.

Many thanks.

pleku commented 4 years ago

Hi. Thanks for the report, I think this is the same as https://github.com/vaadin/flow/issues/8053

You can try to verify with 15.0-SNAPSHOT (you need to add the vaadin-prereleases maven repository with snapshots enabled) or wait for the upcoming 15.0.5 release, coming out probably latest next Monday.

markhm commented 4 years ago

Thank you for your response, @pleku .

I am on 15.0-SNAPSHOT now, but unfortunately, the problem is still there.

I can see in the browser console that the required files (mapbox.js and mapbox.css) are not found and that instead an autogenerated index.html is returned, please see attached screenshots.

I include the necessary files as follows (which works under 14.1.25, as mentioned):

  page.addStyleSheet("frontend://com/github/markhm/mapbox-flow/mapbox.css");
  page.addJavaScript("frontend://com/github/markhm/mapbox-flow/mapbox.js");

Might this be another problem...?

Best, Mark

Capture Capture-2

pleku commented 4 years ago
page.addStyleSheet("frontend://com/github/markhm/mapbox-flow/mapbox.css");
page.addJavaScript("frontend://com/github/markhm/mapbox-flow/mapbox.js");

Right, you cannot use the protocol frontend:// as is not supported starting from V15 / Flow 3.0 onwards. You should use a similar path like it is done here https://github.com/vaadin/component-starter-flow/blob/master/src/main/java/org/vaadin/artur/paperslider/PaperSlider.java#L14

frontend:// was used earlier to automagically refer to either es-5 or es-6 resources based on what the browser required. Since 15 es5 transpilation is no longer supported so the protocol was dropped.

But I'm not sure what the JS errors I see in the browser console are about and if they are affected by not finding the proper resources or not.

pleku commented 4 years ago

If this indeed is just about wrongly using frontend:// with 15 and nothing else, we should change the code to fail with a much more clearer error instead of this, but lets see if it helps.

markhm commented 4 years ago

I had already tried various combinations of adding the files (with/without frontend://) at the beginning of the class or in the method without success.

Based on your comments, the beginning of my class now looks like this:

@JavaScript(value = "https://api.tiles.mapbox.com/mapbox-gl-js/v1.9.1/mapbox-gl.js")
@StyleSheet(value = "https://api.tiles.mapbox.com/mapbox-gl-js/v1.9.1/mapbox-gl.css")
@JavaScript(value = "https://api.tiles.mapbox.com/mapbox.js/plugins/turf/v3.0.11/turf.min.js")
@CssImport("./com/github/markhm/mapbox-flow/mapbox.css")
@JsModule("./com/github/markhm/mapbox-flow/mapbox.js")
public class MapboxMap extends Div
{
    ...
}

Unfortunately, this does not result in correctly loading of the files under 15.0-SNAPSHOT at the moment.

I should note that I have never been able to get the @JsModule tag to work. Under 14.1.25, it does work if I load the two local files - the last two elements - from within a render() method as follows:

private void render()
{
    page.addJavaScript("./com/github/markhm/mapbox-flow/mapbox.js");
    page.addStyleSheet("./com/github/markhm/mapbox-flow/mapbox.css");
    ...
}

At the moment, loading from within a render() method like this does not work for me under 15.0-SNAPSHOT.

Hope we can get to the bottom of this.

markhm commented 4 years ago

FYI: I've just checked and v15.0.5 does not solve this issue.

haijian-vaadin commented 4 years ago

Hi @markhm, thanks for the feedback, indeed it's a bug in Vaadin 15 that page.addStyleSheet() doesn't work.

But meanwhile, the recommended way to import js files is to use the @JsModule or @JavaScipt, the difference is that @JavaScript allows importing external js files.

Why it doesn't work for you is that the mapbox.js file is bundled with other frontend resources by Webpack, which means that all the variables and functions as global scope are not available globally anymore.

As a quick workaround, you can add window prefix to the variables and functions that you want to use globally (e.g. window.map). I did some experiment on the mapbox.js file and seems it's work, you can take a look at the attached zip file.

mapbox-flow.zip

markhm commented 4 years ago

Hi @haijian-vaadin, thank you for looking into this.

I see the window. prefix that you added in the mapbox.js file, but it is unclear to me why this is suddenly needed since V15.

I note that I am only interested in using the MapboxMap component and the corresponding JS file locally (vs globally), so perhaps I should revise how it is integrated on JS side?

I am unable to find instructions for this in the Vaadin 15 Documentation.

Can you point me to an example? 🙏🏻

markhm commented 4 years ago

Never mind my previous response, @haijian-vaadin. I had hoped to keep using my mapbox-flow component without looking into PolymerTemplates, but perhaps it is time to bite the bullet now.

markhm commented 4 years ago

After taking the first steps in migrating to a Polymer template-based implementation, I can confirm that the @JsModule include works for v15.0.5, although not completely, see the following two screenshots.

This is how it should be, v14.0.2.alpha11:

v14 0 2 alpha11

This is exactly the same codebase, but with v15.0.5:

v15 0 5

If you think this is worth investigating, I will open a separate issue for this.

Thanks again for your help. 👍🏻

haijian-vaadin commented 4 years ago

Hi @markhm, sorry for the late reply and glad to hear that you migrated to a Polymer template-based implementation on your own in such a short time. 👍🏻

Thanks for sharing with us, yes, that's definitely sth we should investigate.

markhm commented 4 years ago

Closing this issue.