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

Lazy Sub-Rows Example refetching the whole table #1094

Open
1 task done
ricardo-reis-1970 opened this issue Apr 13, 2024 · 3 comments
Open
1 task done

Lazy Sub-Rows Example refetching the whole table #1094

ricardo-reis-1970 opened this issue Apr 13, 2024 · 3 comments

Comments

@ricardo-reis-1970
Copy link

material-react-table version

v2.12.1

react & react-dom versions

v18.2.0

Describe the bug and the steps to reproduce it

This example looks very convenient, until you try to implement it. I sunk significant costs with it, and then I decided to check on DevTools what was actually being required and, to my shock, here's what this does: it constantly refetches the whole table! Whenever a row is expanded, all table data is brought in from the backend.

If a table is big and growing, let's say 100 rows, lots of levels and many rows already expanded, one more expansion will mean requesting the whole table again, including all other currently open [sub]*-rows.

To add insult — and some more injury — to injury, it stands to reason that the very API must be changed to accomodate this flaw.

What is interesting is that the example I am referring to:
https://www.material-react-table.com/docs/examples/lazy-sub-rows
refers to a React app. React being a SPA library, this approach of requesting the whole dataset is sourly reminescent of "good old" PHP templating.

It's a shame, and a tad indecent, that this is not once referred in the page that no effort is made to add data to the table, but rather a full reboot is the way forward. This begs the question: is the example just irresponsible, or is it the case tha material-react-table is just incapable of updating its data in any grain smaller than 100%?

And before anyone comes with the usual defence of "You know, these are just examples.", or "We're not here to do your job.", or the ultimate "This is open source! What more do you want?", I guess what I want is intellectual honesty.

Needless to say, I intend to further pursue the issue and eventually solve it. Should this be achieved with material-react-table, I will be sharing my own version of your solution here (assuming your API would cease to be UI-dependent). However, it's highly likely that I just fall back to tanstack-table: uglier, but more flexible.

Minimal, Reproducible Example - (Optional, but Recommended)

Nothing here. The MRE is your code.

Screenshots or Videos (Optional)

No response

Do you intend to try to help solve this bug with your own PR?

Yes, I think I know how to fix it and will discuss it in the comments of this issue

Terms

  • I understand that if my bug cannot be reliably reproduced in a debuggable environment, it will probably not be fixed and this issue may even be closed.
@KevinVandy
Copy link
Owner

KevinVandy commented Apr 13, 2024

A few things here:

This is definitely the nastiest message I've ever received in OSS. There has to be a better way to communicate about optimizations that you want to add to an example who's only purpose is to inspire and get you started.

Lazy sub-rows is a very hard problem to solve. This solution uses server-side pagination to keep it efficient, so that you don't literally have to refetch every single row upon row expansion changes. Just the rows on the current page. Furthermore, react-query is used here so that a lot of these fetches can use caching so that if a specific row expansion configuration has already been fetched, react-query can just read from the local cache instead of refetching.

There are different solutions that you can use if you want. Fetching only a few new sub-rows and then splicing them in on the front-end like it sounds like you want to do is possible, but just needs a lot more code. If you're using pagination, 100 rows will be the max it will fetch, so I see this as a small issue, actually.

Also, being mad that you need to change your back-end API to accomplish this is kinda silly. This is a full-stack problem that requires a full-stack solution.

We are always trying to improve our recommendations and examples. I'm both the maintainer of this library and TanStack Table itself. If you have an example to share for either, we'd love to collaborate. But you have to do it in a way less rude manner, please.

@ricardo-reis-1970
Copy link
Author

ricardo-reis-1970 commented Apr 14, 2024 via email

@robmen
Copy link

robmen commented Apr 14, 2024

@ricardo-reis-1970 You can disagree with the solution provided in the example without attacking the person who wrote it. Providing feedback that the example has specific problems can benefit the author and anyone who tries using the example in the future.

At the same time, you can feel frustrated/disappointed/whatever because the example did not work as well as you hoped and that it took time for you to figure that out. But making your complaints personal only upsets the very people who could be helpful.

Here's a potential set of changes you could make in your responses that would make your feedback helpful without being hurtful. Remember the goal is to improve the example for the author and any who come after you.

To add insult — and some more injury — to injury,

You can just delete this.

It's a shame, and a tad indecent, that this is not once referred in the page that no effort is made to add data to the table, but rather a full reboot is the way forward. This begs the question: is the example just irresponsible, or is it the case tha material-react-table is just incapable of updating its data in any grain smaller than 100%?

And before anyone comes with the usual defence of "You know, these are just examples.", or "We're not here to do your job.", or the ultimate "This is open source! What more do you want?", I guess what I want is intellectual honesty.

I think this can be replaced with a single statement, something like: "It would be great if the example pointed out what was necessary when data was being added."

Call me nasty all you want and all the hundred other caveats on open source
and all, but there is no way you can escape the fact that your example
becomes very easy to compare with those plastic buns that come off the oven
in TV cooking shows: perfect looking and FAKE!

You can just delete this.

Your solution requires the whole API to be adjusted to the UI, which is a
great definition of "bad practice". You could also put that one down for
"impossible", as sometimes, go figure, our software actually consumes 3rd
party APIs that don't give a flying 6th letter to our needs. That and the
fact that said APIs might actually be well designed and not wish to
clutter. At some point, you must realise that your solution for this is
just fake, period.

This could be simplified to: "This example requires the API to be adjusted to the UI, which isn't always possible. For example, a 3rd party API that cannot be changed. That will render this example unusable."

Any — and I mean ANY —solution that would change table data incrementally
would be welcome, but your two approaches seem to be:

You could replace this with: "Updating the table data incrementally seems to be missing from the example. As I understand it, you are proposing two approaches:"

100% client*: we dump our (arbitrarily ginormous) data all on the browser
and whatever survives of its CPU and RAM will handle pagination, sorting,
sub-rows, detailed panels and whatever else locally, in ram. If you happen
to have 10000 rows of heavy data (pictures of patients, x-rays) or a
potentially high sub-row count, "buy more RAM".

This could be rewritten: "100% client: we have lots of data that could overwhelm the browser. That would not make this a good solution for our situation where the rows are heavy data (pictures of patients, x-rays) or a
potentially high sub-row count."

100% server: pretty simple: whatever you need, you ask of the API. You
just want to open a sub-row and stuff is already heavy? Tough! Just ask for
absolutely everything you already have on screen, plus the sub-row you want!

I think you're saying it's hard to update your API to be able to request sub-rows, or maybe you're trying to point out that the example can't make sub-row requests, or maybe a third option I don't understand. Ultimately, I think the communication would be improved if there was less emotion and instead focused on helping to improve the example.

I mean, dude, I am an average dev scraping for a living in a small company
and trying to keep up with the great stuff people like you put out there.
You, on the other hand, are quite a big person in the community. The mere
fact that I need to be telling you all this does not feel like I am big
stuff myself. It just feels awkward.

All of this is just personal attacks and does nothing to help the conversation. It does not reflect well on you or your small company.

Also, the Discord community would be a possible great place to go for my
kind of question, except that those... nice gents require I give them my
phone number and that's a big no.

This is a reasonable issue to raise, but this is not the time or place. Raise that issue in the appropriate place (maybe just open a new issue?).

I hope this tone agrees better with your good intentions, although it is
highly dissonant to my own legitimate concerns about sunken costs. And my
apologies if the occasional bent on the moral high ground speech rules (the
6th letter is an "F", oops!).

I don't know how @KevinVandy would take it, but your second response does not improve my impression of you as a bystander in this public forum. You still sound more focused on attacking the person who wrote the example than actually trying to improve it.

I hope this finds you better and the example in question improves for the better of all in the world.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants