fluentui icon indicating copy to clipboard operation
fluentui copied to clipboard

[Bug][Datagrid]: changing columns triggers the callback with the old columns

Open izkizk8 opened this issue 1 year ago • 3 comments

Library

React Components / v9 (@fluentui/react-components)

System Info

System:
    OS: Windows 11 10.0.22631
    CPU: (8) x64 Intel(R) Core(TM) i7-9700 CPU @ 3.00GHz
    Memory: 12.16 GB / 31.79 GB
  Browsers:
    Edge: Chromium (123.0.2420.97)
    Internet Explorer: 11.0.22621.1

Are you reporting Accessibility issue?

no

Reproduction

https://stackblitz.com/~/github.com/izkizk8/fluentui-datagrid

Bug Description

repro:

  • open the demo, we can see four columns(File, Author, Last updated, Last update)

  • attach breakpoints as below

  • click top button

  • we can see there three columns(Author, Last updated, Lat update) has been passed image

  • resume to next the breakpoint, we can see it still have File column callback image

Actual Behavior

it triggers the callback with the File column

Expected Behavior

it shouldn't triggers the callback with the File column

Logs

No response

Requested priority

Blocking

Products/sites affected

No response

Are you willing to submit a PR to fix?

no

Validations

  • [X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
  • [X] The provided reproduction is a minimal reproducible example of the bug.

izkizk8 avatar Apr 28 '24 08:04 izkizk8

Hi @izkizk8, thanks for providing a repro, I can confirm that I can experience this with your sandbox. If you keep on resuming, you'll see that the second breakpoint is hit more than once for each header, and that you eventually start only seeing the new headers after the update, which might indicate that internal processing is happening where that information takes a bit to propagate to that point. I'll refer to @ling1726 on what is actually happening and if this is a bug or just something expected by design.

khmakoto avatar Apr 29 '24 23:04 khmakoto

Hi, this is an effect of the architecture of V9 components.

Internally in DataGrid, we're managing the state of it. One part of the state is the definition of columns. This state, together with the column definition is then passed into a context, where its later consumed by the DataGrid children.

When you change the columns prop, during the first pass, the state is updated and the context is fed new data. This doesn't propagate to the DataGridRow component yet, as it only updates when the context value changes, i.e. after the render.

When the context value changes (now with the new columns), the DataGridRow picks up this change and re-renders itself.

Is this behavior causing any issues for you?

george-cz avatar May 07 '24 10:05 george-cz

Hi @george-cz , thank you for your explanation!

I have implemented my own renderHeaderCell function, which must use the latest columns to prevent errors. Therefore, when I change the columns, the renderHeaderCell function may use outdated columns and produce errors due to the presence of an effect.

Can I implement logic within the renderHeaderCell function to handle outdated columns, such as returning an empty value, to prevent errors caused by using outdated columns and does not affect the rendering for the new columns?

izkizk8 avatar May 07 '24 13:05 izkizk8

hello @izkizk8,

You could definitely check if the current column should still be rendered in the renderHeaderCell, by comparing it to the up-to-date state. I understand this is not ideal, but probably the best work-around for now.

Let me know if I can help with anything else.

george-cz avatar May 10 '24 09:05 george-cz

hi @george-cz, thanks very much!

I have taken this kind of workaround, thank you very much for your answer.

izkizk8 avatar May 13 '24 02:05 izkizk8