Charts-2.0 violates Vaadin architecture

Looks like Charts 2.0 add-on programming style does not follow basic Vaadin architecture rules. For example server side class Chart depends on classes from client side package com.vaadin.addon.charts.client.ui. There is no clean difference between the shared and client part of the add-on.

Basically any Vaadin installation should be able to run without vaadin-client(-compiled) jar file. Installing all vaadin files is problem on systems with limited resources like our embedded system.

Hi Robert,

You are correct, sir. I’ve filed a
ticket
for this issue.

I’m afraid that Vaadin Charts was never designed with embedded systems with very limited resources in mind. I gather that we’d need to split vaadin-charts.jar into three parts: charts.jar, charts-shared.jar and charts-client.jar for you to be able to deploy it on your system. Is this correct? Are there any other things blocking your deployment?

Thanks,
/Jonatan

Because most embedded systems with limited resources use some kind of repackiging into a new one-jar-module to avoid instalation of unused resources, it is enough (at least for our build system) to avoid code references between client and server classes.

Example - looking at the class Chart.java you can see following imports

import com.vaadin.addon.charts.client.ui.ChartClientRpc; import com.vaadin.addon.charts.client.ui.ChartConnector; import com.vaadin.addon.charts.client.ui.ChartServerRpc; import com.vaadin.addon.charts.client.ui.ChartState; import com.vaadin.addon.charts.client.ui.MouseEventDetails; Placing this classes into the (currently not existing) package com.vaadin.addon.charts.shared would allow us to exclude com.vaadin.addon.charts.client.* packages from our build script. If you split charts packages into 3 modules, it would be optimal for systems where original modules are directly deployed.

Got it, thanks. We’ll need to split the actually shared stuff into a shared package to ensure that client classes aren’t imported.

Thanks a lot for bringing this issue to my attention!
/Jonatan

Attached is the screenshot of my refactoring. I moved some classes to the shared package. Additionaly I had to create the shared ChartConstants class with the constant definitions originally located in the Chart class. Overall it took about 1 hour to refactor the Chart addon, not so much work.
17734.png

With the new version 2.1 we decided to give Vaadin Charts the last chance, because the ticket is now reported as fixed/released. Sorry Vaadin - unusable for systems with limited resources. There are still server side classes referencing classes in client package. Did you even test it?

Looks like Vaadin developers do not understand the basic Vaadin architecture. I has nothing to do with ‘designing an addon with embedded systems with limited resources in mind’. In Vaadin the client side is compiled into a GWT widgetset and the server side communicates with the widgetset via RPC. On clean designed Vaadin sites following the framework basics the vaadin-client.jar package do not have to be deployed because it’s content was compiled into JavaScript (widgetset). Let you try on your server to remove vaadin-client.jar and start any Charts App. You will end up with ClassNotFound error. Charts design needs vaadin-client.jar to be deployed on the server and because vaadin-client has it’s own dependencies you will probably end up with deployment of Jetty packages although you will never use them (even worse then Charts is the Touchkit - it’s a bunch of Java packages with client-server class references without any logical path).

My hint - do not keep in mind system resources, let you keep in mind Vaadin architecture. Then you will save resources automatically.

Hi,

I apologize for the misunderstanding. Could you provide a test case that would show the problem you are encountering? I tried with a simple new project that has the vaadin-client.jar as a provided dependency and added charts. Seems to work.

If you would like the client-side code of Charts to be a completely separate dependency, that would require some refactoring on our part. If that’s what you need, please let us know and I’ll create an ticket for it.