Skip to content

fix(core): clamp Calendar numberOfMonths to 1|2 at runtime#3248

Open
durvesh1992 wants to merge 2 commits into
facebook:mainfrom
durvesh1992:fix/calendar-number-of-months-clamp
Open

fix(core): clamp Calendar numberOfMonths to 1|2 at runtime#3248
durvesh1992 wants to merge 2 commits into
facebook:mainfrom
durvesh1992:fix/calendar-number-of-months-clamp

Conversation

@durvesh1992

Copy link
Copy Markdown
Contributor

Summary

numberOfMonths on Calendar (and the DateInput / DateTimeInput that forward to it) is typed 1 | 2, but nothing enforced that at runtime. A free-form caller — or the docsite Properties control — could pass 0 (renders no months) or 1000 (locks the page up trying to render 1000 month grids in Array.from({length: numberOfMonths})).

This clamps the value to the supported union in Calendar: anything that isn't exactly 2 falls back to 1, with a dev-only console.warn when an out-of-range value is passed. DateInput and DateTimeInput inherit the guard for free since they forward the prop straight to <Calendar>.

Note: the docsite should also render a 1/2 toggle instead of a free-form number input for small-literal-union props (item 3 in the issue) — that's a separate docsite-side change and not included here.

Test plan

  • Added unit tests: numberOfMonths={1000} and ={0} both render exactly one month and emit the dev warning.
  • Existing multi-month tests (={2}) still pass — 32/32 Calendar tests green.

Closes #2704

@vercel

vercel Bot commented Jun 29, 2026

Copy link
Copy Markdown

Someone is attempting to deploy a commit to the Meta Open Source Team on Vercel.

A member of the Team first needs to authorize it.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jun 29, 2026

@cixzhang cixzhang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, console warn makes sense although I don't think process exists in runtime.

Comment thread packages/core/src/Calendar/Calendar.tsx Outdated
// Clamp to the supported union: anything that isn't exactly `2` falls to `1`.
const numberOfMonths = numberOfMonthsProp === 2 ? 2 : 1;
if (
process.env.NODE_ENV !== 'production' &&

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might not always exist and is likely causing the build errors.

process is not guaranteed to exist in the runtime bundle; match the
codebase convention (Field/Popover) of an unconditional console.warn.
@durvesh1992

Copy link
Copy Markdown
Contributor Author

Good catch — process isn't guaranteed in the runtime bundle, which was the build error. I've dropped the process.env.NODE_ENV guard entirely and now call console.warn unconditionally on an out-of-range value, matching the existing convention in Field and Popover (neither gates its warn behind an env check). The clamp logic and tests are unchanged — 32/32 Calendar tests still pass. Pushed in b882749.

@cixzhang cixzhang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this. I was reading through #2704 and thinking about the kinds of guardrails we want for this. Since the type is already constrained by the type I think having additional console warnings is likely not going to be quite as helpful as the type constraint. Given that we don't have a good way to handle in-component errors in a traceable way for consumers (ie if an end user encounters this issue it needs to be logged somewhere for the developer) and we have type safety, I'm thinking we can skip the console warnings for now. The defensive clamping seems okay although I'm not fully sure we need it yet. The main fix would be to avoid getting into this situation in the docsite's properties panel by having it read the 1 | 2 union type and render a selector instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] [bug-bash] Calendar/DateInput: numberOfMonths typed as 1|2 but no runtime enforcement (0 renders nothing, 1000 crashes)

2 participants