Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Opening grid editor fails when a non-edited ComponentRenderer column returns null for that row #19629

Open
guttormvik2 opened this issue Jun 24, 2024 · 1 comment

Comments

@guttormvik2
Copy link

Describe the bug

I have a ComponentRenderer column that returns either a VaadinIcon.CLOCK.create(), a Div with some text, or null.
When I click on a row (which also opens the editor) where this has value, everything is fine.
When I click on a row where this is null, I get

13:43:55,264 ERROR [com.ptsmc.vaadin.CustomErrorHandler] (default task-26) Logged error: java.lang.NullPointerException: Cannot invoke "com.vaadin.flow.component.Component.getElement()" because "recreatedComponent" is null
    at deployment.ptsmc.ear//com.vaadin.flow.data.provider.AbstractComponentDataGenerator.refreshData(AbstractComponentDataGenerator.java:51)
    at deployment.ptsmc.ear//com.vaadin.flow.data.provider.CompositeDataGenerator.lambda$refreshData$2(CompositeDataGenerator.java:62)
    at java.base/java.lang.Iterable.forEach(Iterable.java:75)
    at deployment.ptsmc.ear//com.vaadin.flow.data.provider.CompositeDataGenerator.refreshData(CompositeDataGenerator.java:62)
    at deployment.ptsmc.ear//com.vaadin.flow.data.provider.CompositeDataGenerator.lambda$refreshData$2(CompositeDataGenerator.java:62)
    at java.base/java.lang.Iterable.forEach(Iterable.java:75)
    at deployment.ptsmc.ear//com.vaadin.flow.data.provider.CompositeDataGenerator.refreshData(CompositeDataGenerator.java:62)
    at deployment.ptsmc.ear//com.vaadin.flow.data.provider.DataCommunicator.refresh(DataCommunicator.java:405)
    at deployment.ptsmc.ear//com.vaadin.flow.component.grid.Grid$AbstractGridExtension.refresh(Grid.java:1213)
    at deployment.ptsmc.ear//com.vaadin.flow.component.grid.editor.EditorImpl.requestEditItem(EditorImpl.java:152)
    at deployment.ptsmc.ear//com.vaadin.flow.component.grid.editor.EditorImpl.lambda$editItem$2d5ea2ce$1(EditorImpl.java:137)
    at deployment.ptsmc.ear//com.vaadin.flow.internal.StateTree.lambda$runExecutionsBeforeClientResponse$2(StateTree.java:397)
    at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183)
    at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:179)
    at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1625)
    at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
    at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
    at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
    at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
    at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
    at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
    at deployment.ptsmc.ear//com.vaadin.flow.internal.StateTree.runExecutionsBeforeClientResponse(StateTree.java:392)
    at deployment.ptsmc.ear//com.vaadin.flow.server.communication.UidlWriter.encodeChanges(UidlWriter.java:394)
    at deployment.ptsmc.ear//com.vaadin.flow.server.communication.UidlWriter.createUidl(UidlWriter.java:170)
    at deployment.ptsmc.ear//com.vaadin.flow.server.communication.UidlRequestHandler.createUidl(UidlRequestHandler.java:155)
    at deployment.ptsmc.ear//com.vaadin.flow.server.communication.UidlRequestHandler.writeUidl(UidlRequestHandler.java:145)
    at deployment.ptsmc.ear//com.vaadin.flow.server.communication.UidlRequestHandler.synchronizedHandleRequest(UidlRequestHandler.java:115)
    at deployment.ptsmc.ear//com.vaadin.flow.server.SynchronizedRequestHandler.handleRequest(SynchronizedRequestHandler.java:40)
    at deployment.ptsmc.ear//com.vaadin.flow.server.VaadinService.handleRequest(VaadinService.java:1574)
    at deployment.ptsmc.ear//com.vaadin.flow.server.VaadinServlet.service(VaadinServlet.java:398)
    at jakarta.servlet.api@6.0.0//jakarta.servlet.http.HttpServlet.service(HttpServlet.java:614)
    ...

When I change the ComponentRenderer to return an empty Div instead of null, it works.

Expected-behavior

No response

Reproduction

See description at top.
I assume this is simply missing null-test and that the stacktrace is enough?
If not I can try to produce an example

System Info

Windows 10, Vaadin 24.3.12, Firefox 127.0

@TatuLund TatuLund transferred this issue from vaadin/platform Jun 24, 2024
@TatuLund
Copy link
Contributor

TatuLund commented Jun 24, 2024

Originally it was requirement that ComponentRenderer must not return null, but it was lifted by this PR: https://github.com/vaadin/flow-components/pull/4361/files

However it seems not to be complete.

Similar fix is needed in here: https://github.com/vaadin/flow/blob/main/flow-data/src/main/java/com/vaadin/flow/data/provider/AbstractComponentDataGenerator.java#L49

Same approach could be used,

if (recreatedComponent == null) {
   recreatedComponent = new Text("");
}

And if the fix is added in AbstractComponentDataGenerator, I would assume the fix in ComponentDataGenerator becomes redundant.

You seem to have found a corner case where flow is not going via ComponentDataGenerator ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🔖 Normal Priority (P2)
Development

No branches or pull requests

3 participants