Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New NavLink's className syntax when used with styled-components #8161

Closed
wojtekmaj opened this issue Oct 22, 2021 · 6 comments
Closed

New NavLink's className syntax when used with styled-components #8161

wojtekmaj opened this issue Oct 22, 2021 · 6 comments

Comments

@wojtekmaj
Copy link

Not really sure under which repo to file this - while technically it's styled-components not recognizing the pattern, it's React-Router that introduced this non-standard way of handling className prop in the first place.

Version

v6.0.0-beta.7

Test Case

Steps to reproduce

Read through the 1st example provided. There are 6 links created using styled-components styled.a:

  • 2 normal,
  • 2 rendered "as" NavLink,
  • 2 rendered "as" NavLink and active.

In each group, one of them is pink thanks to additional className "pink".

Now, open the 2nd example provided, which was upgraded to React-Router v6. There's only one important change in this example:

-   activeClassName="active"
-   className={className}
+   className={({ isActive }) => clsx(className, { active: isActive })}

and poof! It doesn't work, as styled-component stringifies the function.

Expected Behavior

image

Actual Behavior

image

@timdorr
Copy link
Member

timdorr commented Oct 22, 2021

This isn't an issue with React Router. styled-components isn't forwarding the className prop as a function. There's nothing we can do on our end to fix this.

@timdorr timdorr closed this as completed Oct 22, 2021
@Mario-Eis
Copy link

@timdorr We are facing the exact same issue in several projects. We are mainly using the emotion style library and styled components.
ClassName is supposed to be a name (string), as its prop name suggests. That is a convention I never saw broken in all those years. Accordingly I understand why style libraries are not passing functions as a name. Nor does classnames, clsx, cx or other className libraries I know of. At least not according to their documentation (if I am wrong, please feel free correct me).
So in all projects I am participating, NavLink was perfectly fine in v5, but is a worthless and deprecated component in v6.

@Mario-Eis
Copy link

Mario-Eis commented Nov 10, 2021

@wojtekmaj Here is our first workaround from our code bases. Maybe this helps you more than "the others are to blame".

const DirtyHackForNavLinkThatAcceptsAFunctionAsAClassName: FunctionComponent<NavLinkProps> = ({className, ...props}) => {
    const checkActive = useCallback(({isActive}) => isActive ? cx(className as string, "active-link") : className as string, [className]);
    return <NavLink  {...props} className={checkActive}/>
}

const StyledNavLink = styled(DirtyHackForNavLinkThatAcceptsAFunctionAsAClassName)`
  text-decoration: none;

  &.active-link .link-text {
    color: ${({theme}) => theme.palette.primary.main};
  }
`;

There are still some hacky hacks and casts for typescript to accept that workaround. So maybe building a NavLink Component from scratch using hooks would be more effective.

Update: Here is an example for a custom active Link from the docs:
https://stackblitz.com/github/remix-run/react-router/tree/main/examples/custom-link?file=src/App.tsx
So I guess that could be the new way to go.

@jacobedawson
Copy link

This is pretty annoying, but there is a quick way to get an active class, I'll paste the code I'm using (after spending an hour trying to find a way to make emotion & react-router isActive play nice)..

import { Link as RouterLink, useLocation } from "react-router-dom";
const location = useLocation();
// other code
{sidebarData.map(({ icon, text, route }) => {
          return (
            <RouterLink
              key={text}
              to={route}
            >
              <HStack spacing={4}>
                <Text>{icon}</Text>
                // I needed to target this nested component when the route is active
                <Text className={location?.pathname === route ? "xxx" : ""}>
                  {text}
                </Text>
              </HStack>
            </NavLink>
          );
        })}

Where "sidebarData" is for example:

const sidebarData = [
  {
    icon: "route-icon",
    text: "My Route",
    route: "/my-route"
  }
]

Ideally styled components / emotion wouldn't transform the function passed to className into a string, and possibly there are smarter ways to do this, but for getting the job done this works.

Hope it helps a fellow googler on a deadline ;)

@Haraldson
Copy link

Haraldson commented Dec 8, 2021

This really bothers me as well. React Router shouldn’t be so opinionated that it breaks styled-components or other popular ecosystem libraries and conventions. Nowhere else is className a function that is being called with a piece of state. Passing a simple prop is infinitely simpler and works everyhere.

I’m guessing it’s too late to revert this decision, but are there any new official recommendations for how to go about solving this, which does not involve manually checking for string equality?


Edit: Found a simpler solution over in this styled-components issue: Just apply styles using the aria-current attribute React Router adds to the DOM:

export const Tab = styled(NavLink)`


    &[aria-current] {
        color: red;
    }
`

@mqklin
Copy link

mqklin commented Mar 17, 2022

Active NavLink also has .active className. I hope this won't be broken in the future / there will be an official way to integrate with styled-components.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants