React-dates: Unknown props found: id warnings

[erin-doyle]

I'm using the react-a11y library and I'm getting the following PropType warnings from react-dates:

Warning: Failed prop type: DayPicker: unknown props found: id
Warning: Failed prop type: DayPickerNavigation: unknown props found: id
Warning: Failed prop type: CalendarMonthGrid: unknown props found: id
Warning: Failed prop type: CalendarMonth: unknown props found: id
Warning: Failed prop type: CalendarDay: unknown props found: id

The reason these id's are being added to these components is because the react-a11y library adds ids to all elements so that it can test for accessibility standards being met. I think it would be helpful if react-dates allowed ids as props for these components unless there is a really good reason it is not allowed. It would be really nice for this library to be compatible with a library that tests for and warns of violations to accessibility standards.

[ljharb]

In general, element IDs are an antipattern that allows for problematic CSS specificity as well as tight coupling of DOM to JS - allowing them is not a simple matter of "being compatible".

I'm confused; why does react-a11y need IDs instead of just inspecting the DOM? React adds its own react-id, can it not use that? @wyattdanger, any thoughts here?

[erin-doyle]

You make a really good point about ids as a practice and using a library specific id like React does. Though if react-a11y used some other *-id property react-dates would still complain about it.

After doing a little more looking at react-a11y (which I guess I should have done first, sorry...) it looks like it's not even being maintained anymore 😡. There is a fork, however, that does look like it's being maintained: romeovs/react-a11y. I will try that out to see if the ids are no longer being added and if they are, post an issue there about seeing if that can be changed. I don't know enough about how the code works to say if they are "necessary" or maybe just a poorly designed approach in the first place.

All that being said, it does still seem a little strict that react-dates prevents them. I guess I'm on the fence about understanding the strictness. I understand wanting to prevent users from screwing things up, but also if users do things they shouldn't maybe that's their problem?

[erin-doyle]

Also, feel free to close this as I probably went barking up the wrong tree in the first place! Mea Culpa! 😳 I am curious to hear more about the forbidding unknown props design and think it could be educational if you have a moment to explain? 🤔

[ljharb]

The philosophy behind using forbidExtraProps from airbnb-prop-types is that passing extra props is almost always a bug - either a typo, or someone thinking a prop will work when in fact, it will not (like, say, "onClick" or "id" or "className", or even something like "enableFunctionality").

Although I highly recommend making propType warnings fail tests, it's also worth noting that they aren't triggered in production, and are merely warnings otherwise, so it's not actually "restrictive", it's informational and helpful.

Specifically, without the propType warning, you might have merrily passed IDs and not known that we weren't looking at this.props.id, and they would have silently done nothing - with the warnings, you were notified that something was wrong.