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

[vue table] row values cache doesn't play well with vue proxy-based reactivity #4876

Open
2 tasks done
jonatan-zint-tfs opened this issue May 25, 2023 · 10 comments
Open
2 tasks done

Comments

@jonatan-zint-tfs
Copy link

jonatan-zint-tfs commented May 25, 2023

Describe the bug

While creating table component with vue using @tanstack/table I realized that the tanstack table doesn't play well with reactivity using

useVueTable({
    [...]
    get data() {
        return props.data
    },
})

If you change the contents of props.data (like push a new element) it's perfectly fine for the vue reactivity model. Even the data() function does get reexcecuted - so I'm guessing that the memo() function in the utils of tanstack table decides that props.data did not actually change and it won't get propagated to the row model. See the codesandbox attached, which will make the issue clear I think.

Your minimal, reproducible example

https://codesandbox.io/p/sandbox/busy-browser-61p7of?welcome=true

Steps to reproduce

  1. Use a reactive array in get data() when setting up the table with useVueTable
  2. Push a new element to that array
  3. Realize that it gets properly updated if i.e. just rendered on the template
  4. Realize that no new row is displayed in the table (table.getRowModel()) returns old data

Expected behavior

I'd like to harmonize vue reactivity with the tanstack reactivity at this point.

I can workaround this issue by copying the data before using it in data() - but that'd create an unnecessary extra copy step:

const data = computed(() => {
  return [...props.data];
});

const table = useVueTable<RowData>({
  get data() {
    return data.value;
  },
});

How often does this bug happen?

Every time

Screenshots or Videos

No response

Platform

OS: Windows11 with WSL2 on Ubuntu 22.04
Browser: FF 113

react-table version

8.7.9

TypeScript version

5.0.4

Additional context

No response

Terms & Code of Conduct

  • I agree to follow this project's Code of Conduct
  • I understand that if my bug cannot be reliable reproduced in a debuggable environment, it will probably not be fixed and this issue may even be closed.
@jonatan-zint-tfs jonatan-zint-tfs changed the title memo function doesn't play well with array in vue props [vue table] memo function doesn't play well with array in vue props May 25, 2023
@Tanimodori
Copy link

This library won't reprocess data unless it changes reference as compared via Object.is (docs). Despite it claims to be an "Framework Agnostic" lib, this limitation is originated in react useState (e.g. data/setData) which has to change reference in order to trigger a full-update. Similarly, changing existing data object properties in parent won't trigger update. It is a pity that this library cannot handle partial update like how vue does.

@radusuciu
Copy link
Contributor

radusuciu commented Jul 10, 2023

This library won't reprocess data unless it changes reference as compared via Object.is (docs). Despite it claims to be an "Framework Agnostic" lib, this limitation is originated in react useState (e.g. data/setData) which has to change reference in order to trigger a full-update. Similarly, changing existing data object properties in parent won't trigger update. It is a pity that this library cannot handle partial update like how vue does.

That reading makes sense but I'm having trouble pinpointing where in the core package this limitation is introduced as the only instances of useState or setData seem to occur in the React examples.

Do you think it's possible to fix this by only touching the vue adapter, or is the issue with the memo function as this issue suggests?

@jonatan-zint-tfs
Copy link
Author

As far I can see there is no way to do it in the adapter without taking a performance penalty.

Vue reactivity makes use of proxy objects to trace dependency changes. the memo function essentially only does this to track changes.

    const depsChanged =
      newDeps.length !== deps.length ||
      newDeps.some((dep: any, index: number) => deps[index] !== dep)

So it only will notice a change if it's a primitive value that has changed or the reference of an object/array changed.
A "proper" framework agnostic approach could be that you instantiate a memo function making use of the reactivity system of the framework of your choice, but that probably be a substantial rework.

Maybe exposing a manual trigger that you can hook up with a "watch" function could be a useful api

@radusuciu
Copy link
Contributor

That all makes sense, thanks for discussing.

@jonatan-zint-tfs
Copy link
Author

Hello, circling back to this comment as this becomes a deal breaker for our project now.

I'd like to ask the maintainer @tannerlinsley whether this issue can be acknowleged. I'll try and get an approval to work on it in tanstack. I just like to have a prospect of this being merged into upstream. I'm suspecting I'll need some changes in the core module, not only the wrapper.

@jonatan-zint-tfs
Copy link
Author

Okay I revisited the issue and came to the conclusion that it's actually not the memo function. The objects in tanstack are still the proxies so vue reactivity is well maintained. So what's the issue? the cell.getValue() function that caches the cell values.

It'll cache the value and never invalidates, unless the row gets rebuild, which happens only if the reference to the object in the data property changes:
https://github.com/TanStack/table/blob/main/packages/table-core/src/core/row.ts#L43

This sandbox i made visualized the problem quite well: https://codesandbox.io/p/github/jonatan-zint-tfs/vue-tanstack-example/main?file=/src/components/Table.vue:97,10&workspaceId=14f3e36b-abb4-4b1a-96c2-08e79b17d539

I don't know how this problem doesn't replicate in react apparently. I'd ask @tannerlinsley if we can turn this cache off with a flag, or pass a custom getValue function?

@jonatan-zint-tfs
Copy link
Author

Actually I just pushed an example where you would return a computed in the accessor function, as it can track on the row object it's actually reactive and usable. https://github.com/jonatan-zint-tfs/vue-tanstack-example/blob/c4df6fc8d5db96e2a1ba37caff859d0870e617bb/src/App.vue#L50C19-L50C19

However that's not really intuitive

@radusuciu
Copy link
Contributor

radusuciu commented Sep 20, 2023

Hello, circling back to this comment as this becomes a deal breaker for our project now.

I'd like to ask the maintainer @tannerlinsley whether this issue can be acknowleged. I'll try and get an approval to work on it in tanstack. I just like to have a prospect of this being merged into upstream. I'm suspecting I'll need some changes in the core module, not only the wrapper.

@tannerlinsley @KevinVandy could you please take a look at this issue and associated offer to help develop a patch (if it has a chance of being merged)? There are a few vue issues that I think are all related to this..

jonatan-zint-tfs added a commit to jonatan-zint-tfs/table that referenced this issue Oct 23, 2023
When using tanstack table with proxy objects as row data, copying cell
values to the cache is unwanted behavior as they are no longer connected
to the row proxy object. In these scenarios i'd like to turn off the value cache.

ref TanStack#4876
@jonatan-zint-tfs jonatan-zint-tfs changed the title [vue table] memo function doesn't play well with array in vue props [vue table] row values cache doesn't play well with vue proxy-based reactivity Oct 24, 2023
@jonatan-zint-tfs
Copy link
Author

Sadly this issue seems to be completely disregarded - I guess we need to move forward using a fork

@9mm
Copy link

9mm commented Dec 15, 2023

This may not be the right place for this but it looks related.....

I have a very large table and I'm literally just trying to update a single row within the table after the user uploads a file.

So far the only way I can trigger an update in the table is by redoing the http fetch data call, which changes the root reference and the entire table re-renders......

how do I just update a single row, or force a refresh of the data? This is insanely confusing for something so simple

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

4 participants