Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DatePicker] Warning in console related to useState hook in examples #24169

Closed
MV88 opened this issue Sep 25, 2019 · 9 comments · Fixed by #24848
Closed

[DatePicker] Warning in console related to useState hook in examples #24169

MV88 opened this issue Sep 25, 2019 · 9 comments · Fixed by #24848
Labels
component: date picker This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@MV88
Copy link

MV88 commented Sep 25, 2019

Hello,
I was playing with your lib and found out the following.
I see a warning in console (dev mode) when using handlers of useState directly in onChange prop like in the usage example

const [selectedDate, handleDateChange] = useState(new Date());
<DatePicker value={selectedDate} onChange={handleDateChange} />

I know there is a way to avoid this by using:

const [selectedDate, handleDateChange] = useState(new Date());
<DatePicker value={selectedDate} onChange={val => handleDateChange(val)} />

Environment

Tech Version
@material-ui/pickers 3.2.6
material-ui 4.4.2
React 16.9.0
Browser chrome 77.0.3865.90
Peer library

Steps to reproduce

Just checkout examples in the documentation

see Live Example section

Expected behavior

I expect to not see any warning or to see correct examples in documentation

Actual behavior

i have this warning in the console when changing date of KeyboardDatePicker

index.js:1375 Warning: State updates from the useState() and useReducer() Hooks don't support the second callback argument. To execute a side effect after rendering, declare it in the component body with useEffect().
    in Rifm (created by KeyboardDateInput)
    in KeyboardDateInput (created by InlineWrapper)
    in InlineWrapper (created by Wrapper)
    in Wrapper (created by PickerWithState)
    in PickerWithState (at EditForm.js:36)
    in div (created by ForwardRef(Grid))
    in ForwardRef(Grid) (created by WithStyles(ForwardRef(Grid)))
    in WithStyles(ForwardRef(Grid)) (at EditForm.js:29)
    in EditForm (created by ConnectFunction)
    in ConnectFunction (at Drawer.js:231)
    in div (created by ForwardRef(Paper))
    in ForwardRef(Paper) (created by WithStyles(ForwardRef(Paper)))
    in WithStyles(ForwardRef(Paper)) (created by Transition)
    in Transition (created by ForwardRef(Slide))
    in ForwardRef(Slide) (created by TrapFocus)
    in TrapFocus (created by ForwardRef(Modal))
    in div (created by ForwardRef(Modal))
    in ForwardRef(Portal) (created by ForwardRef(Modal))
    in ForwardRef(Modal) (created by ForwardRef(Drawer))
    in ForwardRef(Drawer) (created by WithStyles(ForwardRef(Drawer)))
    in WithStyles(ForwardRef(Drawer)) (at Drawer.js:209)
    in div (at Drawer.js:130)
    in MiniDrawer (created by ConnectFunction)
    in ConnectFunction (at App.js:17)
    in div (at App.js:16)
    in MuiPickersUtilsProvider (at App.js:15)
    in Provider (at App.js:14)
    in App (at src/index.js:7)

and this one when using DatePicker
index.js:1375 Warning: State updates from the useState() and useReducer() Hooks don't support the second callback argument. To execute a side effect after rendering, declare it in the component body with useEffect().

Live example

https://codesandbox.io/s/material-ui-pickers-usage-demo-tm9bi
open console from dev tool (chrome)
Try to edit the date of KeyboardDatePicker,

I can provide a PR for updating examples with (val) => handleDateChange(val) fix if you accept

@dmtrKovalenko
Copy link
Member

Yeah. We are sending second argument in onChange callback. Moreover there is a note about this warning on the docs site.

@MV88
Copy link
Author

MV88 commented Sep 26, 2019

Ah, I see, I missed that line in docs. Sorry for the noise

@davidenke
Copy link

davidenke commented May 20, 2020

Yeah. We are sending second argument in onChange callback. Moreover there is a note about this warning on the docs site.

Where is this note? On which site of the docs?

Edit: I found it here, if somebody got stuck here as well:

Make sure that onChange has a second parameter here. So if you will not do onChange={date => handleDateChange(date)} you will get the console warning that useState hook does not accept a second callback argument.

@oliviertassinari
Copy link
Member

oliviertassinari commented May 20, 2020

@dmtrKovalenko Sometime, we provide a reason argument to the onChange handler. This is useful for the advanced components when people need to filter out a few of the interactions.

Should mitigate all the demos away from the direct association pattern?

const [selectedDate, handleDateChange] = useState(new Date());

<DatePicker value={selectedDate} onChange={handleDateChange} />

It would allow adding a reason argument without having to worry about people that copy & paste the demos (which a lot do). It would also avoid people falling into this trap. Alternatively, we could wait for the need to come.

@dmtrKovalenko
Copy link
Member

I think it is already migrated. But definitely yes!

P.S. I am rewriting demos when changing them from the onChange={setDate} to onChange={date => setDate(date)}

@oliviertassinari

This comment has been minimized.

@oliviertassinari oliviertassinari changed the title Warning in console related to useState hook in DatePicker and KeyboardDatePicker examples Warning in console related to useState hook in examples May 25, 2020
@michael-d-t
Copy link

Is this still an issue? I can pick it up

@oliviertassinari oliviertassinari changed the title Warning in console related to useState hook in examples [DatePicker] Warning in console related to useState hook in examples Dec 29, 2020
@oliviertassinari
Copy link
Member

@michael-d-t Yes, it's definitely still an issue. At this point, I believe there is only two demos to migrate:

diff --git a/docs/src/pages/components/date-picker/CustomDay.tsx b/docs/src/pages/components/date-picker/CustomDay.tsx
index f7bae2de4f..28d3b1e6bf 100644
--- a/docs/src/pages/components/date-picker/CustomDay.tsx
+++ b/docs/src/pages/components/date-picker/CustomDay.tsx
@@ -32,19 +32,19 @@ const useStyles = makeStyles((theme) => ({

 export default function CustomDay() {
   const classes = useStyles();
-  const [selectedDate, handleDateChange] = React.useState<Date | null>(new Date());
+  const [value, setValue] = React.useState<Date | null>(new Date());

   const renderWeekPickerDay = (
     date: Date,
     _selectedDates: Date[],
     PickersDayComponentProps: PickersDayProps<Date>,
   ) => {
-    if (!selectedDate) {
+    if (!value) {
       return <PickersDay {...PickersDayComponentProps} />;
     }

-    const start = startOfWeek(selectedDate);
-    const end = endOfWeek(selectedDate);
+    const start = startOfWeek(value);
+    const end = endOfWeek(value);

     const dayIsBetween = isWithinInterval(date, { start, end });
     const isFirstDay = isSameDay(date, start);
@@ -67,8 +67,10 @@ export default function CustomDay() {
     <LocalizaitonProvider dateAdapter={AdapterDateFns}>
       <DatePicker
         label="Week picker"
-        value={selectedDate}
-        onChange={handleDateChange}
+        value={value}
+        onChange={(newValue) => {
+          setValue(newValue);
+        }}
         renderDay={renderWeekPickerDay as any}
         renderInput={(params) => <TextField {...params} variant="standard" />}
         inputFormat="'Week of' MMM d"
diff --git a/docs/src/pages/components/date-time-picker/BasicDateTimePicker.tsx b/docs/src/pages/components/date-time-picker/BasicDateTimePicker.tsx
index 6e8345646a..140ae9f8e5 100644
--- a/docs/src/pages/components/date-time-picker/BasicDateTimePicker.tsx
+++ b/docs/src/pages/components/date-time-picker/BasicDateTimePicker.tsx
@@ -5,15 +5,17 @@ import LocalizationProvider from '@material-ui/lab/LocalizationProvider';
 import DateTimePicker from '@material-ui/lab/DateTimePicker';

 export default function BasicDateTimePicker() {
-  const [selectedDate, handleDateChange] = React.useState<Date | null>(new Date());
+  const [value, setValue] = React.useState<Date | null>(new Date());

   return (
     <LocalizationProvider dateAdapter={AdapterDateFns}>
       <DateTimePicker
         renderInput={(props) => <TextField {...props} />}
         label="DateTimePicker"
-        value={selectedDate}
-        onChange={handleDateChange}
+        value={value}
+        onChange={(newValue) => {
+          setValue(newValue);
+        }}
       />
     </LocalizationProvider>
   );

@oliviertassinari oliviertassinari transferred this issue from mui/material-ui-pickers Dec 29, 2020
@oliviertassinari oliviertassinari added component: date picker This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation good first issue Great for first contributions. Enable to learn the contribution process. labels Dec 29, 2020
@ydubinskyi
Copy link
Contributor

I can take this one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: date picker This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants