Grid fails to update with new data after initial page load
archiecobbs opened this issue · 31 comments
Description of the bug
On the initial page load, a Vaadin application running under Tomcat displays an empty Grid and starts a background task to do an initial load of the data. In the demonstration project, this background task takes one second.
After th one second delay, the data is available and the DataProvider signals refreshAll()
. The Grid updates on the server but does not send any update to the client. So from the client perspective, it appears as if the load is taking forever. The user is staring at an empty table indefinitely. But really there is just a missed update somehow.
Once some other unrelated event triggers the server to send an update to the browser, then the client properly synchronizes and the problem resolves itself. But this could take an arbitrarily long time.
Expected behavior
The client display should update immediately when the Grid receives new data on the server. There is a missing update.
Minimal reproducible example
https://github.com/archiecobbs/vaadin-grid-load-fail-issue
Note this demo shows the status of the load operation via a spinner/progress widget. The spinner widget should spin for exactly one second after the page loads. By watching the log in the console you can see that after the initial page load it only spins for one second on the server, but on the client it spins indefinitely.
Here is the Firefox error log. There are a bunch of errors, some probably unrelated from other tabs.
'http://localhost:8080/': Load failed with status 404 for script 'http://localhost:8080/sw.js'.
Note in this example the page load occurred at time 16:56:16 but the grid didn't update on the browser until 16:56:49.
Versions
- Vaadin / Flow version: 24.4.12
- Java version: 17
- OS version: Mac OS 14.6.1
- Browser version: Firefox 130.0.1
- Application Server: Tomcat 10.1.28
Notes:
- This bug does not occur in Vaadin 23
- Could possibly be related to #16775? (just a wild guess, it also occurs in 24 but not 23)
Just tested the linked project locally and cannot reproduce.
At first load, the spinner disappears after one second and the data in the grid is loaded and visible in the browser.
Tested with both Chrome and Firefox; Apache Tomcat/10.1.30
The only error in the browser console is a 404 for the favicon.
One thing to notice in your screenshot is that the Atmosphere version logged in the PUSH connection URL seems old: X-Atmosphere-Framework=2.2.13-vaadin5-javascript
Sorry - that log messages is from a different browser tab running an older Vaadin application (note the URL path is pcom
instead of example
).
My reproducer test case is not 100% reliable. This morning for some reason I can't reproduce the bug whereas yesterday I could do it reliably. Bleh.
Here's another data point... on the real application, using Chrome, when the problem occurs I see this error in the JS console:
Also, is this error relevant?
Failed to register/update a ServiceWorker for scope ‘http://localhost:8080/’: Load failed with status 404 for script ‘http://localhost:8080/sw.js’.
I note that http://localhost:8080/sw.js
does not exist but http://localhost:8080/example/example/sw.js
does exist - so maybe Vaadin is looking for this file in the wrong location?
I note that http://localhost:8080/sw.js does not exist but http://localhost:8080/example/example/sw.js does exist - so maybe Vaadin is looking for this file in the wrong location?
I added the PWA annotation to the linked project and I locally I can see the service worker is correctly fetched from http://localhost:8080/example/example/sw.js.
So, no idea why it is failing for you.
Could you trace who is the initiator of the request to http://localhost:8080/sw.js?
Hmm, I think there may be some issue relating to Firefox with multiple tabs being open. When I get rid of all other tabs I can't reproduce the problem on Firefox.
I am going to close this issue for now until I can more reliably reproduce it.
Thanks.
OK I can reproduce the problem more reliably now. This is using Chrome instead of Firefox, and with only one tab open.
However, it seems that a required part of the puzzle is the reverse proxy we use for the application. Our Tomcat instance sits behind an Apache server. Note we've been doing this with Vaadin for 10+ years and this particular problem has not occurred, so something that changed between 23 and 24 is making Vaadin less robust in this configuration.
In the example below, there is a 10 second delay from 12:02:53 to 12:03:02 during which the browser is still displaying the loading indicator even though the load is completed on the server.
Here's what we see in the Chrome browser:
Notes:
- On the second line where it says
Invoking executeWebSocket, using URL:
notice the URL is malformed - theContent-Type
URL parameter value is not URL-encoded (!) - this seems like an obvious thing to investigate (EDIT: on further review, this is probably just how Chrome displays it and not a bug) - The browser display is wrong until "Websocket failed on first connection attempt. Downgrading to long-polling and resending" when it synchronizes and properly shows the data in the table
Here's what we see in the server logs:
2024-09-27 12:02:51,036 [http-nio-8080-exec-1] DEBUG: Properties loaded from classpath.
2024-09-27 12:02:51,036 [http-nio-8080-exec-1] INFO : Vaadin is running in production mode.
2024-09-27 12:02:51,066 [http-nio-8080-exec-1] DEBUG: Using 'com.vaadin.flow.server.communication.IndexHtmlRequestHandler' in client mode bootstrapping
2024-09-27 12:02:51,072 [http-nio-8080-exec-1] DEBUG: Using mapping /example/* from servlet VaadinServlet as the frontend servlet because this was the first deployed VaadinServlet
2024-09-27 12:02:51,807 [http-nio-8080-exec-4] DEBUG: Invalid request state.
2024-09-27 12:02:51,915 [http-nio-8080-exec-5] INFO : determined URI https://ops2.patientexp.com/site/debug/example/example/
2024-09-27 12:02:51,915 [http-nio-8080-exec-5] DEBUG: attaching MainRoot[id=1]
2024-09-27 12:02:52,018 [http-nio-8080-exec-5] DEBUG: connecting com.example.gui.ExampleDataProvider@72022d4d
2024-09-27 12:02:52,022 [http-nio-8080-exec-5] DEBUG: triggered reload, session=com.vaadin.flow.server.VaadinSession@3410e13c
2024-09-27 12:02:52,022 [primaryTaskExecutor-1] DEBUG: reload sleeping for 1000ms...
2024-09-27 12:02:52,024 [http-nio-8080-exec-5] INFO : spinner: got STARTED -> start spinning
2024-09-27 12:02:53,082 [primaryTaskExecutor-1] DEBUG: reload wokeup after 1000ms...
2024-09-27 12:02:53,089 [primaryTaskExecutor-1] DEBUG: updating data provider, session=com.vaadin.flow.server.VaadinSession@3410e13c
2024-09-27 12:02:53,094 [primaryTaskExecutor-1] DEBUG: refreshAll() invoked
2024-09-27 12:02:53,097 [primaryTaskExecutor-1] DEBUG: done updating data provider...
2024-09-27 12:02:53,098 [primaryTaskExecutor-1] INFO : spinner: got COMPLETED -> stop spinning
2024-09-27 12:02:57,014 [http-nio-8080-exec-7] DEBUG: Invalid request state.
2024-09-27 12:03:02,625 [http-nio-8080-exec-1] DEBUG: Sending cached message [com.vaadin.flow.server.communication.AtmospherePushConnection$PushMessage@7f54ae4f] to 6f6b8b39-b8e2-4c1b-bbe8-7905ad88de13
Regarding the request that failed at time 12:02:5:
Here's the corresponding 501
error logged by Apache:
[IPAddressRedacted] - archie [27/Sep/2024:12:02:51 -0500] "GET /site/debug/example/example/VAADIN/push?v-r=push&v-uiId=0&v-pushId=0fded26c-629f-4756-8060-1ae0e7412707&X-Atmosphere-tracking-id=0&X-Atmosphere-Framework=3.1.2-javascript&X-Atmosphere-Transport=websocket&X-Atmosphere-TrackMessageSize=true&Content-Type=application/json;%20charset=UTF-8&X-atmo-protocol=true&X-Vaadin-LastSeenServerSyncId=0 HTTP/1.1" 501 - "-" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/129.0.0.0 Safari/537.36"
That error was forwarded from Tomcat. Here's is the 501
error logged by Tomcat (which originated from Vaadin):
0:0:0:0:0:0:0:1 - - [27/Sep/2024:12:02:51 -0500] "GET /example/example/VAADIN/push?v-r=push&v-uiId=0&v-pushId=0fded26c-629f-4756-8060-1ae0e7412707&X-Atmosphere-tracking-id=0&X-Atmosphere-Framework=3.1.2-javascript&X-Atmosphere-Transport=websocket&X-Atmosphere-TrackMessageSize=true&Content-Type=application/json;%20charset=UTF-8&X-atmo-protocol=true&X-Vaadin-LastSeenServerSyncId=0 HTTP/1.1" 501 5
Presumably that 501
error corresponded to the Invalid request state
Vaadin server error (quoted in previous comment):
2024-09-27 12:02:51,807 [http-nio-8080-exec-4] DEBUG: Invalid request state.
About the Content-Type parameter, nothing has changed in Flow recently.
I can reproduce the issue with Apache HTTPD reverse proxy, but everything works fine connecting directly to tomcat and also with nginx as reverse proxy.
Debugging the connection request, I couldn't detect any errors on the Flow side: the atmosphere handler completes its job correctly. Also, on Apache access log I see a 200 status code for the websocket connection request.
But still there's the error logged in the browser console and the reconnection attempt with long-polling transport.
BTW, I was unable to find any log with "Invalid request state" text from Vaadin. Could you please enable the category on the log system configuration or point out which component is producing that message?
So, it looks like to me that the following configuration (that used to work for me in the past) is not performing the connection upgrade
RewriteEngine on
RewriteCond %{HTTP:Upgrade} websocket [NC]
RewriteCond %{HTTP:Connection} upgrade [NC]
RewriteRule ^/?(.*) "ws://vaadin:8080/example/example/$1" [P,L]
ProxyPass http://vaadin:8080/example/example/
ProxyPassReverse http://vaadin:8080/example/example/
But I was able to make it work by using the upgrade=websocket
flag in the ProxyPass
directive, as mentioned in the docs
ProxyPass http://vaadin:8080/example/example/ upgrade=websocket
ProxyPassReverse http://vaadin:8080/example/example/
Here's the link to the Apache docs example: https://httpd.apache.org/docs/2.4/mod/mod_proxy.html#wsupgrade
I hope this will help
Thanks very much for your help.
Status update: I updated my Apache reverse proxy config to use the new upgrade=websocket
flag and that fixed my test case, but I'm still seeing the problem in the real application (even after making that same change there).
I will continue to try to get a reproducer test case.
In the meantime, can you determine whether this error is meaningful? I see this error in Chrome when the bug occurs in the real application, and I don't see this in the test case (which as mentioned is now working properly):
This error appears to be coming from this code in Grid.java
.
Maybe @yuriy-fix or @web-padawan can provide some advice on how to debug this error with Grid
I tested the attached app but was unable to reproduce Cannot read properties of undefined (reading 'getByNodeId')
:
Screen.Recording.2024-10-08.at.16.27.52.mov
Unfortunately, the stacktrace alone isn't enough to determine the cause – a reproducible example is needed.
Hi @vursen,
Thanks. I'm also unable to reproduce the problem using my test case. However, it still occurs reliably in the actual application. I'm not sure what difference is the key. I am continuing to try to get the test case to also reproduce the problem.
I am not familiar with Vaadin client-side debugging. Can someone provide me with simple instructions for how to properly debug the getByNodeId
error? I am happy to try them using my actual application, which reliably reproduces the problem.
Thanks.
Here is a video of the problem occurring in the real application.
Notes:
- Using Chrome browser Version 129.0.6668.90 (Official Build) (arm64)
- This is not going through the reverse proxy (direct connection to tomcat on localhost)
- The
getByNodeId
error happens when the grid is first loaded, but then when reloading the browser tab the error does not occur and the data appears - There is an associated misalignment between column headers and columns; unlike the
getByNodeId
problem, this problem persists after the reload
VaadinGridBugVideo.mp4
There is one thing that looks fishy in the mentioned code fragment. It checks if the column is attached to a UI using column.getUI()
but gets the app id using e.getUI()
. It seems like the latter could potentially reference an outdated UI if the column got somehow reattached to a different UI between the attach event and beforeClientResponse
.
However, a reproduction is still required to confirm this. Could probably be reproducible under similar conditions as those described in vaadin/flow-components#5493
There is one thing that looks fishy in the mentioned code fragment.
I played with that in my application and didn't see any problems. Logging e.getUI()
vs. column.getUI()
did not reveal any discrepancy.
I learned how to set breakpoints in Chrome though :) Here's where the bug happens. Note that that i = ""
which looks suspicious... ?
I learned how to set breakpoints in Chrome though :) Here's where the bug happens. Note that that i = "" which looks suspicious... ?
The stacktrace suggests that you are using ComponentRenderer in your grid, which isn't present in the test app. This might be a potential reason why the issue isn't reproducible there. Unfortunately, I don't have specific instructions for debugging this issue. You might try gradually adding stuff from your actual grid to the test app until the issue becomes reproducible.
I've reproduced it with my test program. The problem is here in ComponentRenderer.java
:
@Override
protected String getTemplateExpression() {
var appId = UI.getCurrent() != null
? UI.getCurrent().getInternals().getAppId()
: "";
return "${Vaadin.FlowComponentHost.getNode('" + appId
+ "', item.nodeid)}";
}
The problem is that this method sets appId
to empty string if UI.getCurrent()
returns null. But that's wrong and it causes the Grid load to fail. This method should not assume UI.getCurrent()
will return non-null.
Here's a sample stack trace:
2024-10-13 11:28:47,992 [Thread-816] INFO : **** COMPONENT RENDERER: getTemplateExpression(): UI.getCurrent()=null appId=""
java.lang.Throwable: HERE
at com.vaadin.flow.data.renderer.ComponentRenderer.getTemplateExpression(ComponentRenderer.java:152) ~[classes/:?]
at com.vaadin.flow.data.renderer.LitRenderer.lambda$createJsRendererFunction$7e58a72d$1(LitRenderer.java:223) ~[vaadin-renderer-flow-24.4.12.jar:?]
at com.vaadin.flow.dom.Element$1.execute(Element.java:1254) ~[flow-server-24.4.8.jar:24.4.8]
at java.base/java.util.ArrayList.forEach(ArrayList.java:1597) ~[?:?]
at com.vaadin.flow.internal.StateNode.fireAttachListeners(StateNode.java:931) ~[flow-server-24.4.8.jar:24.4.8]
at com.vaadin.flow.internal.StateNode.onAttach(StateNode.java:335) ~[flow-server-24.4.8.jar:24.4.8]
at com.vaadin.flow.internal.StateNode.setParent(StateNode.java:287) ~[flow-server-24.4.8.jar:24.4.8]
at com.vaadin.flow.internal.nodefeature.NodeFeature.attachPotentialChild(NodeFeature.java:80) ~[flow-server-24.4.8.jar:24.4.8]
at com.vaadin.flow.internal.nodefeature.StateNodeNodeList.add(StateNodeNodeList.java:55) ~[flow-server-24.4.8.jar:24.4.8]
at com.vaadin.flow.internal.nodefeature.ElementChildrenList.add(ElementChildrenList.java:44) ~[flow-server-24.4.8.jar:24.4.8]
at com.vaadin.flow.dom.impl.AbstractNodeStateProvider.insertChild(AbstractNodeStateProvider.java:107) ~[flow-server-24.4.8.jar:24.4.8]
at com.vaadin.flow.dom.Node.insertChild(Node.java:391) ~[flow-server-24.4.8.jar:24.4.8]
at com.vaadin.flow.dom.Node.appendChild(Node.java:163) ~[flow-server-24.4.8.jar:24.4.8]
at com.vaadin.flow.dom.Node.appendChild(Node.java:147) ~[flow-server-24.4.8.jar:24.4.8]
at com.vaadin.flow.component.HasComponents.lambda$add$1(HasComponents.java:75) ~[flow-server-24.4.8.jar:24.4.8]
at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:184) ~[?:?]
at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:215) ~[?:?]
at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:215) ~[?:?]
at java.base/java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:1024) ~[?:?]
at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:570) ~[?:?]
at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:560) ~[?:?]
at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151) ~[?:?]
at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174) ~[?:?]
at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:265) ~[?:?]
at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:636) ~[?:?]
at com.vaadin.flow.component.HasComponents.add(HasComponents.java:75) ~[flow-server-24.4.8.jar:24.4.8]
at com.vaadin.flow.component.HasComponents.add(HasComponents.java:58) ~[flow-server-24.4.8.jar:24.4.8]
at com.example.gui.AbstractRoot.updateLowerPanel(AbstractRoot.java:257) ~[classes/:?]
at com.vaadin.flow.server.VaadinSession.accessSynchronously(VaadinSession.java:982) ~[flow-server-24.4.8.jar:24.4.8]
at com.example.gui.MainRoot.lambda$1(MainRoot.java:41) ~[classes/:?]
at java.base/java.lang.Thread.run(Thread.java:1575) [?:?]
This bug is reminds me of vaadin/flow-components#4204 which had the same underlying cause - someone mistakenly assuming that there will be a current UI
when that's not at all guaranteed.
With enterprise applications, there are always things happening in random background threads, and these background threads are going to report their updates back to the user interface via VaadinSession.access()
, where "reporting updates" means invoking virtually any Vaadin method. Therefore, every Vaadin method (except for those specially documented as such) should be written to work properly without a current UI
.
It's easy to find other suspicious examples where it looks like we are doing something that seems important when there is a current UI
, but when there's not, we're just going about our business and pretending it wasn't really necessary after all...
LitRenderer.java-79- private LitRenderer(String templateExpression) {
LitRenderer.java-80- this.templateExpression = templateExpression;
LitRenderer.java-81-
LitRenderer.java-82- int litRendererCount = 0;
LitRenderer.java:83: if (UI.getCurrent() != null) {
LitRenderer.java-84- // Generate a unique (in scope of the UI) namespace for the renderer
LitRenderer.java-85- // properties.
LitRenderer.java-86- litRendererCount = UI.getCurrent().getElement()
LitRenderer.java-87- .getProperty("__litRendererCount", 0);
LitRenderer.java-88- UI.getCurrent().getElement().setProperty("__litRendererCount",
LitRenderer.java-89- litRendererCount + 1);
LitRenderer.java-90-
LitRenderer.java-91- }
LitRenderer.java-92- propertyNamespace = "lr_" + litRendererCount + "_";
LitRenderer.java-93- }
Spreadsheet.java-1180- private void updateAppId() {
Spreadsheet.java:1181: Optional.ofNullable(UI.getCurrent()).ifPresent(ui -> {
Spreadsheet.java-1182- getElement().setProperty("appId", ui.getInternals().getAppId());
Spreadsheet.java-1183- });
Spreadsheet.java-1184- }
CellValueManager.java-128- public CellValueManager(Spreadsheet spreadsheet) {
CellValueManager.java-129- this.spreadsheet = spreadsheet;
CellValueManager.java-130-
CellValueManager.java:131: UI current = UI.getCurrent();
CellValueManager.java-132- if (current != null) {
CellValueManager.java-133- updateLocale(current.getLocale());
CellValueManager.java-134- }
CellValueManager.java-135- }
ChartOptions.java-48- private void updateOptions() {
ChartOptions.java:49: UI ui = UI.getCurrent();
ChartOptions.java-50-
ChartOptions.java-51- if (ui == null) {
ChartOptions.java-52- return;
ChartOptions.java-53- }
ChartOptions.java-54-
ChartOptions.java-55- JsonObject configurationNode = getJsonFactory()
ChartOptions.java-56- .parse(ChartSerialization.toJSON(this));
ChartOptions.java-57- ui.getElement().executeJs(
ChartOptions.java-58- "customElements.get('vaadin-chart').__callHighchartsFunction('setOptions',$0,$1)",
ChartOptions.java-59- true, configurationNode);
ChartOptions.java-60- }
VirtualList.java:258: var appId = UI.getCurrent() != null
VirtualList.java-259- ? UI.getCurrent().getInternals().getAppId()
VirtualList.java-260- : "";
VirtualList.java-261- getElement().callJsFunction("$connector.setPlaceholderItem", json,
VirtualList.java-262- appId);
I'm not saying all of the above are definitely bugs, just that they look suspicious.
But wait, there's more...
To make things worse, if a background thread uses VaadinUtil.access()
to report its updates, then whether a UI
is in context when the update executes is random - it happens if:
- The
VaadinSession
is currently locked already by some other thread, and - There happens to be a current
UI
inCurrentInstance
when that other thread unlocks the session
This is why I had so much trouble getting my test case to reproduce the bug.
This is because VaadinService.runPendingAccessTasks()
has a "leak" in which it sets the CurrentInstance
for the VaadinSession
so that the task can execute, but it does not clear the CurrentInstance
for the UI
, VaadinRequest
, or VaadinResponse
from the previous task:
// Dump all current instances, not only the ones dumped by setCurrent
Map<Class<?>, CurrentInstance> oldInstances = CurrentInstance
.getInstances();
CurrentInstance.setCurrent(session);
try {
...
} finally {
CurrentInstance.clearAll();
CurrentInstance.restoreInstances(oldInstances);
}
To be clear, the problem is that CurrentInstance.setCurrent(session)
does not cleari the current UI
, VaadinRequest
, and VaadinResponse
before executing the next task in the queue, even though the tasks in the queue (in general) have nothing at all to do with whatever UI
, VaadinRequest
, and VaadinResponse
happen to be lying around from before.
So I believe this patch should also be applied to fix this cross-thread pollution:
--- a/flow-server/src/main/java/com/vaadin/flow/server/VaadinService.java
+++ b/flow-server/src/main/java/com/vaadin/flow/server/VaadinService.java
@@ -2125,11 +2125,12 @@ public abstract class VaadinService implements Serializable {
// Dump all current instances, not only the ones dumped by setCurrent
Map<Class<?>, CurrentInstance> oldInstances = CurrentInstance
.getInstances();
- CurrentInstance.setCurrent(session);
try {
while ((pendingAccess = session.getPendingAccessQueue()
.poll()) != null) {
if (!pendingAccess.isCancelled()) {
+ CurrentInstance.clearAll(session);
+ CurrentInstance.setCurrent(session);
pendingAccess.run();
try {
But when you do, watch out, other similar bugs may come crawling out ... :)
@archiecobbs Thank you for investigating. I discussed with @mshabarov, and the absence of UI in ComponentRenderer
when using session.accessSynchronously
looks very much like a Flow bug.
Here is a simplified example demonstrating the problem:
@Push // <----- required
public class Application implements AppShellConfigurator {
@Route("grid")
public class GridView extends Div {
public GridView() {
Button addGrid = new Button("Add grid", (event) -> {
final VaadinSession session = VaadinSession.getCurrent();
new Thread(() -> session.accessSynchronously(this::render)).start();
});
add(addGrid);
}
public void render() {
Grid<String> grid = new Grid<>();
grid.addColumn(new ComponentRenderer<Span, String>(item -> new Span(item)));
grid.setItems("Item 1", "Item 2", "Item 3", "Item 4", "Item 5");
add(grid);
}
}
Steps: Click on the button and observe Cannot read properties of undefined (reading 'getByNodeId')
in the browser console.
The error originates from the LitRenderer's attach listener, inside which UI.getCurrent()
unexpectedly returns null
:
The error doesn't happen when using ui.accessSynchronously
instead – the UI is available, and the LitRenderer flow proceeds normally.
I'm transferring the issue to the Flow team.
Hi @vursen,
Thanks for the additional detail.
Just to be clear, I believe there are two separate bugs here:
- The original bug where
UI.getCurrent()
unexpectedly returnsnull
and the Grid fails to display properly - The bug where
VaadinService.runPendingAccessTasks()
fails to null out any leftoverUI
,VaadinRequest
, andVaadinResponse
in theCurrentInstance
before executing the next task
Let me know if I should create a separate issue for the second bug.
Thanks.
Let me know if I should create a separate issue for the second bug.
Bleh! That bug already exists in an ignored state for over five years.
Vaadin 23 LitRenderer
don't use UI.getCurrent()
but it has UI instance which it gets in other way. In Vaadin 24, UI.getCurrent()
is used in LitRenderer
constructor and ComponentRenderer#getTemplateExpression()
. Latter one is most likely culprit for the error as mentioned before.
@vursen Is there something in Flow preventing refactoring ComponentRenderer
to not rely on UI.getCurrent()
call and pass UI instance some other way like via attach event where UI should be available?
e.g. ComponentRenderer#getTemplateExpression()
could have a sibling ComponentRenderer#getTemplateExpression(UI)
.
LitRenderer
constructor's code relying on UI.getCurrent()
could also have alternative that takes in UI instance if code can't be postponed to attach event. Or at minimum LitRenderer#of(String)
could be documented to mention that it requires active UI and if used in external threads, set it with UI#setCurrent(UI)
.
Workaround is to set UI instance with UI#setCurrent(UI)
for threads using these.
It seems we are already equipped with everything necessary to refactor LitRenderer. We can replace the current UI.getCurrent()
calls with something like UI ui = ((StateTree) container.getNode().getOwner()).getUI()
. As for LitRenderer, we'll need to investigate whether it's possible to delay the computation of propertyNamespace
until the container is attached. If not, we should document that LitRenderer only supports instantiation in contexts with a UI.
This is probably obvious already, but while you guys are looking at this it's probably worth reviewing the other examples quoted in this earlier comment in case they suffer from the same problem. Thanks.
Vaadin 23 LitRenderer don't use UI.getCurrent() but it has UI instance which it gets in other way.
I can confirm that the issue with specifically ComponentRenderer is a regression introduced by vaadin/flow-components#4148. I've created a separate issue for this and added it to the backlog.
This is probably obvious already, but while you guys are looking at this it's probably worth reviewing the other examples
And I've also opened a separate ticket about refactoring other mentioned places to avoid UI.getCurrent
(or otherwise document properly). As there seem to be no other things, I'm closing this issue in favour of those two. Please let me know if I missed anything.
As there seem to be no other things, I'm closing this issue in favour of those two. Please let me know if I missed anything.
Looks good - thanks!