Skip to content

fix: prevent timezone override when initialMonth is Date type #2737

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

Merged
merged 1 commit into from
Apr 14, 2025

Conversation

lovebuizel
Copy link
Contributor

What's Changed

When month passed as Date type, it would override the provided timezone settings. This fix ensures proper timezone handling.

image
In react-day-picker\examples\TimeZone.tsx, when passing a Date type for the month parameter, the timezone setting was ignored:

Expected:

  • First day should be: "Tue Apr 01 2025 00:00:00 GMT+1400 (Line Islands Time)"
  • Current day displays correctly

Actual:

  • First day shows: "Tue Apr 01 2025 00:00:00 GMT+0800 (台北標準時間)"
  • Current day displays incorrectly

image

image

After fix:
image

image

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

When initialMonth is passed as Date type, it would override the provided
timezone settings. This fix ensures proper timezone handling.
@gpbl
Copy link
Owner

gpbl commented Apr 13, 2025

Thanks @lovebuizel for your contribution! I believe the behavior of DayPicker is correct here. To pass a time-zoned date as the initial month, you should initialize the date with TZDate:

import { DayPicker, TZDate } from "react-day-picker";

export function TimeZone() {
  const timeZone = "Pacific/Kiritimati";
  return (
    <DayPicker 
      // make sure you use `TZDate` here
      month={new TZDate(2024, 1, 0, timeZone)} 
    />
  );
}

See: https://daypicker.dev/docs/time-zone#working-with-time-zoned-dates

We might want to further clarify this in the documentation.

@lovebuizel
Copy link
Contributor Author

Thank you @gpbl for your response! I have a concern regarding the timezone configuration. I believe the timezone should be specified in only one place to avoid potential conflicts. For example, if I pass timeZone='Pacific/Kiritimati' as a parameter, but simultaneously provide a month parameter as a TZDate object with timeZone='Asia/Taipei', it becomes unclear which timezone should take precedence.

Interestingly, I've noticed that when I don't explicitly pass the timeZone parameter but provide a month as a TZDate object with its own timezone, the timezone from the TZDate object is correctly applied. This behavior essentially renders the separate timeZone parameter redundant and potentially misleading.

This situation not only makes debugging challenging but also presents a potential issue where type checking doesn't catch the inconsistency when passing both a timezone parameter and a month parameter that isn't a TZDate type.

@gpbl
Copy link
Owner

gpbl commented Apr 13, 2025

I see your point @lovebuizel and I agree with your feedback here.

I wonder what happens with the other props, such as selected, where we also pass dates DayPicker 🤔 Should we apply the same treatment to them?

@lovebuizel
Copy link
Contributor Author

lovebuizel commented Apr 14, 2025

Thanks @gpbl for your response!

The selected value, even when provided as a TZDate, does not influence the timezone of the calendar. This is because the calendar's day rendering with timezone is determined by the getInitialMonth method, which specifically depends on props.timeZone.

useEffect(() => {
    const newInitialMonth = getInitialMonth(props, dateLib);
    setFirstMonth(newInitialMonth);
    // eslint-disable-next-line react-hooks/exhaustive-deps
}, [props.timeZone]);

Within the getInitialMonthfunction, several parameters can affect the timezone calculation: month, defaultMonth, today, endMonth, and startMonth. However, it's crucial to note that among these parameters, only the today parameter's default value (dateLib.today()) actually considers props.timeZone in its implementation:

  export function getInitialMonth(
  props: Pick<
    DayPickerProps,
    | "fromYear"
    | "toYear"
    | "startMonth"
    | "endMonth"
    | "month"
    | "defaultMonth"
    | "today"
    | "numberOfMonths"
    | "timeZone"
  >,
  dateLib: DateLib
): Date {
  const {
    month,
    defaultMonth,
    today = dateLib.today(),
    numberOfMonths = 1,
    endMonth,
    startMonth
  } = props;
  let initialMonth = month || defaultMonth || today;
  const { differenceInCalendarMonths, addMonths, startOfMonth } = dateLib;

  // Fix the initialMonth if is after the endMonth
  if (endMonth && differenceInCalendarMonths(endMonth, initialMonth) < 0) {
    const offset = -1 * (numberOfMonths - 1);
    initialMonth = addMonths(endMonth, offset);
  }
  // Fix the initialMonth if is before the startMonth
  if (startMonth && differenceInCalendarMonths(initialMonth, startMonth) < 0) {
    initialMonth = startMonth;
  }

  return startOfMonth(initialMonth);
}

Regardless of which parameter is used, they all ultimately contribute to determining the initialMonth. Therefore, our primary focus should be ensuring that initialMonthis properly converted to a TZDate when a timezone is specified through props.timeZone.

However, there's an interesting edge case to consider: when no explicit timeZone prop is provided, but the monthparameter is passed as a TZDate object. In this scenario, the calendar days' timezone currently matches the timezone of the provided month parameter. To maintain consistent behavior while respecting the user's implicit timezone preferences, maybe we should modify the implementation so that initialMonth remains a TZDate, but either defaults to using the local system timezone when no explicit timezone is specified, or simply preserves its original Date type.

This approach would provide a more intuitive and predictable behavior while maintaining timezone awareness throughout the component.

What's your perspective on this solution?

@gpbl
Copy link
Owner

gpbl commented Apr 14, 2025

@lovebuizel, definitely your PR goes in the right direction, but without tests it's hard to say if it fixes all the cases when using Date instead of TZDate.

For example, while I can confirm your fix solves the case when selecting days, it does not when rendering the month passing a simple Date to month or defaultMonth:

export function TimeZone() {
  const timeZone = "Pacific/Kiritimati";
  return (
    <DayPicker
      defaultMonth={new Date(2024, 0, 0)} // Peru Time: -5 from GMT
      mode="single"
      timeZone={timeZone}
    />
  );
}

Here I'd expect the calendar render the January 2024, but since I'm in a timezone with -5 from GMT hours, it still renders the previous month:

Screenshot 2025-04-14 at 6 02 47 AM

I guess you couldn't see this issue because you are at +8 from GMT.

To move forward, we need to add tests to make sure the whole calendar works with simple Dates objects within the specified timezone, and not just when selecting days.

I can investigate better the issue during the next days.

@gpbl gpbl self-requested a review April 14, 2025 12:29
@lovebuizel
Copy link
Contributor Author

@gpbl, I agree we need to add tests for this. However, when users provide a timezone, they should be responsible for ensuring that the Date values they pass are already correctly adjusted according to the timezone they specified. This is because we cannot determine whether the Date values provided by users have already taken timezone into account or not. Therefore, we should not automatically convert the Date values to TZDate using their specified timezone.

@gpbl
Copy link
Owner

gpbl commented Apr 14, 2025

I think this change is OK for now! Thanks @lovebuizel - we will deal with other issues in a next update.

@gpbl gpbl merged commit 814cb76 into gpbl:main Apr 14, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants