Heads-up: 1.7.0 was a breaking change for us
gouegd opened this issue · 2 comments
Hi ! I just want to share this for other people who will run into a similar issue.
We use code similar to this (simplified):
const StyledElement = styled.div`
cursor: ${ifProp('onClick', 'pointer', 'inherit')}
`
This worked fine with 1.6.0, but in 1.7.0 we had some weird issue: our router was navigating to the wrong page, seemingly with no reason. Turns out this is because styled-tools will now call the our onClick prop (or anything that looks like a function) i n relation to this new feature: #55
So in our case, it was calling a route change with improper parameters (something like /user/undefined), simply by rendering a component.
We fixed it by reverting to the standard styled-components callback:
const StyledElement = styled.div`
cursor: ${({ onClick }) => onClick ? 'pointer' : 'inherit'}
`
We could also have ensured the prop we use is a boolean (perhaps using an additional prop).
This may impact a few places in our code, so we're thinking of making our own simple version of ifProp that respects the legacy behaviour, so that changing our existing code is easier.
Just found out we have the same issue on another component.
To give you a bit of context: this is a Tag component that displays a simple tag (similar to github issue labels). Optionally clicking it may have a side effect, and in this case hovering the tag will toggle the tag's colour. That's where we typically used an ifProp on onClick, which, counting on Javascript's coercion rules, worked fine with any falsy/truthy values, including functions being present or not.
That's it :) I'm not asking for a fix or anything, I just give those details for the few others that did sometimes use styled-tools the way we do.