Skip to content

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

Closed
falkoschindler opened this issue Mar 21, 2024 · 31 comments · Fixed by #4775 · May be fixed by #4694
Closed

Browser can't load page when table rows contain lists #2744

falkoschindler opened this issue Mar 21, 2024 · 31 comments · Fixed by #4775 · May be fixed by #4694
Labels
🌳 advanced Difficulty: Requires deep knowledge of the topic bug Type/scope: A problem that needs fixing ⚪️ minor Priority: Low impact, nice-to-have review Status: PR is open and needs review
Milestone

Comments

@falkoschindler
Copy link
Contributor

Description

As @rohitsathish noticed on #2697, the following minimal example causes the app to freeze:

ui.table(columns=[{'name': 'A', 'label': 'A', 'field': 'A'}], rows=[{'A': [1, 2, 3]}])

It's even more apparent when creating the table dynamically:

ui.button('Table', on_click=lambda: ui.table(columns=[{'name': 'A', 'label': 'A', 'field': 'A'}], rows=[{'A': [1, 2, 3]}]))

Somehow Vue and/or Quasar can't handle the row content. But a minimal Quasar app works with the same data:

<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">
      <q-table :columns="[{'name': 'A','label': 'A','field': 'A'}]" :rows="[{'A': [1, 2, 3]}]" row-key="id" />
    </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 app = Vue.createApp({ setup() { return {}; } });
      app.use(Quasar);
      app.mount("#q-app");
    </script>
  </body>
</html>

I'm out of ideas. Can anyone help to find out what's going on? Thanks!

@falkoschindler falkoschindler added bug Type/scope: A problem that needs fixing help wanted labels Mar 21, 2024
@petergaultney
Copy link
Contributor

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 nicegui JS side).

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 nicegui did the stringification on the Python side as a workaround?

@falkoschindler
Copy link
Contributor Author

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! 🤞🏻

@falkoschindler
Copy link
Contributor Author

@dcslin just reported a very similar problem: #3475

@burnpanck
Copy link

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?

@falkoschindler
Copy link
Contributor Author

@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?

@burnpanck
Copy link

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 ui.table.from_pandas. In the crashing version, one column (inadvertedly) contained tuples of strings (most likely as an object-dtype column). If I fix that to make the table contain all strings, the browser freeze doesn't happen.

@lij55
Copy link

lij55 commented Dec 27, 2024

I hit the same issue with ui.table(rows=[...]). the pages hangs without response to F12.
I works fine if I joined the list to a single string.

@falkoschindler falkoschindler added ⚪️ minor Priority: Low impact, nice-to-have 🌳 advanced Difficulty: Requires deep knowledge of the topic analysis Status: Requires team/community input and removed help wanted labels Mar 28, 2025
@evnchn
Copy link
Collaborator

evnchn commented May 3, 2025

So you obviously can't fight what you can't see, right?

Image

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?

@evnchn
Copy link
Collaborator

evnchn commented May 3, 2025

Use prod_js=False for the below. Forgot to mention.

I got this while stepping through using debugger again and again.

Where I placed the debugger

  computed: {
    convertedColumns() {
      debugger;
      this.columns.forEach((column) => convertDynamicProperties(column, false));
      return this.columns;
    },
  },
Image

@evnchn
Copy link
Collaborator

evnchn commented May 8, 2025

@xaptronic managed to reproduce the infinite loop using just Vue and Quasar, no NiceGUI

#4694 (comment)

This can potentially rekindle this debugging effort, so I'm putting this here.

@falkoschindler
Copy link
Contributor Author

Wow, thanks @xaptronic!
In a nutshell (using NiceGUI's Vue and Quasar versions):

<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.

@evnchn
Copy link
Collaborator

evnchn commented May 8, 2025

Has to be someone knowledge about Vue though.

So maybe you and @xaptronic take a shot?

@xaptronic
Copy link
Contributor

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.

@xaptronic
Copy link
Contributor

xaptronic commented May 14, 2025

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.

Tables expect primitives: Most table components (including Quasar's q-table) expect each cell value to be a primitive (string, number, boolean) or something easily serializable.
Reference types are not rendered as you expect: If you pass an array, object, or date, the table or Vue's rendering engine may try to "render" it, which can result in VNodes, [object Object], or other oddities.
Reactivity issues: If the table or its internals mutate the data (e.g., for selection, expansion, or sorting), and your data contains references, this can cause infinite loops or unexpected reactivity updates.

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>

@burnpanck
Copy link

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 (ui.table.from_pandas). The supplied pandas dataframe had actually tuples in there (in an object dtype column), and rendering them as strings would actually have been good enough for me. But that is arguably quite arbitrary. Instead, NiceGUI could elect to declare such behaviour as unsupported - but the python zen then suggests that it should have given me an exception in this case.

@xaptronic
Copy link
Contributor

NiceGUI could elect to declare such behaviour as unsupported

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 while True: might lock up a machine.

@falkoschindler
Copy link
Contributor Author

Thanks for your investigation, @xaptronic!
I'm also leaning towards documenting this potential pitfall and close this issue as completed.
What about this example:

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? 🙂

@evnchn
Copy link
Collaborator

evnchn commented May 14, 2025

What about this example:

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 body-cell-value slot was defined, that it automatically applies the intervention of {{ Array.isArray(props.value) ? props.value.join(', ') : props.value }} to avoid hanging the browser?

This seems like a pretty light-weight modification, and shouldn't affect any existing users, since their code would crash anyways without this intervention

@xaptronic
Copy link
Contributor

xaptronic commented May 14, 2025

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?

@burnpanck
Copy link

burnpanck commented May 15, 2025

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 object dtype should mean for NiceGUI anyway, and if a user wants to put something fancy into a table for which NiceGUI/Quasar/Vue actually have support, there are lower-level tools available from NiceGUI. But as a python API that NiceGUI is offering, silently doing nothing (or worse bad things) is not a good result. Things might be different for JS, where people are used to silent swallowing of errors, and NiceGUI to some degree does offer a kind of "pass-through" interface to Vue, so there is a bit of tension here. But pandas dataframes are far from a JS object anyway, so having a well-defined simple behaviour without silent errors feels like a good solution to me.

Edit: I realise that my suggestion above exclusively applies to the from_pandas API which I was coming from, but of course the issue covers a larger API surface. I still think it would be preferable, if all APIs would throw exceptions rather than silently causing errors (especially, if those freeze the frontend completely), but in the sense of that "pass-through" behaviour, whatever NiceGUI can pass to the frontend probably should be passed.

@burnpanck
Copy link

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 ui.table at all, where a hint that something may not be supported would reside.

@xaptronic
Copy link
Contributor

I still think it would be preferable

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.

@falkoschindler
Copy link
Contributor Author

I see two scenarios here:

A) The user dumps a list into a table cell.
This can be seen as a user error. It would be costly and complicated to detect automatically, but we can address this pitfall in the documentation.

B) The user uses from_pandas (or from_polars) which contains bad columns.
Since we provide an API saying "just give me your dataframe and we take care of the rest" I see a certain responsibility on our side to at least prevent the browser from freezing. And if I understand correctly and we can easily check the column datatypes, we should do that.

@evnchn
Copy link
Collaborator

evnchn commented May 16, 2025

I agree with your points mostly.

But where we diverge is, I'd reckon that ui.table is aso an API saying "just give me your data and we take care of the rest" as well.

So, also to achieve DRY principle, I believe we might as well add the warning to ui.table

P.S.: Having 2 APIs, one which warns, one which doesn't, is also kinda mismatched and awkward as well.

@burnpanck
Copy link

@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 ( from_pandas/from_polars), meaning that the contract is narrow, and erroring out seems the right thing to do. The other API is potentially very generic, in the sense that a wide set of different effects may be achieved through it, and validation may not be feasible without a lot of duplication.

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 add_slot as far as I can see. Is there any use-case at all for cell values other than a very restricted set of types?

@falkoschindler
Copy link
Contributor Author

I argued for splitting scenarios A and B mainly for two reasons:

  1. While calling ui.table(rows=rows) takes rows directly from user code, calling .from_pandas(df) implicitly converts the dataframe into row data. Like in Dataframes with a column of lists - ui.aggrid can display them but ui.table cannot. #2697, problems seem to arise with the latter because the dataframe is kind of a black box.
  2. Checking rows is probably costly because we need to look into every single cell. Checking the dataframe inside .from_pandas(df), on the other hand, might be cheap if we only need to iterate over df.columns. Maybe it can be handled similarly to what we already do:
    def _pandas_df_to_rows_and_columns(df: 'pd.DataFrame') -> Tuple[List[Dict], List[Dict]]:
    import pandas as pd # pylint: disable=import-outside-toplevel
    def is_special_dtype(dtype):
    return (pd.api.types.is_datetime64_any_dtype(dtype) or
    pd.api.types.is_timedelta64_dtype(dtype) or
    pd.api.types.is_complex_dtype(dtype) or
    isinstance(dtype, pd.PeriodDtype))
    special_cols = df.columns[df.dtypes.apply(is_special_dtype)]
    if not special_cols.empty:
    df = df.copy()
    df[special_cols] = df[special_cols].astype(str)
    if isinstance(df.columns, pd.MultiIndex):
    raise ValueError('MultiIndex columns are not supported. '
    'You can convert them to strings using something like '
    '`df.columns = ["_".join(col) for col in df.columns.values]`.')
    return df.to_dict('records'), [{'name': col, 'label': col, 'field': col} for col in df.columns]

@evnchn
Copy link
Collaborator

evnchn commented May 16, 2025

@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 rows in ui.table is costly, can we check how many times the element was rendered, and just give up if after 1000 loops on the same set of data it's not done (indicating infinite loop)?

That would not be costly if the table is well-behaved, and we don't care about cost if the table is looping.

@xaptronic
Copy link
Contributor

xaptronic commented May 16, 2025

Here is what I think is a reasonable compromise given

"just give me your dataframe and we take care of the rest"

Since we know how to solve this problem in Quasar - having nothing to do with NiceGUI / Python:

Why not have from_pandas(df) implicitly use slot templates for cells that can handle rendering complex types - that way you side step the premise of trying to preprocess data frames, and you get a solution that has basically no cost in the case of a simple dataframe and one could still override the slot templates with something more advance / custom if so desired - and it would give an API that can evolve over time as new cases arise.

There is also:

Maybe it can be handled similarly to what we already do <snippet of code in from_pandas>

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.

@burnpanck
Copy link

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.

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.

@falkoschindler
Copy link
Contributor Author

I just created PR #4775 for ui.table.from_pandas. What do you think? To me it feels cleaner than adding implementing slots just to workaround this issue.

@falkoschindler falkoschindler added this to the 2.18 milestone May 22, 2025
@falkoschindler falkoschindler added review Status: PR is open and needs review and removed analysis Status: Requires team/community input labels May 22, 2025
falkoschindler added a commit that referenced this issue May 22, 2025
…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.
@falkoschindler
Copy link
Contributor Author

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 ui.table(rows=...), feel free to propose suggestions and I'll re-open the thread.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌳 advanced Difficulty: Requires deep knowledge of the topic bug Type/scope: A problem that needs fixing ⚪️ minor Priority: Low impact, nice-to-have review Status: PR is open and needs review
Projects
None yet
6 participants