react Active class name applied, even when `react-router` doesn't support it

Describe the bug A clear and concise description of what the bug is.

Note: As this is a public repo, please be careful not to include details or screenshots from unreleased GitHub products or features. In most cases, a good bug report should be able to describe the new component without including business logic or feature details, but if you must discuss context relating to an unreleased feature, please open an issue in the private Design Systems repo and link to it here.

When using React Router links, with Primer components, primer tries to forward an activeClassName, as was exposed in the v5 and lower apis. React router v6 no longer supports this classname, so it gets dropped onto the dom, and causes the console warning in dev Warning: React does not recognize the activeClassNameprop on a DOM element. If you intentionally want it to appear in the DOM as a custom attribute, spell it as lowercaseactiveclassname instead. If you accidentally passed it from a parent component, remove it from the DOM element.

A workaround, also in the sandbox, exists, by manually filtering it, but that's a bit annoying

import "./styles.css";
import { TabNav } from "@primer/react";
import { BrowserRouter, Link } from "react-router-dom";

const FilteredActiveClassName = ({ activeClassName, ...props }) => {
  return <Link {...props} />;
};
export default function App() {
  return (
    <BrowserRouter>
      <div className="App">
        <TabNav>
          <TabNav.Link as={Link} to="/">
            No filtering of prop
          </TabNav.Link>
          <TabNav.Link as={FilteredActiveClassName} to="/">
            Filtered dom prop
          </TabNav.Link>
        </TabNav>
        <h2>Start editing to see some magic happen!</h2>
      </div>
    </BrowserRouter>
  );
}

<nav class="TabNav__TabNavBody-sc-1ab407u-1 gXszLR">
    <a activeclassname="selected" class="TabNav__TabNavLink-sc-1ab407u-2 eatcUG TabNav-item" href="/">
        No filtering of prop
    </a>
    <a class="TabNav__TabNavLink-sc-1ab407u-2 eatcUG TabNav-item" href="/">
        Filtered dom prop
    </a>
</nav>

To Reproduce I built a small sandbox for this, related to tab nav, but it seems other components like sidenav do similar

https://codesandbox.io/s/still-https-5p2wb?file=/src/App.js

Expected behavior activeClassName is not passed if not necessary (or not included on the as={} component)

Screenshots If applicable, add screenshots to help explain your problem.

Screen Shot 2022-02-03 at 11 17 11 AM

Desktop (please complete the following information):

OS: [e.g. iOS]
Browser [e.g. chrome, safari]
Version [e.g. 22]

Smartphone (please complete the following information):

Device: [e.g. iPhone6]
OS: [e.g. iOS8.1]
Browser [e.g. stock browser, safari]
Version [e.g. 22]

Additional context Add any other context about the problem here.

Feb 03 '22 16:02 mattcosta7

Ouch! Thanks for reporting this, marked this as a bug and added it to project for the team to pick up.

Feb 03 '22 16:02 siddharthkp

We think it would be sensible to remove the dependency on react-router from Primer altogether. Would you agree @mattcosta7? If so, activeClassName setting would be removed during that process. This would usually force a breaking change in the library, so in the interim could you apply the prop filtering client-side as the workaround?

import { BrowserRouter, Link } from "react-router-dom";

In v5 of react-router, using NavLink instead of Link should stop that error from occurring too.

Feb 22 '22 14:02 rezrah

Yes! I definitely agree it makes sense to remove this entirely - it seems weird for primer to handle this, instead of having users define their own.

we already use the workaround, so no issue from us at all!

Feb 22 '22 15:02 mattcosta7

A person can create a new <NavLink /> component with the v5 props described here as another option.

Jun 06 '22 11:06 pco86

Hi! This issue has been marked as stale because it has been open with no activity for 180 days. You can comment on the issue or remove the stale label to keep it open. If you do nothing, this issue will be closed in 7 days.