Add option for passed hrefs (as in Next.js) (anchor-is-valid)

[sk22]

I'd suggest adding an option for a tags that don't have an href because its containing component passes the href to it.

For example, Next.js' Link component takes the href but requires an a tag as a child:

<Link href="/about">
  <a>About</a>
</Link>

Creating such structure triggers the anchor-is-valid rule:

[eslint] The href attribute is required on an anchor. Provide a valid, navigable address as the href value. (jsx-a11y/anchor-is-valid)

At the moment, I don't see any practical usage of the anchor-is-valid rule that allows passing the href to the child.

A possible fix could be to check if the parent component (i.e. Link) has the passHref prop set (which is optional in Next.js when the child is an a tag without an own href).
Other than that, the rule could just check for the <Link href><a></Link> structure.

[sk22]

Here's a workaround (i.e., disabling the rule's noHref aspect):

"jsx-a11y/anchor-is-valid": [ "error", {
  "components": [ "Link" ],
  "specialLink": [ "hrefLeft", "hrefRight" ],
  "aspects": [ "invalidHref", "preferButton" ]
}]

[ljharb]

That sounds like a really strange API; if it’s a Link why would it need an a tag to be passed in? All that does is force a cloneElement, needlessly

[sk22]

It's indeed a strange API. The reason they did it like this is probably so the Link element can be rendered as any, not only an anchor. But there's an as prop on Link, I'll just check out if this can be used instead. Edit: Nevermind as; seems like it does something completely else.

[sk22]

Btw; yep, what you said is literally what the Next.js Link does... https://github.com/zeit/next.js/blob/canary/lib/link.js#L149
Also see this commit: vercel/next.js@6431f5f

[ljharb]

I’m not sure why they wouldn’t accept a component (or string “a”) so they can render the element directly rather than cloning :-/

Either way I’m not sure that supporting this generically is something either possible or reasonable to do.

[sk22]

I see... Thanks!

[ljharb]

I’ll leave this open if somebody has an idea of how to handle it, but hardcoding an arbitrary library’s strange convention is a nonstarter.

[trevordmiller]

I'm not sure how to handle this but I'm also running into the same issue:

[danny-andrews-snap]

Yeah, this is really a fault of the next.js Link API. I submitted an issue. vercel/next.js#5533

[T04435]

Here's a workaround (i.e., disabling the rule's noHref aspect):

"jsx-a11y/anchor-is-valid": [ "error", {
  "components": [ "Link" ],
  "specialLink": [ "hrefLeft", "hrefRight" ],
  "aspects": [ "invalidHref", "preferButton" ]
}]

This will disable all linting a tags without href, is there a way to:

<Link href='/'><a>Disable lint error</a></Link>
<a>Enable lint error</a>

[takebo]

@T04435 looking for the same solution.
Disabling eslint for the nested anchor inside the Link, and not globally.

[trigunam]

In response to @sk22 adding the following to .eslintrc.json still causes errors.

"invalidHref", "preferButton"

Does this also mean, eslint is suggesting to use <button> instead of <a> tag right?
For Next.js will replace a button work within <Link>?

I would rather turn off the rule to make it work with Next.js.

[ljharb]

If it has a URL to navigate to, it should be an a, which in next.js and react-router means a Link. A button should only be for non-navigation behavior.

[FDiskas]

If it has a URL to navigate to, it should be an a, which in next.js and react-router means a Link. A button should only be for non-navigation behavior.

But why not? Button can be used to redirect user to some page. For example the form usually has a "submit" button and "cancel" - and end user by pressing cancel button is waiting to be redirected out of the form and "submit" button also should submit and redirect somewhere. It's a natural behavior.

[ljharb]

@FDiskas because that’s not how semantic html works. Buttons are for submitting forms, and that’s not navigation. Cancel buttons shouldn’t exist; that should either be a link, or the normal browsers’ reload button.

[burntcustard]

The suggested linting workaround to disable the noHref check works, but there are additional checks done on <a>s that can give more errors. For example, adding an onClick={} to an <a> triggers multiple rules incorrectly, including "Static HTML elements with event handlers require a role" (no-static-element-interactions), because anchors "without" hrefs are static elements.

While this is obviously a weirdness with Next.js' Link component inserting the href into the <a> after jsx-a11y is run, a potential solution could be some sort of config to disable all rules for children of particular components that jsx-a11y doesn't understand?

[MartinDevillers]

An alternative (but hacky) solution is to enable the passHref property and set the inner href to a dummy value. Next.js will override the dummy value with the correct value from the outer component, so everything will work correctly and it'll pass the jsx-a11y/anchor-is-valid rule.

  <Link href="/my-amazing-page" passHref>
    <a href="dummy">Go to my amazing page</a>
  </Link>

[zackdotcomputer]

I just put my hacky solution up as a gist - I've been using a wrapper component that internally creates the <Link> and <a> components together. This way you can add the LinkTo component to this rule's components list and it will validate properly.

[wheelmaker24]

As usually you won't use the Link component on many pages (mostly only within a Navigation or Layout component), you could also easily disable the next line:

return (
    <Link href={yourUrl}>
        {/* eslint-disable-next-line jsx-a11y/anchor-is-valid */}
        <a {...yourProps}>{yourLinkName}</a>
     </Link>
);

[zackdotcomputer]

@wheelmaker24 Agreed, a one-by-one disable or the passHref solution above both work fine if your site only needs a few Links and/or they're gathered into a couple central components.

I don't think those are going to be efficient if you're building a site that uses links in more contexts, though, like an ecommerce or a webapp. If you have Links all over the place you're basically stuck either turning off the rule in your config or pulling in a wrapper component to work around the issue until Next or ESLint can fix this. :-\

[ljharb]

There's nothing we can really do except hardcoding the next.js pattern, which is all kinds of brittle. The proper solution is for next.js to follow the react best practice that long predates their existence: avoid patterns that necessitate cloning elements.

[zackdotcomputer]

No disagreement on what the ideal solution is @ljharb, but I disagree that hardcoding an exception for the Next.js pattern is the only non-brittle solution. If we could not only add certain components to the scope of this rule (as the "components" argument allows us to) but also to exclude <a> components (which we are specifically not allowed to do currently), then we could configure this rule to work with the Next.js pattern on our side as the end-user without needing to make any hardcoding to this project.

[ljharb]

@zackdotcomputer true. but then users would be able to exclude <a> components when not using this specific Next.js pattern, and I don't think that downside is worth it.

[zackdotcomputer]

If you're worried about missing un-nested <a> tags in a Next.js project, I know we could find a way to make the configuration sufficiently targeted (e.g. make it specifically "ignore <a> tags that are direct children of these tags...").

If you're just worried about developers being able to disable <a> validation in a more general way, I think it'd still be a net upside if a flag like that could rescue developers who would otherwise disable <a> validation by just disabling this rule entirely. Even as a developer who cares enough about a11y and linting to be here in this thread, if a rule is fighting my tech stack and I can't configure them to play nicely, I'm going to turn off the rule before I swap out the tech stack.

[ljharb]

Hmm. I suppose if:

• Link is in the components setting
• the <a> is missing an href, but is directly contained as the only child in an otherwise-valid link component
• a new option is provided to enable this behavior

then perhaps the risk is minimal, and the only downside would be that we'd have to maintain a rule option forever because a single framework made a poor design choice and hasn't fixed it after a number of years :-/

[zackdotcomputer]

¯\_(ツ)_/¯ yup, that's about the speed of things. Just eslint's bad luck as a build-tool that the framework that made that bad choice also happened to become super popular.

Since I'm having trouble reading the tone of voice on your post as to whether you will or won't consider adding this option, I'm going to open a PR to add a "Case: I use Next.js Link Components" to the description of this rule so that folks can more easily find this thread of workarounds. If this option winds up getting added, that case can be updated to point users to it.

[T04435]

This is my work around

// MyLink.tsx
import React, { PropsWithChildren } from 'react';
import Link, { LinkProps } from 'next/link';

// This will allow you to pass any other props that the next Link supports
export type MyLinkProps = LinkProps;

const MyLink = (props: PropsWithChildren<MyLinkProps>) => {
  const { children, ...linkProps } = props;
  return <Link {...linkProps} passHref ><a href="passRef">{children}</a></Link>;
};

export default MyLink;


# usage

<MyLink href="/about" scroll >About</MyLink>

[ljharb]

@zackdotcomputer your tone read is accurate, as I'm not convinced either way myself, as every outcome seems differently terrible. I'll take a look at the PR.

[chiptus]

I know this problem from nextjs, but just started using @ui-router/react for an hybrid angularjs application (I know, ugh), and it has the same api.

[zackdotcomputer]

@chiptus I noticed that the new NextJS eslint env turns off this rule since it can't be configured to work with their API. I think the unfortunate end state here for at least the near future is that this rule will have to be disabled if one is using a framework that follows the "wrapper component takes the href" pattern.

That pattern doesn't seem to be going away and there doesn't appear to be any movement to alter this rule to support it.

[Gamote]

I'm also looking 👀 for a way to deal with this. My current work around looks like this:

import Link, { LinkProps } from "next/link";
import React, { FC } from "react";

type NextLinkProps = LinkProps & {
  href: string;
  className?: string;
};

/**
 * Standard way of using the Next's `Link` tag together with the `a` tag
 */
const NextLink: FC<NextLinkProps> = ({ href, className, children, ...rest }) => (
  <Link href={href} {...rest}>
    <a href={href} className={className}>
      {children}
    </a>
  </Link>
);

export default NextLink;

Usage:

<NextLink href={"/register"} className="font-medium">
  Register
</NextLink>

[ljharb]

This seems resolved, both on the next side in a future major and on this side via config. I’ll reopen if that’s not the case.

[visnup]

This seems resolved, both on the next side in a future major and on this side via config. I’ll reopen if that’s not the case.

Could you link to the respective changes in Next and config you're referring to if you have them handy?

[ljharb]

vercel/next.js#8207 (comment)

The needed config is mentioned multiple times upthread.