vaadin / flow

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

DataProvider Improvements #8054

Open pleku opened 4 years ago

pleku commented 4 years ago

This is an epic for looking at various planned improvements for DataProvider. The list below may change and there are no guarantees on what gets done at this point, but we are looking into these things.

knoobie commented 4 years ago

@pleku I wasn't sure where to put this, so it landed in the epic - I was looking at the latest pr (https://github.com/vaadin/flow/pull/8550) and was wondering why some methods return the instance of the DataView instead of a simple boolean (like java.util.List does as example) - was there chaining of operation in mind? I can see why some would chain addFilter, but I can't really wrap my head around add/remove item methods, especially because you have operation that accept a single item or a list of items. Should the developer do something with the returning instance, save / overwrite his old instance(local variable) of it - that shouldn't be needed because you call refresh() on the underlying dataProvider?

pleku commented 4 years ago

@knoobie I have to admit we had not though about just too much - you're probably right that there is not much chaining to be done at add/remove operations. But I have to admit I'm not sure what the boolean is useful for in this case too, but it would be of course with on line with Collection API. But also the case for returning false simple add(newItem) method would be quite weird scenario and if we return boolean one might start to think about this :) (cc: @mshabarov )

knoobie commented 4 years ago

For example, if we look at the pr mentioned above - you can see that all methods have some kind of "error / fall back" handling (just return this) if the item isn't in the list or if one item is already in the list. This matches 100% with the javadoc of Collection::add as example Returns true if this collection changed as a result of the call. (Returns false if this collection does not permit duplicates and already contains the specified element.) -> Vaadin's DataProvider doesn't permit duplicates. You could even align with the exceptions that Collection::add uses. (I'm not a fan of exceptions, but this would also align with the Java method)

I'm a big fan of "don't reinvent the wheel" especially when there is a "proven" Java way of doing things all your users already know and feel "home" :)

You normally don't check what Collection::add returns, but if you got a bug or it's a critical application - you start to debug those cases or you go with something like this:

if(collection.add(myItem)) 
  Notification.show("All Good");
else
  Notification.show("Could not add item"); 

cc: @mshabarov because it was his pr ;)

pleku commented 4 years ago

The following enhancements are now to be shipped in Vaadin 17 and later on (probably) backported to Vaadin 14 LTS.

8052 Unknown size for data provider / optional size query

4510 Expose fetched data from data provider

8189 No need to learn about data provider concept to use a component / Easier creation of data providers directly from component API

vaadin/spring#388 Support for Spring Data repositories