v7 Feedback & Ideas

[Grsmto]

Hi,
First, I'm trying v7, it's awesome to work with Hooks! Trying it for the first time so my comments might be wrong.

Describe the bug
This are more of an architecture comment than a bug:
If you disable useExpanded hook, the table breaks.
I know in userland you can't disable this hook but the fact that this happens, (edit: actually you can) probably demonstrates wrong app design: I figured out this is happening because you're building initial state within the useExpanded hook (row.cells = ...).

Expected behavior
I think that adding or removing hooks should not break the entire component. Hooks should be encapsulated.

Codesandbox!
https://codesandbox.io/s/nk9rmym6m4

Solution
That initial state logic could be within useTable itself (tested and works fine).
I can provide a PR if needed.

[tannerlinsley]

I totally agree. I've done my best so far to make them isolated, but there are still some artifacts like this one you discovered. I'll rework that hook to be more isolated.

[Grsmto]

Hey,
Sorry to comment here again regarding v7 architecture but I don't know if you guys have an open discussion somewhere about it so I put my thoughts here:

Hook clashes

I think it needs a pattern for hooks clashes: currently the different react-table hooks APIs are mutating the different react-table props to expose the hooks API:
ex:

• column.getSortByToggleProps()
• column.getGroupByToggleProps()
• column.sortedDesc
• row.toggleExpanded()
  etc.

This approach has different issues:

• if you let user add custom hooks, it can quickly lead to clashes between properties/functions name.
• if offers poor API discoverability. It gets hard for the user to know what prop is where. You have to read doc etc.

Proposed solution:

• prop should be namespaced. A simple solution would be to just use the hook name:
  useGroupBy hook -> row|column|header.getGroupByProps()
  This way you avoid props clashes.
  Also I think everything should be under a get...Props() function instead of directly added to the objects like column.sortedDesc (is there any downside of doing this?).

Let me know if this helps, I would be happy to provide some more contributions if you guys are actually open to discussion!

Thanks!

[tannerlinsley]

This is valuable feedback. I'll definitely take it into consideration as I'm flexing the API over the next few weeks.

[tannerlinsley]

Some notes while it's on my mind:

• I will not be possible to add core hooks to the table system. (They could clearly mutate api.hooks to add their own anyway, but the idea behind the hook system is to have a common layer that multiple hooks can plug into without adding additional API the user has to know about during rendering.
• If we're designing a system that will encourage users to request more and more hooks to be added to the core, then we're doing something wrong. Up to this point, I don't think we are, because....
• The non-hook, renderer-facing api uses very generic prop getters for different parts of the table which are likely to either not change, or will cover most use-cases.
• As for the variables that are directly available on the model (or anything expensively calculated in the table for that matter) like column.sorted or column.sortedDesc are intended. These variables are only computed optimistically and via memoization, so their performance impact is negligible.
• The reason for the internal hooks plugin architecture is because data grids actually have 2 main phases of a render:
  1. Setup, change detection, memoization, immutable data transformation, materialization of the final API
  2. Pre-render modifications, inversion of control to the user, pre-rendering logic, anything that requires the final API

More to come.

[Grsmto]

• I will not be possible to add core hooks to the table system. (They could clearly mutate api.hooks to add their own anyway

Why not? For example when I have been experimenting with the v7, I have been adding a useSelectRow hook to add support for select/unselect and batch actions on the table. It's been working just great!
I feel like you miss an opportunity here to provide a "plugin-ready" table where users could add their own feature via hooks.

• If we're designing a system that will encourage users to request more and more hooks to be added to the core, then we're doing something wrong. Up to this point, I don't think we are, because....

Yeah that's why a plugin system is great, if a user needs more feature, he can just add it himself without spamming the maintainers.

I think the main issue at the moment with the architecture is that the hooks are mutating the data. I think that hooks should act more like "reducers" (as in Redux reducers for example), take the data as param and return new (mutated) data.
That way you could let the users add hooks without worrying for the table to break.
That's pretty much what the hooks are doing at the moment and I'm not sure why that mutation is necessary.

[tannerlinsley]

I think I see what you're referring to now.

The first api is a reducer pattern, which is essentially what produces the final API. It's started in the useTable hook, then reduced through each plugin hook. Each plugin is responsible for returning the newly decorated or changed API as a replacement. Nothing is mutated there.

As all of the hooks are being reduced down to the final API, an internal hooks: {} object is currently used to keep track of functionality that must be deferred and applied later (after the final API has been reduced). Maybe hooks is a bad name internally, since it conflicts with the actual React hooks vocabulary. I'll try and think of a better one. Anyhow, the mutations you're referring to like api.hooks.getTableProps.push(...) could easily be avoided by creating a method like api.addHook('getTableProps', fn). Still, this technically wouldn't protect a user from mutating the hooks list though, since it is a property of the api that is being reduced. Again, we could pull that list out and only pass down the addHook function, but I feel like it's a bit overkill right now.

Under those circumstances, I think you're very correct that in addition to adding more functionality to existing hooks (like adding a prop to a header) you'll also want to be able to add new hooks (and their accompanying methods) to the system like what you mentioned: <input {...row.getSelectInputProps()} />. That way the user would have a new method to call in their markup AND other hooks could extend that api by using api.addHook('getSelectInputProps', fn).

Did I miss anything?

[janewang]

[Feature] Could we have a hook for update entire columns? Right now we are bounded to cells and rows level updates. We want to pivot the table which would make it awesome, is this possible?

[Aj1402]

[Feature] We can add support for virtualized rendering when used without pagination, this can increase performance of rendering huge tables without pagination.

[gary-menzel]

@Aj1402 RT7 is headless - the developer using the library handles the rendering. The sample @tannerlinsley provided on codesandbox (https://codesandbox.io/s/m5lxzzpz69) shows how infinite scroll can be implemented by creating a hook. Check src/components/Table.js and look for useInfiniteScroll - as well as just clicking the buttons on the demo. I don't think it is guaranteed, but that hook may be supplied as part of a set of hooks in the production release.

[jackpope]

Are there docs in the works for v7? I realize it is still in alpha release but it will help get more usage and feedback. I am trying to bring v7 into my project (for the first time, never used prior versions) and am having to jump into the source to figure out how to use the APIs. For example, I'm trying to fire a callback when filters change and see onFilteredChange referenced in the docs/issues but don't see it in the code base.

[tannerlinsley]

Docs are still in the works unfortunately.

Also, the concept for handling changes to state have changed drastically from v6's onFilterChange:

// Hoist the table state out into your own code
const state = useTableState({ pageCount: 0 });

// Grab the piece of table state you want to watch
const [{ filters }] = state;

// Use an effect that triggers when it changes
useEffect(
  () => {
    // filters changed
  },
  [filters]
);

// Pass the hoisted state to the table
useTable({
  ...
  state,
  ...
})

Essentially, you hoist the table state out into your own code which allows you to watch it for changes with useEffect probably being the easiest way to do this.

[dbertella]

What is the best way to start and play with version 7, I'm looking forward to see all the possibility this rewrite will open and I would like to start testing it and possible contributing to it. I believe the only example out there is the code sanbox linked above, correct? Right now I'm trying to make a table with a nice treeview and I'm wondering if it can be a good feature for v7 and easier to implement instead of hacking something on top of v6 (I tried to use the HoC in v6 but is not doing a great job at the moment)

[gary-menzel]

@dbertella - it will definitely be easier (in some ways) to do a Tree View in V7 but, as with all tree data, it is going to depend on how your tree is organised. I am still learning V7 myself but hope to add a tree view example. You would need to write a hook to manage the state (i.e. nodes open/closed etc.) and would possibly also need to implement other hooks to manage what "sorting" or "filtering" means against your tree data. I recommend having a look at the useExpanded hook as it is currently used to do the pivoting node management.