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

[Bug]: react/jsx-no-target-blank fix does not add "noopener" #3803

Open
2 tasks done
rafael-nogueras opened this issue Aug 15, 2024 · 3 comments
Open
2 tasks done

[Bug]: react/jsx-no-target-blank fix does not add "noopener" #3803

rafael-nogueras opened this issue Aug 15, 2024 · 3 comments

Comments

@rafael-nogueras
Copy link

Is there an existing issue for this?

  • I have searched the existing issues and my issue is unique
  • My issue appears in the command-line and not only in the text editor

Description Overview

The --fix behavior for the react/jsx-no-target-blank rule only adds rel="noreferrer" to all instances of target="_blank" without a rel property; however, it should ALSO add noopener.

Here is a snippet of my .eslint configuration:

{
  "settings": {
    "linkComponents": ["Link", "ClickableTile", "Button"]
  },
  "rules": {
    "react/jsx-no-target-blank": "error"
  },
  "globals": {
    "JSX": true
  }

FWIW, the fixed entries with the above behavior are React <Link> elements.

The fix is partial: instead of adding rel="noopener noreferrer", the --fix flag only adds rel="noreferrer".

Command used:

eslint --cache --cache-location ../../node_modules/.cache/eslintrc/shared . --ext .js,.jsx,.ts,.tsx

Expected Behavior

The --fix flag should add rel="noopener noreferrer" when it finds target="_blank" without a rel property.

<Link target="_blank" href={url}>text</Link>

should be fixed to:

<Link target="_blank" rel="noopener noreferrer" href={url}>text</Link>

Also, it would be good if the rule always placed the rel property immediately AFTER the target property: it currently mostly puts it at the end of the attribute list.

eslint-plugin-react version

v7.35.0

eslint version

v7.31.0

node version

v20.11.1

@akulsr0
Copy link
Contributor

akulsr0 commented Aug 20, 2024

@rafael-nogueras For custom link components, you also need to pass linkAttribute. Can you try following:

{
  settings: {
      linkComponents: ['Link', 'ClickableTile', 'Button'],
      linkAttribute: 'href',
  },
}

Also,

the react/jsx-no-target-blank rule only adds rel="noreferrer" to all instances of target="_blank" without a rel property; however, it should ALSO add noopener.

noreferrer is superset of noopener, so its fine. Read more here

@rafael-nogueras
Copy link
Author

@akulsr0 Oh, interesting: I am not passing the linkAttribute property and noreferrer is being added just fine with --fix. Maybe href is the default value, if unspecified?

Re: not adding noopener: is there any harm in adding noopener as well, even if noreferrer is a superset? For instance, we are using a security scan tool which apparently triggers when it doesn't find both noopener and noreferrer: I had to use the --fix rule and then manually do a text replace to add the noopener.

Can the noopener be added as well, for backwards compatibility? Does it harm anything to add both keywords?

@ljharb
Copy link
Member

ljharb commented Sep 11, 2024

href is definitely the default value.

Typically eslint rules don't have options that do nothing but change the autofix. However, an option that makes it so that noopener is always required, whose autofix adds it, seems like it'd do what you want.

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

No branches or pull requests

3 participants