mui-x icon indicating copy to clipboard operation
mui-x copied to clipboard

[DatePicker] `shouldDisableDate` runs on all dates ever, not just visible ones before opening

Open DanielAndrews43 opened this issue 3 years ago • 12 comments

Duplicates

  • [X] I have searched the existing issues

Latest version

  • [X] I have tested the latest version

Current behavior 😯

When using DesktopDatePicker it runs shouldDisableDate on 100s of years of dates when first opening the modal. This delays the modal opening for several seconds. When switching between months, it only runs shouldDisableDate on the visible dates, which looks correct.

  • Broken on first open
  • Works great once it's already open

https://user-images.githubusercontent.com/6037887/151841250-8c85bcff-61b2-4a5d-84fe-bd59074a3e3d.mov

Expected behavior 🤔

It should only run shouldDisableDate on the visible dates

Steps to reproduce 🕹

Steps:

  1. Create a DesktopDatePicker component with shouldDisableDate prop
  2. Click on the calendar icon to open the date picker modal
<LocalizationProvider dateAdapter={AdapterDateFns}>
  <DatePicker
    label="Basic example"
    value={value}
    onChange={(newValue) => {
      setValue(newValue);
    }}
    shouldDisableDate={() => false} // this function will run on every date.
    renderInput={(params) => <TextField {...params} />}
  />
</LocalizationProvider>

Context 🔦

We are creating invoicing software and only want dates to be selectable as a due date if they are on the correct billing day. This can be an expensive function for us to run if we need to pull additional data about exceptions. If it only runs on visible days there is almost no delay visible to the user, but if it's running on all historical dates then it ends up in 100s of network requests and slows down everything.

Your environment 🌎

`npx @mui/envinfo`
 System:
    OS: macOS 12.1
  Binaries:
    Node: 14.1.0 - ~/.nvm/versions/node/v14.1.0/bin/node
    Yarn: 1.22.5 - ~/.yarn/bin/yarn
    npm: 6.14.4 - ~/.nvm/versions/node/v14.1.0/bin/npm
  Browsers:
    Chrome: 97.0.4692.99
    Edge: Not Found
    Firefox: 57.0.4
    Safari: 15.2
  npmPackages:
    @emotion/react: ^11.4.1 => 11.7.1 
    @emotion/styled: ^11.3.0 => 11.3.0 
    @mui/base:  5.0.0-alpha.66 
    @mui/core:  5.0.0-alpha.50 
    @mui/lab: ^5.0.0-alpha.66 => 5.0.0-alpha.66 
    @mui/material: ^5.0.3 => 5.0.3 
    @mui/private-theming:  5.0.1 
    @mui/styled-engine:  5.0.1 
    @mui/system:  5.0.3 
    @mui/types: ^7.0.0 => 7.0.0 
    @mui/utils:  5.0.1 
    @types/react:  17.0.11 
    react: 17.0.0 => 17.0.0 
    react-dom: 17.0.0 => 17.0.0 
    styled-components: ^4.4.0 => 4.4.1 
    typescript: ^4.1.3 => 4.3.4 

DanielAndrews43 avatar Jan 31 '22 17:01 DanielAndrews43

+1, makes over 28k calls to that function on load. Also if this were to be fixed, would it be put into MUI 4.x ?

gl-aagostino avatar Feb 07 '22 17:02 gl-aagostino

I'm having the same problem.

apenab avatar Feb 23 '22 10:02 apenab

@apenab @DanielAndrews43 @gl-aagostino Could you give me a reproduction case on Codesandbox ? I tried a few things but I always have 35-50 calls to shouldDisableDate

This template could be a good starting point

flaviendelangle avatar May 02 '22 11:05 flaviendelangle

@flaviendelangle I am also having this problem, however when I use your sandbox it doesn't happen. I did set my versions of "@mui/*" in that sandbox and still can't reproduce. Any advice how to proceed appreciated.

Dragomir-Ivanov avatar Jul 08 '22 12:07 Dragomir-Ivanov

Without a reproduction case it's hard to to a diagnostic. Are you passing the same props to our component both in your application and in the codesandbox ?

flaviendelangle avatar Jul 08 '22 12:07 flaviendelangle

@flaviendelangle Yes, I am trying to. Can we trust CodeSandbox, that it really uses downgraded dependencies, and not some leftovers?

Dragomir-Ivanov avatar Jul 08 '22 12:07 Dragomir-Ivanov

The version should be respected yes Can you link me your codesandbox ?

If you successfully reproduce it locally on an empty repo (with CRA or Next.JS for instance) I can clone it and test it on my machine as well. Codesandbox is of course a lot easier but if it does not work.

flaviendelangle avatar Jul 08 '22 13:07 flaviendelangle

@flaviendelangle Thank you, I will try make a separate project. Could it be Vite.js issue, since I am using Vite. Stay tuned.

Dragomir-Ivanov avatar Jul 08 '22 13:07 Dragomir-Ivanov

@flaviendelangle I think I succeeded reproducing this. Can you try: https://codesandbox.io/s/date-and-time-pickers-forked-3nelms

Latest versions of all @mui/* libraries. Note that shouldDisableDate always returns true, if you make it return false it works as expected.

Dragomir-Ivanov avatar Jul 08 '22 16:07 Dragomir-Ivanov

@flaviendelangle Here is another codesandbox: https://codesandbox.io/s/date-and-time-pickers-forked-pqlc5j

This actually uses your template. Changes are:

  1. Provide initialized value.
  2. Make all dates disabled.

Dragomir-Ivanov avatar Jul 08 '22 18:07 Dragomir-Ivanov

Hi @flaviendelangle I have reproduced the issue in codesandbox. Do you managed to see it? Can I help with anything else?

Dragomir-Ivanov avatar Jul 14 '22 09:07 Dragomir-Ivanov

This does not look like a bug. When no date is open, the component is looking for the closest valid date. You can verify this by returning.

Here is an example with only one valid date (the 5 June 2020) And the component will loop over days until it find it. After that the onChange is called to set the valid date. https://codesandbox.io/s/date-and-time-pickers-forked-ry20f7?file=/src/App.tsx:578-596

If you have not available date, you should disable the component. If you know the first available date, setting it will prevent this search.

@flaviendelangle I think you already fixed this in #6146

alexfauquette avatar Sep 16 '22 16:09 alexfauquette

Yep, #6146 should close the problem :+1:

flaviendelangle avatar Sep 23 '22 14:09 flaviendelangle

@flaviendelangle when we can expect release?

sheppard30 avatar Oct 16 '22 00:10 sheppard30

The fix has only been done on the v6 alpha. But it should definitely be back-porter to the v5 releases, I'll do it :+1:

flaviendelangle avatar Oct 17 '22 05:10 flaviendelangle