adjustDayPickerHeight not calculating predictably when setting numberOfMonths prop dynamically

[LMulvey]

On our project, we're using a grid system that optionally passes us a screenClass prop to tell us what breakpoint we're currently in. We're using that breakpoint to pass a numberOfMonths prop to the DayPickerRangeController to render either two months for desktop or one month for mobile.

When we're doing this, our function initializes as sm, which initially passes numberOfMonths={1}. It's changed to numberOfMonths={2} shortly after once window.innerWidth is captured and the method is run.

Setting it to 1 on initial load breaks adjustDayPickerHeight for a month either 2 or 3 greater than the current one. As it's calculated based off of calendarMonthWeeks, which is only set on componentDidMount right now, that value becomes stale until it's triggered again.

Possible Fix

Adding a check to componentDidUpdate for a variation in numberOfMonths and then triggering setCalendarMonthWeeks again fixes the issue.

[haocow]

I am also running into this issue currently on v21.8.0

[LMulvey]

@haocow I put in a PR three years ago that fixed the problem but unfortunately the reviewer wanted me to fix their entire test suite in addition to the problem I identified. I never had time to do that, nor do I agree that I should have had to do that. We ended up going in a different direction because of it as it was a blocker for us.

Feel free to review #1395 and see if that would still apply to this library–you may be able to fix it on a local fork or attempt another PR.

EDIT: Though in reviewing some of this repo's history, it looks like this isn't really maintained anymore. Last merge to this repo was in 2019.

[ljharb]

@LMulvey the repo is still maintained; the latest commit was authored in 2019, but it was merged 29 days ago (if you click on the commit, and to the linked PR, you'll see that).

As for #1395, I don't think that's a fair characterization of what I asked you, and there were no updates after #1415 (comment) to respond to.

[LMulvey]

@ljharb You're right on the commit part for sure, apologies for that. As for #1395, perhaps I misunderstood your comment but my PR involved adding updated tests to reflect the fix I had made. This, however, revealed that an entire test block had been skipped for some time and was not functional. Your comment sounded like you were asking me to fix that test suite before my PR could be merged–something that I did not have the capacity to do at the time.

If my comment was mischaracterizing the situation, I apologize! I'm definitely not trying to stir up any issues with anyone.

[ljharb]

All I wanted to ensure is that your new test passes, even if the other tests next to it don't. You implied that you'd be able to get them all passing, so I asked if you'd mind doing that in a separate PR, but #1415 deletes existing tests and it wasn't clear to me that it was the proper approach. After that it seems like neither of us revisited #1395. If you're willing to rebase it, I can get it landed.

[LMulvey]

Sounds like this was a misunderstanding on my part and for that, I apologize. Thanks for stepping in and clearing it up. Looks like #2095 was created by someone following this thread and has been rebased!