Grid<T>() -> val grid = Grid<T>(T::class.java, false)

[vlipovetskii]

First of all, my congratulations ! Karibu-dsl became "official" part of Vaadin kotlin support.

I tried to migrate my projects from 0.7.5 to 1.0.2 and met the error about duplicating of a bean property in BeanPropertySet.java.

Before Grid.kt leveraged default constructor of Grid in Grid.java init(Grid<T>()) { Now Grid.kt leverages another constructor val grid = Grid<T>(T::class.java, false) The effective difference in code of the constructors is

    public Grid(Class<T> beanType, boolean autoCreateColumns) {
        this();

        Objects.requireNonNull(beanType, "Bean type can't be null");
        this.beanType = beanType;
        propertySet = BeanPropertySet.get(beanType);
...

When one of my grids is created, the method

    public static <T> PropertySet<T> get(Class<? extends T> beanType) {
        Objects.requireNonNull(beanType, "Bean type cannot be null");
        InstanceKey key = new InstanceKey(beanType, false, 0, null);
        // Cache the reflection results
        return (PropertySet<T>) INSTANCES.computeIfAbsent(key,
                ignored -> new BeanPropertySet<>(key)).copy();
    }

raises error about duplicating BeanPropertry. I have not investigated yet, why beanProperty is treaated as duplicated. Maybe it's java/kotlin collision.

As a workaround: Is it acceptable to have in karibu-dsl two grid/treeGrid methods, which use different constructors (default and with BeanProperty support) ?

If no, I will do the methds with default constructor as part of my code.

[mvysny]

First of all, my congratulations ! Karibu-dsl became "official" part of Vaadin kotlin support.

Thank you so much :+1: :-)

raises error about duplicating BeanPropertry. I have not investigated yet, why beanProperty is treaated as duplicated. Maybe it's java/kotlin collision.

Interesting, it also sounds to me like like there could be a java/kotlin collision. Could you please paste a stack trace of the exception? It would be really good to have this documented, in case we revert back to using the parameterless Grid constructor.

Is it acceptable to have in karibu-dsl two grid/treeGrid methods, which use different constructors (default and with BeanProperty support) ?

That would be acceptable (or even better, the grid() function could have a parameter, something like 'passItemClass` defaulting to true. However, before we jump to this solution, could we perhaps try to solve the "duplicated bean property" issue? It would be really good to have this documented.

[mvysny]

Just to document why the change was made: having the item class will allow us two things:

1. to reference bean properties as Strings when adding columns;
2. to auto-create binder for the bean when editing a row.
3. I wonder if there are additional benefits?

However, both use-cases are not really that important:

1. Instead of calling addColumn("address") you should use addColumnFor(KProperty) anyway, to benefit from a type-safe system.
2. The Binder which is created automatically doesn't honour JSR303 validation annotations, so it's good practice to set the editor binder manually anyway.

So, all in all, I'm open to reverting the Grid construction code back to zero-arg constructor; but first I wonder whether we can do something about that duplicated bean property.

[vlipovetskii]

I have tried to migrate my project to investigate the issue. Unfortunately, a new issue appeared. I have a generic class with generic parameter for Grid Row. New grid and treeGrid functions leverage reified generic parameter, and thus my class can't be leveraged with the new grid and treeGrid. I don't leverage Binder. I will create (temporarily) in my code, copies of your grid/treeGrid functions (see fragment below) without reified and with default constructor to migrate to karibu-dsl 1.x.

BTW, Why not to have two implementations of grid/treeGrid (your new implementations and functions like below, but with more pretty names) ?

@VaadinDsl
fun <T : Any?> (@VaadinDsl HasComponents).grid1(dataProvider: DataProvider<T, *>? = null, block: (@VaadinDsl Grid<T>).() -> Unit = {}) =
        init(Grid<T>()) {
            if (dataProvider != null) this.dataProvider = dataProvider
            block()
        }

@VaadinDsl
fun <T : Any?> (@VaadinDsl HasComponents).treeGrid1(dataProvider: HierarchicalDataProvider<T, SerializablePredicate<T>>? = null, block: (@VaadinDsl TreeGrid<T>).() -> Unit = {}) =
        init(TreeGrid<T>()) {
            if (dataProvider != null) this.dataProvider = dataProvider
            block()
        }

[mvysny]

BTW, Why not to have two implementations of grid/treeGrid (your new implementations and functions like below, but with more pretty names) ?

I so far have failed to come up with a proper naming with the alternate functions. The problem is that the advantages of passing the item Class to Grid are very minor; I fail to see the major differentiator which could then be used in the function name.

Because of that, I'm actually toying with the idea of reverting the code back to use the zero-arg constructor (or having additional parameter in the grid function). However, first I'd like to get to the bottom of that duplicate BeanProperty issue. Could you please paste a stacktrace here, just for the documentation purposes?

[mvysny]

I have a generic class with generic parameter for Grid Row. New grid and treeGrid functions leverage reified generic parameter, and thus my class can't be leveraged with the new grid and treeGrid.

Oh - I've missed this important piece. Actually this works for me: the following code compiles happily with Kotlin 1.4.0:

data class TestingClass<T>(var item: T? = null)
UI.getCurrent().grid<TestingClass<String>>()

[mvysny]

Regarding the duplicite bean property: this PR sounds like it fixes that very issue in Vaadin 8: https://github.com/vaadin/framework/pull/12091

Perhaps we should introduce a similar PR into Vaadin 14 as well.

[mvysny]

By fixing #28 I've realized that, by using those new functions, we will be able to pass in null class as follows:

UI.getCurrent().grid<Person>(klass = null)

or

UI.getCurrent().grid<Person>(clazz = null)

@vlipovetskii what do you think? Would this work for you please?

[vlipovetskii]

Yes, if to pass null and leverage Grid constructor without beanType parameter in the case of null ?

[mvysny]

Yup, that's exactly the idea. I'll add a docu regarding this shortly.

[mvysny]

Please see https://github.com/mvysny/karibu-dsl/tree/master/karibu-dsl#grid for more details; the code is merged in Karibu-DSL master (but hasn't been released yet).