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

[pickers] Week no longer respect date-fns weekStartsOn option #12323

Closed
evan-buss opened this issue Mar 2, 2024 · 6 comments · Fixed by #12416
Closed

[pickers] Week no longer respect date-fns weekStartsOn option #12323

evan-buss opened this issue Mar 2, 2024 · 6 comments · Fixed by #12416
Assignees
Labels
component: pickers This is the name of the generic UI component, not the React module! regression A bug, but worse

Comments

@evan-buss
Copy link

evan-buss commented Mar 2, 2024

Steps to reproduce

After upgrading my dependencies, the calendar popup no longer adjusts the week start based on date-fns configuration.

Link to live example: (required)

v6.16.0

v6.19.6

Current behavior

No response

Expected behavior

No response

Context

import { LocalizationProvider, DatePicker } from '@mui/x-date-pickers';
import { Typography } from '@mui/material';
import { AdapterDateFns } from '@mui/x-date-pickers/AdapterDateFns';
import { setDefaultOptions } from 'date-fns';
import './App.css';

setDefaultOptions({ weekStartsOn: 1 });

function App() {
  return (
    <>
      <LocalizationProvider dateAdapter={AdapterDateFns}>
        <Typography align="left" variant="body2">
          v6.19.6
        </Typography>
        <DatePicker />
      </LocalizationProvider>
    </>
  );
}

export default App;

Your environment

npx @mui/envinfo
  Don't forget to mention which browser you used.
  Output from `npx @mui/envinfo` goes here.

Search keywords: week start, monday

Search keywords:

@evan-buss evan-buss added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Mar 2, 2024
@fahdmk
Copy link

fahdmk commented Mar 3, 2024

I think this is a question of prefrences here is someone who went through a lot of trouble to change the starting day to sunday before the update #7670

@zannager zannager transferred this issue from mui/material-ui Mar 4, 2024
@zannager zannager added the component: pickers This is the name of the generic UI component, not the React module! label Mar 4, 2024
@flaviendelangle
Copy link
Member

I think this is a question of prefrences here is someone who went through a lot of trouble to change the starting day to sunday before the update #7670

The components should follow the locale
This issue you linked is for someone using the en-US locale, which should start on Sunday.
But if someone explicitly set the start of the week the Monday on date-fns, it should work.

I'll have a look

@flaviendelangle
Copy link
Member

This regression was kind of introduced in v6.19.0 by #11462
In date-fns, it looks like the locale takes prevalence over the setDefaultOptions({ weekStartsOn: 1 }); config when using startOfWeek
Before, we were not setting a default locale so when using the pickers without any explicit locale you did not have the bug, but it was probably already here with custome locales.

The solution is probably to pass the weekStartsOn explicitely to startOfWeek and endOfWeek

  public startOfWeek = (value: Date) => {
    return startOfWeek(value, {
      locale: this.locale,
      weekStartsOn: (getDefaultOptions() as any).weekStartsOn,
    });
  };

But this seems to not be compatible with SSR because (in NextJS at least), date-fns getDefaultOptions() method does not return the options you set on the server, resulting in an hydration error 😬

cc @LukasTy

@flaviendelangle flaviendelangle added regression A bug, but worse and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Mar 5, 2024
@flaviendelangle flaviendelangle changed the title [Date Picker Regression] Week no longer starts on Monday [pickers] Week no longer respect date-fns weekStartsOn option Mar 5, 2024
@flaviendelangle
Copy link
Member

@evan-buss, changing the value inside the locale works:

const customLocale = {
    ...enUS,
    options: { ...enUS.options, weekStartsOn: 1 as const }
}

<LocalizationProvider dateAdapter={AdapterDateFns} adapterLocale={customLocale}>
  {children}
</LocalizationProvider>

I'm not sure we will be able to properly support setDefaultOptions because of the SSR problem stated above.
We are going to take a deeper look and update the doc example if needed 👍

@LukasTy LukasTy self-assigned this Mar 11, 2024
@LukasTy
Copy link
Member

LukasTy commented Mar 11, 2024

As Flavien has mentioned, there does not seem to be a better approach than just going with what date-fns recommends.
date-fns uses the defaultOptions only when no locale is provided.
Going with a custom solution of only updating the startOfWeek and endOfWeek seems a bit flaky. 🙈

Looks like updating the recommendation is our best course of action. 👌

Copy link

⚠️ This issue has been closed.
If you have a similar problem, please open a new issue and provide details about your specific problem.
If you can provide additional information related to this topic that could help future readers, please feel free to leave a comment.

How did we do @evan-buss?
Your experience with our support team matters to us. If you have a moment, please share your thoughts through our brief survey.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: pickers This is the name of the generic UI component, not the React module! regression A bug, but worse
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants