The `selected` prop is not working without setting `onSelect`

[jiwon79]

To reproduce

stackblitz Link: https://stackblitz.com/edit/vitejs-vite-rdyvfyjg?file=src%2FApp.tsx

import { useState } from 'react';
import { DayPicker } from 'react-day-picker';
import 'react-day-picker/style.css';

const DAY_IN_MS = 24 * 60 * 60 * 1000;

function App() {
  const [range, setRange] = useState({
    from: new Date(),
    to: new Date(),
  });

  const changerPrev = () => {
    setRange({
      from: new Date(range.from.getTime() - DAY_IN_MS),
      to: range.to,
    });
  };

  return (
    <>
      <button onClick={() => changerPrev()}>prev</button>
      {/* it doesn't work */}
      <DayPicker mode="range" selected={range} />
      {/* it work */}
      <DayPicker mode="range" selected={range} onSelect={() => void 0} />
    </>
  );
}

export default App;

Actual Behavior

In the first case, the selected value does not change. In the second case, the selected value changes.

Expected Behavior

In both the first and second cases, the selected value should change.

Screenshots

[rodgobbi]

Hi @jiwon79 Thanks for the feedback!

@gpbl I agree with @jiwon79, I noticed this behavior and was planning to discuss at some point. Internally the DayPicker only uses the selected prop if the onSelect is defined. IMO, it's not ideal because developers may want to use other ways to control the selected date. This GH issue has one example, and another one that comes to my mind is using the onDayClick event handler instead of onSelect to compute the selected value.

What do you think of removing this behavior and only using the selected prop if it's defined?

The issue with this change of behavior is that I think it's a breaking change, isn't it?

[jiwon79]

Yes, it’s a BC, but I think we have to removing this behavior.

In all input-like components, component is uncontrolled when value is undefined. The current DayPicker range mode's behavior is hard to predict for someone using it for the first time.

[devdpontes]

Also bumped into this issue. I'm migrating from v7 to v9. Couldn't figure out why the selected prop wasn't working as before.

[gpbl]

Thanks for the feedback @devdpontes.

I think the original idea was a similar behavior as an HTML checkbox in React, where the checked and onChange props are designed to work together. Wondering if a defaultSelected could help here to reintroduce the same behavior - without adding a breaking change. Thoughts?

[rodgobbi]

@gpbl

I tested the idea of the dafaultSelected, there are some caveats about it, even about the current API, lemme share my thoughts.

1. I think dafaultSelected could indeed be a workaround for this issue, but I think it's technically a breaking change to change the behavior from the selected prop only work when onSelect is defined to always using the selected value.

2. The reacty-day-picker right now uses undefined to represent an empty value (no date selected), which works fine most of the time, but there's an edge case that would cause a bug: when trying to reset the selected value from outside the DayPicker to undefined, internally the DayPicker will switch to uncontrolled mode because using only undefined doesn't allow the logic to differentiate between an empty value or the prop not being passed to work as uncontrolled mode.
   E.g. if the DayPicker is used in a form, and there's "reset form" button that resets the form state to the initial values, if the selected prop changes to undefined, the DayPicker would not consider this as an empty value, but as selected not being passed, thus switching to uncontrolled mode, and using the internal state of the component.
   This is why the Spectrum DatePicker API for the selected and defaultSelected accepts DateValue | null, null representing the empty value for the date picker.
   Changing the API to accept null would be significant breaking change though, requiring further discussion.

3. I'd like to also discuss the relevance of having an uncontrolled mode with perhaps a defaultValue prop.
   For input component like the DayPicker, the majority of the use cases, developers are interested in what is the selected date state, to use it somewhere else in the UI or send it to the server in a form submission.
   Using Spectrum DatePicker again as reference, it accepts defaultValue for uncontrolled mode, but besides the prop, the Spectrum DatePicker mounts a hidden input with the internal selected / value state, which allows the uncrontolled state to be sent as part of a form submission.
   The reacty-day-picker doesn't mount this input though, which means that developers need to a always keep track of the selected value to then use it. (developers may not pass the selected date state to the DayPicker as selected prop, but the state most of the use cases needs to be managed outside the component to be able to use it elsewhere)
   What do you think of removing the uncontrolled mode?
   This would simplify the API, and also prevent developers from running into the bug I mentioned in point 2, where the state changes outside the DayPicker but it doesn't reflect to the component itself because it's in uncontrolled mode.

[gpbl]

@rodgobbi, thanks so much for your thoughtful response!

I see how the behavior of the selected and onSelect props might be confusing for users. Our main goal should be to avoid breaking changes while focusing on progressive enhancements. Since many users are still on v8 or v7, we should only consider breaking changes once v9 has broader adoption.

Few ideas:

• To make the migration to v9 easier, we could start by adding a note to the upgrading guide.
• If I understand correctly, introducing a new defaultSelected prop might simplify the upgrade process to v9 (e.g., "replace selected with defaultSelected") without requiring a major release.
  • Another idea could be to make the modifiers={{ selected: selectedDates }} prop work without onSelect. However, I’m not sure how much of a breaking change this would cause.
• While I agree the uncontrolled mode should be removed, it would also be a breaking change. It might be better to save this for a future major release.
• I like the idea of using null as a workaround for issues with undefined. Do you think this could be implemented in a minor release?

I think defaultSelected could indeed be a workaround for this issue, but I think it's technically a breaking change to change the behavior from the selected prop only working when onSelect is defined to always using the selected value.

I’m not sure I fully understand your point here. The idea behind defaultSelected is to avoid changing the behavior of the selected and onSelect props. Could you clarify?

[rodgobbi]

@gpbl for sure, thanks for the feedback 👍

While I agree the uncontrolled mode should be removed

Thanks for providing this feedback. I wanted to confirm if it also makes sense for you.

we should only consider breaking changes once v9 has broader adoption

Good point. I wasn't sure how it would be the best time to work on breaking changes, but you gave a good guideline. 👍

I like the idea of using null as a workaround for issues with undefined. Do you think this could be implemented in a minor release?

I think not, because existing consumers that rely on undefined as a selected value meaning the date is empty would break. E.g. onSelect is called with undefined when unselecting the date, which would break any code doing equality check like === undefined.
To be able to start supporting null, we would need to separate the old API referring to undefined and only use null for the new API.
It seems to me it's not safe to infer this, so the only approach that comes to my head now is having an explicit API like nullEmptyValue, which would treat null as the empty value.
I don't know if this is worth it. This could be used as a migration plan for a future breaking change.
Let me know if we should think more about it and try to come up with other approaches.

I’m not sure I fully understand your point here. The idea behind defaultSelected is to avoid changing the behavior of the selected and onSelect props. Could you clarify?

I see your point.
I understand a bit better now, so the goal of the defaultSelected you proposed is to have a more explicit API that is equal to selected without onSelect defined. This would not be a breaking change for sure.

But this implementation of defaultSelected doesn't solve this GH issue, as the suggestion here is to always use the selected prop as the selected date, which right now only is used when onSelect is defined.

I think the suggestion to "replace selected with defaultSelected" in the migration guide will not work for all use cases, as the intention of the prop is to use the component in controlled mode, so I feel that writing something like "always pass a onSelect callback to use in controlled mode" would cover the migration of this use case.
And we could also add defaultSelected for consumers that want to use it in uncontrolled mode.
What do you think?