[Fix] DateInput: isTouchDevice should only be determined in componentDidMount
#336
Conversation
…onentDidMount`. Fixes #335.
ljharb
added
the
semver-patch: fixes/refactors/etc
ljharb
force-pushed
the
fix_dateinput_server_render
branch
from
aebef0f to
18a439e
Compare
| const { dateString } = this.state; | ||
| const { | ||
| dateString, | ||
| isTouchDevice: isTouch, |
There was a problem hiding this comment.
I'm confused about the need for this name change. Can't we just keep this as isTouchDevice?
There was a problem hiding this comment.
That identifier name is already used on line 5
| @@ -9,5 +9,6 @@ | |||
| "rules": { | |||
| "no-mixed-operators": [2, { "allowSamePrecedence": true }], | |||
| "jsx-a11y/no-static-element-interactions": 1, // TODO: enable | |||
| "react/no-did-mount-set-state": 0, // necessary for server-rendering | |||
There was a problem hiding this comment.
Can we exclusively put this on the DateInput instead of as a universal exception?
There was a problem hiding this comment.
I can, but it should be a universal exception, because it's necessary for any component to be server-rendered. I need to disable this rule upstream in the airbnb config too - setState in componentDidMount is a good thing.
There was a problem hiding this comment.
Really? we're fine with a double-render on first mount? I feel like that's something we should work against.
There was a problem hiding this comment.
Every single one of airbnb's hypernova components works like that already. We can definitely strive to avoid it, but it's not possible to avoid for client-required functionality.
There was a problem hiding this comment.
I guess that's fair, but given that this repo is small, I would prefer to turn it off on a case by case basis.
There was a problem hiding this comment.
If you prefer that I can do it - but we might need to make the same change in any component that has touch detection.
| const { dateString } = this.state; | ||
| const { | ||
| dateString, | ||
| isTouchDevice: isTouch, |
ljharb
force-pushed
the
fix_dateinput_server_render
branch
from
18a439e to
d978e54
Compare
Fixes #335.