feat(Table)!: allow passing all tanstack options as root props

[MuhammadM1998]

🔗 Linked issue

#2441

❓ Type of change

• 📖 Documentation (updates to the documentation or readme)
• 🐞 Bug fix (a non-breaking change that fixes an issue)
• 👌 Enhancement (improving an existing functionality)
• ✨ New feature (a non-breaking change that adds functionality)
• 🧹 Chore (updates to the build process or auxiliary tools and libraries)
• ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

As discussed in #3065, This PR allows the user to pass all accepted options to useVueTable. This has caveats discussed below, until then I'll leave it in draft.

This PR implements these points:

• Removes the explicitly declared Tanstack options from props and instead introduces a tanstackOptions prop that accepts all the possible useVueTable options. This means the user can implement any Tanstack feature without waiting on the library to implement it.
• Moves the previously passed options as defaults passed to defu with the first argument being the tanstackOptions prop to allow the user to override them if needed.
• Adds getPaginationRowModel as a default to have pagination baked-in similar to sorting and the other common features we have as defaults.

Caveats:

This has 2 problems that needs to be addressed.

• Moving the previously declared tanstack options from top-level props to tanstackOptions is a breaking change.
• Now the behavior is not consistent. For example, the user would pass a ref to the top level sorting model value but pass options to the tanstack-options prop. which is not ideal.

I've tried doing the following to extend the Tanstack options as top-level props which would solve both of the above problems.

+ export type TableTanstackOptions<T> = Omit<TableOptionsWithReactiveData<T>, 'data' | 'columns' | 'getCoreRowModel'>

- export interface TableProps<T> {
+ export interface TableProps<T> extends TableTanstackOptions<T> {

But the playground & docs crashes with the following error similar to the one mentioned in #2291

[@vue/compiler-sfc] Failed to resolve extends base type.
If this previously worked in 3.2, you can instruct the compiler to ignore this extend by adding /* @v
ue-ignore */ before it, for example:

interface Props extends /* @vue-ignore */ Base {}

When I use /* @vue-ignore */ the app works correctly but then the top level options are passed as attrs not props as mentioned in the error instruction, so no way to extract them and pass them useVueTable.

📝 Checklist

• I have linked an issue or discussion.
• I have updated the documentation accordingly.

[pkg-pr-new]

npm i https://pkg.pr.new/@nuxt/ui@3177

commit: e679c46

[MuhammadM1998]

Hey @sandros94. I've tried both Dev Drive and WSL as you suggested and the same issue persists. Can you please take a look and share any ideas you have?

The errors happens when I extend the props with tanstackOptions, declaring a prop with the same type works fine.

// This Works
export interface TableProps<T> {
  tanstackOptions: TableTanstackOptions<T>, 
}
 
// This errors
export interface TableProps<T> extends TableTanstackOptions<T> {

[MuhammadM1998]

After some digging, probably it's not supported yet vuejs/core#8482

[sandros94]

I still think that this is completely unrelated to path length or anything on that topic.

I see this as two different issues, one related to types (the requirement of /* @vue-ignore */, that for some reason doesn't work for me) and another for correctly passing the table's related props to useVueTable. This latter one is easy to fix but I'm struggling with the first one.

Will update in a few minutes (famous last words)

[sandros94]

After some digging, probably it's not supported yet vuejs/core#8482

but if this was it (as Vue needs to compile the types, so they cannot be fully dynamic) why it works when dynamic types are passed to as tanstackOptions?: TableTanstackOptions<T> 🤔

[sandros94]

Indeed the issue seems to be related to TableOptionsResolved from @tanstack/vue-table, and I still think it is unrelated to vue's compiler.

I found a potential fix, but before pushing it I want to double check it further

[MuhammadM1998]

but if this was it (as Vue needs to compile the types, so they cannot be fully dynamic) why it works when dynamic types are passed to as tanstackOptions?: TableTanstackOptions<T> 🤔

The problem is the intersection/union of TableProps & TableTanstackOptions. Using only one of them works fine, that's why it works when using tanstackOptions?: TableTanstackOptions<T> as we're using only one type

To confirm it further I tried extending the most basic type and I also got the same error.

export interface TableProps<T> extends { foo: 'bar' }

[sandros94]

@MuhammadM1998 not really, take a look at 20307f4.

As you can see some of the FeatureOptions are commented out, I still haven't looked into which of them cause vue to error out, but I did notice some of them are dynamic types

Now I'm experimenting with the base options for those features that I currently commented out, as I suspect that the issue is more related to their functions.

I've also removed getPaginationRowModel, as it is more project dependent. As it easily breaks/forces pagination in context where a different configuration is needed (particularly via server-side pagination), updating docs rn.

[MuhammadM1998]

Hmm I might be missing something I don't get why CoreOptions would work and not TableTanstackOptions 🤔

Another thing I noticed is you moved the table props to defu as last argument, wouldn't this ignore passed props by the user if it's already defined inside the component? For example, passing a custom onGlobalFilterChange to the component won't run as the declared onGlobalFilterChange in the component will always have a higher priority?

[sandros94]

Hmm I might be missing something I don't get why CoreOptions would work and not TableTanstackOptions 🤔

CoreOptions is the only part that fully works. It's the FeatureOptions, the other extend, that introduces many issues and breaks quite a few things here and there. For example my latest commit broke both selection and pagination

Another thing I noticed is you moved the table props to defu as last argument, wouldn't this ignore passed props by the user if it's already defined inside the component? For example, passing a custom onGlobalFilterChange to the component won't run as the declared onGlobalFilterChange in the component will always have a higher priority?

Yes indeed, and I'm probably going to revert it. The reason why I did move it is because it could easily break the various emits.

[MuhammadM1998]

I get what you mean now. I'll try to test it and see if I catch anything. Thanks for the help 🙏

[sandros94]

I'll try to test it and see if I catch anything. Thanks for the help 🙏

I'm always happy to help where I can 😬

Personally, I'm still not fully convinced about having all the tanstack options as top-level props. I had to rely on /* @vue-ignore */ a bit too much for my liking, and potentially see us fixing strange issues or edge-cases due to some tanstack option being wrongly compiled by Vue's props (or just conflict with other). As you have noticed the errors don't help us, since they don't actually pinpoint the root cause.

@MuhammadM1998 let me know if you notice anything. In my limited experience with tanstack it does seem to work correctly now (although some types are missing due to the requirement of vue ignore mentioned).

@benjamincanac Are we sure that the DX we want to achieve is completely encapsulated within a single component? What about creating a wrapper composable for useVueTable (for example useTable) and then use its output directly within UTable? At that point we could also subdivide it in UTableH, UTableR and UTableD for more advanced uses.

Also, any particular reason why:

ui/src/runtime/components/Table.vue

Lines 201 to 203 in f21e923

	defineExpose({
	tableApi
	})

instead of just defineExpose(tableApi)? This way we could simplify the DX by providing table?.setPageIndex(p - 1) instead of table?.tableApi.setPageIndex(p - 1) (if we want to keep everything in a single component, without a dedicated composable)

[benjamincanac]

Trying to catch-up here, is this ready for review? 🤔

@sandros94 This is how components are made in Nuxt UI v3, if you want a component for td, th, etc. it's more shadcn-vue philosophy I guess.

[benjamincanac]

@sandros94 In my last commit, I've cleaned a bit and moved the tableApi in the defineExpose as you suggested 😊

[sandros94]

@sandros94 This is how components are made in Nuxt UI v3, if you want a component for td, th, etc. it's more shadcn-vue philosophy I guess.

Or Reka UI, but yes out scope of Nuxt UI

Trying to catch-up here, is this ready for review? 🤔

Technically speaking, yes.

What doesn't fully convince me is that quite a few useVueTable's options conflict with Vue's props compiler (those I checked only because are dynamic types and simply not supported). I don't know Tanstack deeply enough to understand if this could increase the maintenance complexity or be safe to do.

What I left untouched are all the defineModels. If we want to go with top-level props should we keep them? Currently useVueTable isn't reactive (except for the data)

[sandros94]

What I left untouched are all the defineModels. If we want to go with top-level props should we keep them? Currently useVueTable isn't reactive (except for the data)

Rethinking about this it shouldn't be a problem. Basically, talking out loud, the DX would become like:

1. remove all current defineModule
2. new props are only responsible for initializing the table (data and options)
3. use the table's template ref to alter the data/options (eg.: hiding/showing/pinning columns) and provide the upstream functionalities as they got developed

@benjamincanac WDYT?