[anchor-is-valid] Allow Object spread in href/specialLink values

[caub]

We get this error Cannot read property 'expression' of undefined as eslint output when using

const a = {};
const foo = () => (
    <Link
        to={{
            pathname: '/bar',
            query: { ...a },
        }}
    />
);

and .eslintrc:

{
    "extends": [
        "eslint:recommended",
    ],
    "plugins": ["jsx-a11y"],
    "env": {
        "browser": true,
        "node": true
    },
    "parserOptions": {
        "ecmaVersion": 2018,
        "sourceType": "module",
        "experimentalObjectRestSpread": true,
        "ecmaFeatures": {
            "jsx": true
        }
    },
    "rules": {
        "no-unused-vars": "warn",
        "jsx-a11y/anchor-is-valid": ["warn", {
            "components": ["Link"],
            "specialLink": [ "to" ],
            "aspects": ["noHref", "invalidHref", "preferButton"]
        }]
    }
}

here's a passing test:

{
  code: '<Anchor hrefLeft={{ foo: { test: 1 } } }} />',
  options: componentsAndSpecialLinkAndNoHrefAspect,
},

and a failing test, when object spread is used (even with experimentalObjectRestSpread in parserOptions or babel-eslint as parser with the right .babelrc..):

{
  code: '<Anchor hrefLeft={{ foo: { ...{ test: 1 } } }} />',
  options: componentsAndSpecialLinkAndNoHrefAspect,
},

Message:
      A fatal parsing error occurred: Parsing error: Unexpected token ...

I reported the error in eslint originally: https://github.com/eslint/eslint/issues/9960#issuecomment-365179752

@AlmeroSteyn I can try to solve it, if you have any advices maybe?

[stefanmaric]

I'm also experiencing this.

[ljharb]

This is an interesting request; a url is a string, and that’s what this is meant to enforce.

I think if react-router’s Link is sufficiently different from an <a>, then it warrants its own react-router-specific rule.

[AlmeroSteyn]

@caub Wow sorry somehow missed the @ CC

Yes, as @ljharb mentions, it was not built to cater for objects in the link prop.

The React Router Link component renders to an accessible <a> tags and the to prop is guarded with a prop-type isRequired declaration and, when omitted, it will be reported on by React itself as can be seen on the following console capture:

I am not sure if this should be double checked by this linter as when the React error is dealt with, an accessible <a> will be rendered.

Thoughts?

[justinanastos]

I created a repo repo with this issue before I realized there was already any issue. Here's the link if it helps anyone:

https://github.com/justinanastos/eslint-plugin-jsx-a11y-anchor-is-valid-error

---

That being said, I'd like to throw my opinion into the ring: whether or not this rule should accurately lint the to={[ Object ]} case, this lint plugin should not crash eslint. I think that would mean that using to={[Object ]} would make this rule incompatible with aspects: ['invalidHref'], which completely makes sense. One can chose to not include the invalidHref aspects in their configuration as long as this rule doesn't crash. Right now, if you include Link in components and then ever use an object in to=, this will crash eslint regardless of the other configuration options.

If the maintainers are interested in a workaround, I'd be happy to submit a PR; I just need some guidance on what would be acceptable.

[ljharb]

I absolutely agree that it should not crash the plugin; a PR that prevents that (without silently allowing code that failed to pass before) would be great.

[kaitlynbrown]

Any updates on this?

[ljharb]

No, nobody’s submitted a PR yet.

[shanecav84]

Possibly related to https://github.com/evcohen/jsx-ast-utils/issues/69. Workaround involves extracting to into a variable and passing that variable to the component prop; e.g.:

const a = {};
const to = {
  pathname: '/bar',
  query: { ...a },
}
const foo = () => <Link to={to}/>;

Admittedly, not all that practical if you have a lot of to props with spread objects.

[jessebeach]

This one is really throwing me for a loop. I tried updating the parser config, but that doesn't solve the parser error. Updating eslint doesn't solve it either:

{
  extends: [
    "airbnb-base",
    "plugin:flowtype/recommended"
  ],
  parser: "babel-eslint",
  parserOptions: {
    ecmaVersion: 2018,
    ecmaFeatures: {
      experimentalObjectRestSpread: true,
    }
  },
}

Has anyone figured out the magical incantation to at least get the code to parse? Then I can fix the code error that occurs after that.

[jessebeach]

Alright, this should get the test runner to at least parse the failing case. Now to fix the failing case :)

573

[jessebeach]

I think https://github.com/evcohen/jsx-ast-utils/pull/74 solves the Cannot read property 'expression' of undefined error that @caub notes in the OP.

EDIT: Nope, I just reproduced the OP. It's in evcohen/jsx-ast-utils. I'll get it fixed.

[caub]

Nice if it get fixed!

ps: I wonder if the spread error still happens with:

const a = {};
const foo = () => (
    <Link to={'/bar?' + new URLSearchParams({ ...a })} />
);

I guess it does

[jessebeach]

I've got a pull against jsx-ast-utils to resolve the error: https://github.com/evcohen/jsx-ast-utils/pull/75

[jessebeach]

@caub yes, that element pass as well with the fix:

./node_modules/.bin/jest ./__tests__/src/rules/anchor-is-valid-test.js
 PASS  __tests__/src/rules/anchor-is-valid-test.js
  anchor-is-valid
    valid
      ✓ <Link to={"/bar?" + new URLSearchParams({ ...a })} /> (59ms)

Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total
Snapshots:   0 total
Time:        1.126s, estimated 2s