strapi / design-system

Strapi.io's design system :rocket:
https://design-system.strapi.io
MIT License
470 stars 164 forks source link

Performance degradation of the DatePicker #1170

Open bl96 opened 1 year ago

bl96 commented 1 year ago

Bug report

Required System information

Describe the bug

The DatePicker is degrading drastically the performance of the page which uses them. I am rendering around 100 date pickers per page. This issue started from 4.11.0 - when using version 4.10.7 the DatePickers are rendered without high performance degradation. I tried to use only the basic DatePicker without additional logic and the performance was degraded the same.

Please let me know if more information is needed.

The basic DatePicker: <DatePicker onChange={() => {}} selectedDate={new Date('1990-01-01')} label="Your birthday" onClear={() => {}} />;

Steps to reproduce the behavior

  1. Create a page with table
  2. For each row render a DatePicker, the base one can be used
  3. Check the performance with/without DatePicker

Screenshots

This is performance insight for the page with DatePickers (around 100 rendered), takes 16 seconds:

Screenshot 2023-06-29 220040

After I remove the DatePicker, the webpage is rendered in less then 4 seconds. Only the DatePicker is removed, nothing else:

Screenshot 2023-06-29 220351

joshuaellis commented 1 year ago

Can you describe this a bit more: Create a page with table – Is this something custom you're doing?

bl96 commented 1 year ago

Hi @joshuaellis,

yes exactly.

I have a custom plugin with custom page.

I have created a dummy component which can be used to reproduce the issue:

import React from "react";
import {Box, DatePicker, Table, Tbody, Td, Th, Thead, Tr, Typography} from "@strapi/design-system";

export default function DatePickerPerformanceTableTest() {
  return (
    <Box hasRadius={true}>
      <Table rowCount={1} rows={100} colCount={1}>
        <Thead>
          <Tr>
            <Th width={1}>
              <Typography style={{whiteSpace: "normal"}} variant="sigma">Date Picker Test</Typography>
            </Th>
          </Tr>
        </Thead>

        <Tbody>
          {
            Array(100).fill(0).map(() =>
              <Tr>
                <Td width={1}>
                  <Box>
                    <DatePicker onChange={() => {
                    }} selectedDate={new Date('1990-01-01')} label="Your birthday" onClear={() => {
                    }}/>
                  </Box>
                </Td>
              </Tr>
            )
          }
        </Tbody>
      </Table>
    </Box>
  );
}
joshuaellis commented 1 year ago

IMHO you should be virtualising the list of the table when you want to do a table that large and there's no pagination. Anything over 50 is when you'll start running into rendering problems for large datasets (typically).

We made UX improvements with the DatePicker which naturally increased the amount of code & improved accessibility so it is heavier but with regular usage we haven't seen performance issues, I would categorise this as an extreme case.

bl96 commented 1 year ago

Thank you for your answer.

However, even if I reduce the dataset to 20 for example, it still takes quite a lot of time and the page will freeze for a split second. This happens specifically only with the DatePicker. Is this a normal behaviour for the DatePicker? It did not happen before in 4.10.7.

One more question regarding the pagination - is it possible to use the Strapi default pagination functionality (which is for example used in content management) in custom plugins and pages? I would rather use the Strapi's build-in pagination instead of creating my own.

EDIT: The performance issue was tested on Firefox and Chrome.

joshuaellis commented 1 year ago

However, even if I reduce the dataset to 20 for example, it still takes quite a lot of time and the page will freeze for a split second. This happens specifically only with the DatePicker. Is this a normal behaviour for the DatePicker? It did not happen before in 4.10.7.

hm, we'll move this to the DS so it can be investigated in due course.

One more question regarding the pagination - is it possible to use the Strapi default pagination functionality (which is for example used in content management) in custom plugins and pages? I would rather use the Strapi's build-in pagination instead of creating my own.

FE perspective, yes probably, it's exported from the helper-plugin. BE i'm not sure about.

westende commented 1 year ago

This issue can be exacerbated by setting DateTimePicker step size to 1. This can be done in one of the Storiebook stories for reproduction. I am not entirely sure if this is the same issue as the OP mentions the DatePicker. This issue originates from the ComboBox component in @strapi/ui-primitives.

It can be reproduced with the following Storybook story (simulating step size 1 in DateTimePicker with 60 * 24 values):

export const PerformanceUsage = {
  render: () => {
    const values: number[] = Array.from({ length: 60 * 24 }, (value, index) => index);
    return (
        <Combobox.Root>
          <Combobox.Trigger>
            <Combobox.TextInput placeholder="Pick me" />
            <Combobox.Icon />
          </Combobox.Trigger>
          <Combobox.Portal>
            <Combobox.Content>
              <Combobox.Viewport>
                {values.map((value) =>
                    <Combobox.Item value={value} key={value}>
                      <Combobox.ItemText>Option {value}</Combobox.ItemText>
                      <Combobox.ItemIndicator>
                        <Check />
                      </Combobox.ItemIndicator>
                    </Combobox.Item>
                )}
                <Combobox.NoValueFound>No value</Combobox.NoValueFound>
              </Combobox.Viewport>
            </Combobox.Content>
          </Combobox.Portal>
        </Combobox.Root>
    )
  },
  name: 'Performance',
} satisfies Story;
mrnateriver commented 1 year ago

In my case React Profiler measures the component's render time at ~80ms: image

That's already a noticeable delay when opening a page with the component, and I can easily imagine two or more such fields on a page.

xtul9 commented 3 months ago

The severity should be higher. Even a single DateTimePicker causes a major performance drop on every other input component. At this point I'm forced to create a custom date picker.