Block datatypes which causes Vue to grind to halt in ui.table rows
#4694
Conversation
evnchn
added
bug
|
Thanks for the initiative, @evnchn! The problem with freezing browsers due to lists in table cells is bugging us for a while now. But I have some concerns:
|
|
Thank you @falkoschindler on the performance and code compatability insights. Personally I am quite hesitant on this PR since it does seem very heavy-handed. But I will say the, old API is also way too lenient. I would expect Point 1: What happens on passing The behaviour is by-design to match what Vue does. Check the 5th row of the image:
They will get "123", because that's what happens if there are multiple text nodes as well. 
Though there are multiple text nodes, it is equivalent in terms of HTML. In fact, if you click to try edit, this happens: 
This indicates that, indeed, [1,2,3] should be considered as "123" Point 2: Why only NiceGUI affected Something may be wrong with the mounting process, which your MRE does not capture as there is no component and there is no mounting to be done. Check out the following patch to beforeCreate() {
console.log('Attributes before create:', JSON.stringify(this.$attrs.rows));
},
beforeMount() {
console.log('Attributes before mount:', JSON.stringify(this.$attrs.rows));
},
mounted() {
console.log('Attributes:', JSON.stringify(this.$attrs.rows));
},And note the console output: So something in the mounting process has turned my strings and integers in the list into This may be an interesting thing to look into. Point 3: Actually I agree this is quite heavy-handed. But I just don't want the code to hang the browser on prod. Perhaps we can do with a warning instead of the filtering process? What do you think? Point 4: That's an oversight on my part. Oops. Given the above, this PR isn't 100% perfect. More like 70%. Should we rather handle this issue with better documentation and perhaps a helper function to help users do the filtering? |
|
Your analysis in #4694 (comment) is pretty interesting. I don't think concatenating 1, 2 and 3 into 123 does make much sense, but at least the browser isn't freezing. The different mounting process could be the key to an explanation. Maybe we can re-create the problem with a Vue example including components and mounting? If so, we should continue the discussion on the original issue #2744. I still think the best way forward is to find out what is creating the recursion and try to stop it. Therefore I suggest to close this PR because I don't find it feasible to check every single cell just because some users might feed invalid data (shooting their own foot). |
But that could be intended behaviour from Quasar / Vue. I'd imagine it's like an implicit
Sorry but I am weak with regards to Vue. It's a black box to me. Perhaps you or Cursor can try it.
I may be pessimistic, but I don't think we could ever find a way to stop the recursion. It is because, if this Reddit reply is anything to go by, "anything is reactive with the Options API", so it will always recursively update itself due to the reactivity. https://www.reddit.com/r/vuejs/comments/15jiu52/comment/jv2ct4k/
And in fact, I recall I have experimented with passing a mutated copy of Sorry but I did not have source-control active for that experiment, and I don't have the code anymore. My opinion is, without turning off Vue's intervention which cannot be possible without #4673 and composition API, I don't see a way out. At least, I don't think I could ever find a way to stop the recursion, with my limited understanding. @xaptronic Sorry for the mention, but you seem much more smarter than me in terms of how Vue works. Would you like to check this out? However, the need to prevent browsers from grinding to a halt is a real one. I suggest the following approach, with the primary goal being to minimize the number of browsers hung in the meantime.
I also have an idea: Can the Vue component screen for VNodes in the rows and columns, and refuse to render and error out if so? This also prevents the infinite loop, but the server code can stay the same, automatically ensuring user code not broken in Python side. @falkoschindler what do you think? Still, this PR has to be a draft, since as it stands it breaks user code. |
|
Plain quasar min reproducible example, which attempts to mimic what NiceGUI does - the difference between the "plain Quasar app" and NiceGUI is that, in NiceGUI, <html>
<head>
<link
href="https://fonts.googleapis.com/css?family=Roboto:100,300,400,500,700,900|Material+Icons"
rel="stylesheet"
type="text/css"
/>
<link href="https://cdn.jsdelivr.net/npm/quasar@2.15.1/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/dist/vue.global.prod.js"></script>
<script src="https://cdn.jsdelivr.net/npm/quasar@2.15.1/dist/quasar.umd.prod.js"></script>
<script>
const TestTable = {
template: `
<q-table
:columns="columns"
:rows="rows"
row-key="id"
/>
`,
props: {
rows: {
type: Array,
required: true,
},
},
data() {
return {
columns: [
{
name: "A",
label: "A",
field: "A",
},
],
};
},
};
const app = Vue.createApp({
components: {
TestTable,
},
data() {
return {
tableRows: [
{ A: "string" },
{ A: 123 },
{ A: true },
{ A: new Date("2025-05-03") },
{ A: [1, 2, 3] },
{ A: { key: "value" } },
{ A: null },
{
A: function () {
return true;
},
},
{ A: Symbol("a") },
],
};
},
});
app.use(Quasar);
app.mount("#q-app");
</script>
</body>
</html>To work around this, one way is to "clone" the rows as a computed property - here is a crude example using JSON.parse/JSON.stringify <html>
<head>
<link
href="https://fonts.googleapis.com/css?family=Roboto:100,300,400,500,700,900|Material+Icons"
rel="stylesheet"
type="text/css"
/>
<link href="https://cdn.jsdelivr.net/npm/quasar@2.15.1/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/dist/vue.global.prod.js"></script>
<script src="https://cdn.jsdelivr.net/npm/quasar@2.15.1/dist/quasar.umd.prod.js"></script>
<script>
const TestTable = {
template: `
<q-table
:columns="columns"
:rows="clonedRows"
row-key="id"
/>
`,
props: {
rows: {
type: Array,
required: true,
},
},
data() {
return {
columns: [
{
name: "A",
label: "A",
field: "A",
},
],
};
},
computed: {
clonedRows() {
/* inflate/deflate in order to send something to q-table that isn't memory referenced to the prop to avoid reactivity updates due to mutations */
return JSON.parse(JSON.stringify(this.rows));
},
},
};
const app = Vue.createApp({
components: {
TestTable,
},
data() {
return {
tableRows: [
{ A: "string" },
{ A: 123 },
{ A: true },
{ A: new Date("2025-05-03") },
{ A: [1, 2, 3] },
{ A: { key: "value" } },
{ A: null },
{
A: function () {
return true;
},
},
{ A: Symbol("a") },
],
};
},
});
app.use(Quasar);
app.mount("#q-app");
</script>
</body>
</html>I didn't look as far as why the prop values are being mutated.
regarding this comment, the first thing I tried was to use explicit I don't think this is really related to composition vs options API, and imo, I don't think introducing behaviour to modify the data being sent through to avoid this bug in the python/NiceGUI layer is quite the right way to solve this. |
3 participants
This PR fix #2744 with a rather heavy-handed approach of converting all data types which could possibly cause Vue to grind to a halt to be string, and disables dicts altogether.
This brings the behaviour in line to Vue