TanStack / table

🤖 Headless UI for building powerful tables & datagrids for TS/JS - React-Table, Vue-Table, Solid-Table, Svelte-Table
https://tanstack.com/table
MIT License
24.84k stars 3.07k forks source link

Without StrictMode / In Production Mode onColumnSizingChange does not return any values during resize #4479

Closed bjoe87 closed 1 year ago

bjoe87 commented 1 year ago

Describe the bug

No values are returned when using onColumnSizing during the resize event when app is not wrapped with strictmode. Provided sandbox is based of https://tanstack.com/table/v8/docs/examples/react/column-sizing

Uncomment StrictMode at the bottom to get column resizing working.

Your minimal, reproducible example

https://codesandbox.io/s/dazzling-lake-pr2n28?file=/src/main.tsx

Steps to reproduce

  1. go to https://codesandbox.io/s/dazzling-lake-pr2n28?file=/src/main.tsx
  2. Try to resize any column

Expected behavior

Column should resize. The values for onColumnSizingChange should be passed to the updater.

How often does this bug happen?

No response

Screenshots or Videos

No response

Platform

react-table version

v8.5.15

TypeScript version

No response

Additional context

No response

Terms & Code of Conduct

talatkuyuk commented 1 year ago

It is very strange. The resizing feature has broken in my nextjs app a week ago. I tried to figure it out. It screwed my several days.

Then I see this issue. Yes, reactStrictMode: false in the next.config.js.

Then I turned it into true, then resizing started to work. But I would like to keep it false, since I haven't migrated the useEffects in my app according to React18 yet.

talatkuyuk commented 1 year ago

Another strange thing : There is a weird workaround for the issue :)

See that the table resizing feature is working, even if the StrictMode is false or without React.StrictMode.

I have no idea what the reason is. Cheers.

bjoe87 commented 1 year ago

Was able to look into it today. The underlying problem is the newColumnSizing variable does not contain what gets calculated within the updater. The quick fix is to just move newColumnSizing outside of updateOffset function. (I confirmed this and it works) https://github.com/TanStack/table/blob/main/packages/table-core/src/features/ColumnSizing.ts#L229

rdecoito commented 1 year ago

Looks like https://github.com/TanStack/table/pull/4606 reverted the above MR, but it doesn't seem as if there was any explanation why this happened. I'm having this problem in my production build, and I can replicate the problem easily with v8.7.3 (my project is using v8.7.0, so it's present on that version as well). I just tested it with a minimum reproducible example, and the issue was definitely fixed in v8.7.2 by the above MR, so I'd really love to know why the fix was reverted.

I spent a long time migrating our rather large codebase to v8 only to discover this bug once I pushed it up to our dev environment, so that's been rather frustrating.

Side note, in the spirit of documentation I want to place my observations here. I've been able to consistently see the following behavior, and you can verify this in OP's sandbox example:

  1. If you click and drag the resize grip at a normal speed (casually click, drag, let go) you will not see the column size update. However,
  2. The column size WILL update if you let go of the mouse button extremely quickly after moving the mouse. So mouse down on the resize grip and hold, then flick your wrist and very quickly let go of the mouse button. You'll see the column size update.

The second behavior is consistent, as long as you flick your wrist and let go of the button within a certain amount of time (maybe half a second? hard to tell). This makes me think it could be down to some sort of mount/update condition, or a debounce, or something else.

Anyway, can we get some information about what happened with the fix? Thanks y'all.

KevinVandy commented 1 year ago

Sorry for not being clear on this. Somehow, v8.7.2 introduced some build errors in some next.js projects. I first reverted this to rule out that PR as a possibility. This will be added back in soon.

KevinVandy commented 1 year ago

The fix was returned in v8.7.4 as it was not the cause of build errors