-
-
Notifications
You must be signed in to change notification settings - Fork 759
Browser can't load page when table rows contain lists #2744
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
Browser can't load page when table rows contain lists #2744
Comments
I'm running into this today. Thank you for creating this - I was beginning to wonder if I was sane. 😄 I don't have a great idea of how I might go about debugging this, but I'm interested in talking it through a bit. I can see that there's high CPU usage for a period of time after the Python code has apparently stopped doing anything (I assume it has at least sent the data to the Is there a reasonably easy way to enable some kind of debugging logic on the JS side, so that I can try to see what messages are being received by the client? In the meantime, it appears that a known workaround is converting the lists into strings before they're sent across - presumably, Quasar/Vue support JavaScript objects like arrays by doing JSON.stringify at some point? I wonder how much of an actual change to nicegui behavior it would be if |
I think I tried adding some console logging in JavaScript, but without much success, since everything hangs when the problem occurs. Somewhere in nicegui.js is probably a good place to start though. Looking forward to your findings! 🤞🏻 |
I believe I have just run into this too. I was updating a ui.table dynamically, and the browser tab seems to freeze immediately with the update. Opening a new browser window onto the same backend after the dynamic update again immediately freezes the tab. For that was on macOS with Chrome. I was unable to get much out of the browser developer console, basically because everything seems to stop responding immediately. I found out only by "bisection" of functionality of my app that ultimately, replacing the ui.table with a ui.list fixes the issue. I don't need that ui.table specifically, but its certainly an ugly failure mode. Is there something I can do to support any efforts here? |
@burnpanck Thanks for offering your help! I don't really know how to proceed, especially because the browser's developer tools are pretty much useless when the tab freezes. But maybe we can find some relevant discussions in Vue/Quasar forums? Or maybe we can find a way to reproduce the behavior with a plain Quasar app - despite my failed attempt. Or maybe there is a browser-independent way to analyse what is going on, maybe using curl or Wireshark to analyse the network traffic? |
I thought I could scrape the data being sent using Proxyman, but apparently, requests to localhost don't go through the system proxy. It looks like I can capture the traffic using wireshark, but I don't know what to do with the scraped websocket data. I guess it would be easier to inject something at the right layer on the python side to capture the application-level data that is being sent. I did notice however that the crash seems to be related by the data. I create the table using |
I hit the same issue with ui.table(rows=[...]). the pages hangs without response to F12. |
So you obviously can't fight what you can't see, right? ![]() https://stackoverflow.com/a/79072070 If you download Firefox, when the script grinds on the infinite loop, you can press "Debug Script" and that puts you in debugger. @falkoschindler you may want to give this another shot, or perhaps we can do pre-processing to ensure that they are never lists to begin with? |
@xaptronic managed to reproduce the infinite loop using just Vue and Quasar, no NiceGUI This can potentially rekindle this debugging effort, so I'm putting this here. |
Wow, thanks @xaptronic! <html>
<head>
<link href="https://fonts.googleapis.com/css?family=Roboto:400|Material+Icons" rel="stylesheet" type="text/css" />
<link href="https://cdn.jsdelivr.net/npm/quasar@2.16.9/dist/quasar.prod.css" rel="stylesheet" type="text/css" />
</head>
<body>
<div id="q-app"><test-table :rows="tableRows" /></div>
<script src="https://cdn.jsdelivr.net/npm/vue@3.4.38/dist/vue.global.prod.js"></script>
<script src="https://cdn.jsdelivr.net/npm/quasar@2.16.9/dist/quasar.umd.prod.js"></script>
<script>
const TestTable = {
template: `<q-table :columns="columns" :rows="clonedRows" />`, // :rows="rows" would cause the browser to freeze
props: { rows: Array },
data: () => ({ columns: [{ name: "id", label: "ID", field: "id" }] }),
computed: {
clonedRows() {
return JSON.parse(JSON.stringify(this.rows));
},
},
};
const app = Vue.createApp({
components: { TestTable },
data: () => ({ tableRows: [{ id: [1, 2, 3] }] }),
});
app.use(Quasar);
app.mount("#q-app");
</script>
</body>
</html> Interestingly only arrays and date objects seem to be affected. Cloning all table rows is certainly not a viable solution, but an interesting insight nonetheless! Does this qualify as a bug in Quasar or even Vue? We could try our luck and post it somewhere upstream. |
Has to be someone knowledge about Vue though. So maybe you and @xaptronic take a shot? |
I wouldn't mind taking a look in QTable implementation to determine if the logic / handling of rows is doing something obvious and open an issue based on what is found there. |
OK - I managed to do some further investigation here. As best as I understand, this is not really a bug with Quasar or Vue. Passing complex data structures in as props can have somewhat undefined / unexpected behaviour depending on what a component is doing with the data. In this case it is rendering the table values as nodes.
So with that, it seems that in this case, it's really up to the developer to provide logic for what to do with complex object types. I think it raises the question of "what are you trying to do" / "why are you passing array data to a table cell" - it would seem the "join behaviour" is a byproduct of the array values being rendered as text nodes that happen to sit next to each other in the DOM. I still don't buy that NiceGUI should be preprocessing data here, as it's essentially acting as a passthrough to the "expected" behaviour of passing the same data to Quasar in a normal JS application. However in the above example (from @falkoschindler) we can use a slot template in the TestTable component to check for array or other types and render them how we'd like; basic case could be an array join, but there could also be much more detailed rendering done depending on the use case. Perhaps the right course is to document this as an example in NiceGUI and link that in threads where folks are running into this issue? <html>
<head>
<link href="https://fonts.googleapis.com/css?family=Roboto:400|Material+Icons" rel="stylesheet" type="text/css" />
<link href="https://cdn.jsdelivr.net/npm/quasar@2.16.9/dist/quasar.prod.css" rel="stylesheet" type="text/css" />
</head>
<body>
<div id="q-app"><test-table :rows="tableRows" /></div>
<script src="https://cdn.jsdelivr.net/npm/vue@3.4.38/dist/vue.global.js"></script>
<script src="https://cdn.jsdelivr.net/npm/quasar@2.16.9/dist/quasar.umd.js"></script>
<script>
const TestTable = {
template: `<q-table :columns="columns" :rows="rows">
<template v-slot:body-cell-id="props">
<td class="text-right">{{ Array.isArray(props.value) ? props.value.join(', ') : props.value }}</td>
</template>
</q-table>`,
props: { rows: Array },
data: () => ({ columns: [{ name: "id", label: "ID", field: "id" }] }),
computed: {
clonedRows() {
return JSON.parse(JSON.stringify(this.rows));
},
},
};
const app = Vue.createApp({
components: { TestTable },
// data: () => ({ tableRows: [{ id: 1 }] }),
data: () => ({ tableRows: [{ id: [1, 2, 3] }] }),
});
app.use(Quasar);
app.mount("#q-app");
</script>
</body>
</html> |
Well, I would argue that the frontend locking up is never correct, and probably should be considered a Quasar or Vue bug, if it is their code that is indeed locking up. Of course, "not locking up" doesn't mean that they will be able to actually support arbitrary data. Silently discarding, logging an error to the console or reporting an error back to the caller are all better options than locking up. From the point of view of NiceGUI: In my use-case, I simply carelessly used an API offered to me without too much thought ( |
I agree it could do this if we believe we understand the problem completely. Pandas dataframes could contain a wide variety of complex data structures, multi indexes etc, which seems like it could add an amount of complexity go about checking the contents of the dataframe in order to give an exception. It may not be ideal, but I'm not sure if I see a clean solution from NiceGUI view here other than documenting the behaviour - there are many kinds of footguns in computers that are not technically "invalid" and wouldn't raise an exception, but that can lead to "undesired" behaviour - like infinite loops, infinite recursion - these are perfectly valid constructs, but can also cause problems (like the one in this issue). In any case - my post above was merely to add what I found here, and I personally don't see anything worth pursuing here. A discussion / issue could be raised to Quasar perhaps and see if anything comes out of that, but again, I don't really think this is a bug any more than |
Thanks for your investigation, @xaptronic! table = ui.table(rows=[
{'id': 'A', 'value': 1},
{'id': 'B', 'value': '2'},
{'id': 'C', 'value': [3, 4, 5]},
])
table.add_slot('body-cell-value', '''
<td class="text-right" :props="props">
{{ Array.isArray(props.value) ? props.value.join(', ') : props.value }}
</td>
''') Would you like to create a pull request with this demo and a short explanation? 🙂 |
At the end of the day, I think the frustration of this issue, is that the browser hangs up and we don't know why. Would it be desirable, if NiceGUI detects that there are lists and no This seems like a pretty light-weight modification, and shouldn't affect any existing users, since their code would crash anyways without this intervention |
But you have to check the dataframes for all kinds of types and then add potentially overridable behaviour and then now NiceGUI owns some kind of hacky API, which could have side effects, perf issues in certain cases all to mask what is probably somewhat sloppy programming (wholesale rendering some complex data frame into a table?) Why don't you just preprocess your data instead of putting that into all of NiceGUI? |
What speaks against just checking the column dtypes of the dataframe against a set of supported types (i.e. numeric&string), and either rejecting or stringifying anything else? It is unclear to me what an Edit: I realise that my suggestion above exclusively applies to the |
And definitely, at least as long as the upstream Quasar/Vue bug is not fixed (the one actually causing the frontend to freeze), this issue is not "minor"/"low-impact": It freezes a complete application with no remedy, and no indication of the cause whatsoever. Someone running into this will have nothing telling them to look into the documentation of anything related to |
I agree the outcome you are looking for is preferable - my own hesitancy (which matters only to the extent that I am not going to attempt to champion a PR to do what is asked here) is that what must be done in NiceGUI in order to achieve that outcome does not strike me as desirable functionality to own and maintain in NiceGUI codebase. I'm making an assumption that this is an edge case and I'm making the assessment that the proposed solution is not elegant and is not something that should be part of NiceGUI - the ask is basically to add data validation across arbitrarily shaped and arbitrarily sized potentially complex data structures to address an edge case in which the developer has not done their own preprocessing / validation of their data structure and YOLO's it into a table. Again - I agree this is not optimal for cases where this occurs, but the solutions referred to here work and this "problem" can easily be solved via documentation. In my opinion that is a better trade off than adding edge-case workarounds to the main codebase. |
I see two scenarios here: A) The user dumps a list into a table cell. B) The user uses |
I agree with your points mostly. But where we diverge is, I'd reckon that So, also to achieve DRY principle, I believe we might as well add the warning to P.S.: Having 2 APIs, one which warns, one which doesn't, is also kinda mismatched and awkward as well. |
@evnchn: The reason for having two APIs in the first place is to offer different ways to interact with a tool. In this case, one is very high-level ( That said, is that really true here? Unfortunately, I'm way too far away from Quasar to be able to imagine what is actually possible to achieve through the generic API. As far as I can tell, the documentation only shows primitive types; all of the fancy cell contents are achieved through wizardry involving |
I argued for splitting scenarios A and B mainly for two reasons:
|
@xaptronic May I ask, with your Vue/Quasar skills, can you throw a wrench in the works, if we detect that the component is entering an infinite loop as it contains lists in rows? Because, it is always better to have a broken table, than the entire page hanging up. If @falkoschindler says that checking That would not be costly if the table is well-behaved, and we don't care about cost if the table is looping. |
Here is what I think is a reasonable compromise given
Since we know how to solve this problem in Quasar - having nothing to do with NiceGUI / Python: Why not have There is also:
Given this already exists and throws an exception for MultiIndex, there is a strong case that you could throw or transform list / other types of cells, but I still think that is kind of a more messy solution (do you throw? Do you transform, if so to what?) - I think rendering something via an appropriate slot template is better than throwing, and also better than trying to transform to something. Slot template pushes the work out of NiceGUI into a framework that is more optimized (imo) to handle that. |
I still think that locking up a whole browser tab is way outside of the allowable perimeter of acceptable unexpected behaviour. IMHO this one should definitely be upstreamed so that the effect of passing "bad" data stays contained within the affected component. |
I just created PR #4775 for |
…4775) This PR proposes a solution for #2744 by adding the check `pd.api.types.is_object_dtype(dtype)` to the `is_special_dtype` function. Because `pd.IntervalDtype` came up as another JSON-incompatible datatype, it is added as another exception. Test script: ```py ui.table.from_pandas(pd.DataFrame({ 'A': [[1, 2, 3], [4, 5, 6], [7, 8, 9]], 'B': [1, 2, 3], 'C': ['a', 'b', 'c'], 'D': [datetime.datetime(2021, 1, 1), datetime.datetime(2021, 1, 2), datetime.datetime(2021, 1, 3)], 'E': [datetime.timedelta(days=1), datetime.timedelta(days=2), datetime.timedelta(days=3)], 'F': [complex(1, 2), complex(3, 4), complex(5, 6)], 'G': [pd.Period('2014-01'), pd.Period('2014-02'), pd.Period('2014-03')], 'H': [pd.Interval(1, 2), pd.Interval(3, 4), pd.Interval(5, 6)], })) ``` <img width="546" alt="Screenshot 2025-05-21 at 11 00 53" src="https://github.com/user-attachments/assets/d89fcd96-b2d7-43f2-bfa7-ccc4f5d2bd35" /> This PR doesn't solve the problem if `ui.table(rows=...)` is called with cells containing lists. But one could argue that this is a more obvious user error while `ui.table.from_pandas` should handle this case behind the scenes.
This issue has been closed via PR #4775 and because I didn't see a clear path forward. If you feel we should continue working on |
Description
As @rohitsathish noticed on #2697, the following minimal example causes the app to freeze:
It's even more apparent when creating the table dynamically:
Somehow Vue and/or Quasar can't handle the row content. But a minimal Quasar app works with the same data:
I'm out of ideas. Can anyone help to find out what's going on? Thanks!
The text was updated successfully, but these errors were encountered: