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

🐛 Biome --apply Command Hangs Due to Incorrect rel Attribute Recommendation #2675

Closed
1 task done
luarmr opened this issue May 2, 2024 · 4 comments · Fixed by #2687
Closed
1 task done

🐛 Biome --apply Command Hangs Due to Incorrect rel Attribute Recommendation #2675

luarmr opened this issue May 2, 2024 · 4 comments · Fixed by #2687
Assignees
Labels
A-Linter Area: linter L-JavaScript Language: JavaScript and super languages S-Bug-confirmed Status: report has been confirmed as a valid bug

Comments

@luarmr
Copy link

luarmr commented May 2, 2024

Environment information

CLI:
  Version:                      1.7.2
  Color support:                true

Platform:
  CPU Architecture:             aarch64
  OS:                           macos

Environment:
  BIOME_LOG_DIR:                unset
  NO_COLOR:                     unset
  TERM:                         "xterm-256color"
  JS_RUNTIME_VERSION:           "v18.15.0"
  JS_RUNTIME_NAME:              "node"
  NODE_PACKAGE_MANAGER:         "yarn/1.22.19"

Biome Configuration:
  Status:                       Loaded successfully
  Formatter disabled:           false
  Linter disabled:              false
  Organize imports disabled:    false
  VCS disabled:                 true

Linter:
  Recommended:                  true
  All:                          false
  Rules:
Workspace:
  Open Documents:               0

What happened?

In a specific situation biome get stuck and don't exit.

Steps to reproduce:

  • Setup a project with Biome and the rule lint/a11y/noBlankTarget enabled.
  • Include the following JSX code:
<a href="https://example.com" rel="noreferer" target="_blank">
      Visit Example
</a>
  • Run the command yarn biome check --apply .

Why this happend

When the --apply option is not used, the command suggests adding "noreferrer" for security reasons.

  ✖ Avoid using target="_blank" without rel="noreferrer".

    5 │ 		<div>
    6 │ 			<h1>Welcome to My React App!</h1>
  > 7 │ 			<a href="https://example.com" rel="noreferer" target="_blank">
      │ 			                                              ^^^^^^^^^^^^^^^
    8 │ 				Visit Example
    9 │ 			</a>

  ℹ Opening external links in new tabs without rel="noreferrer" is a security risk. See the explanation for more details.

  ℹ Safe fix: Add the "noreferrer" to the existing attribute.

    7 │ → → → <a·href="https://example.com"·rel="noreferrer·noreferer"·target="_blank">
      │

However, if the suggestion is applied and the command is run again, it leads to an erroneous and repetitive rel attribute suggestion, causing a infinite loop in recommendation if you continue applying them.

    7 │ → → → <a·href="https://example.com"·rel="noreferrer·noreferrer noreferer"·target="_blank">   

Current behavior

The command never completes.

Examples

Playground to see the rule

Also create a small repo to easy reproduce: https://github.com/luarmr/reproduceIssueBiome/

Expected result

OR The rel attribute noreferrer is added rel="noreferrer noreferer" without duplication
OR The error is mark for manual fix

Code of Conduct

  • I agree to follow Biome's Code of Conduct
@Sec-ant Sec-ant added A-Linter Area: linter L-JavaScript Language: JavaScript and super languages S-Bug-confirmed Status: report has been confirmed as a valid bug labels May 2, 2024
@Conaclos
Copy link
Member

Conaclos commented May 2, 2024

You made a typo: it is noreferrer, not noreferer (two rr). The action is repeated on the playground because you copied the output of your terminal: in the terminal output, Biome replaces a space with a middle dot ·. If you copy the fixed version, you can note that the action is not repeated.

@Conaclos Conaclos removed A-Linter Area: linter L-JavaScript Language: JavaScript and super languages S-Bug-confirmed Status: report has been confirmed as a valid bug labels May 2, 2024
@Conaclos Conaclos closed this as completed May 2, 2024
@luarmr
Copy link
Author

luarmr commented May 2, 2024

@Conaclos @Sec-ant Please reopen, I believe I've written so much on the description of the issue that the main point might getting lost.

You made a typo: it is noreferrer, not noreferer (two rr).

I am aware of that

If you copy the fixed version

Please be aware, this rule (lint/a11y/noBlankTarget ) it is a "safe fix" rule. If you use --apply, it will instantly implement the recommended changes and rerun It will continue to do this, stopping the command from ending. For example, the PyCharm extension can't correct the file (this or any other issues) because of this problem, and leaving biome running in the background.

Also, the command doesn't display why it is blocked, so finding the problem can take a long time.

@Conaclos Conaclos reopened this May 2, 2024
@Conaclos Conaclos added A-Linter Area: linter L-JavaScript Language: JavaScript and super languages S-Bug-confirmed Status: report has been confirmed as a valid bug labels May 2, 2024
@Conaclos
Copy link
Member

Conaclos commented May 2, 2024

I can confirm that biome lint --apply is hanging in your specific case.
Here a repro:

❯ cat biome.json 
{
	"$schema": "https://biomejs.dev/schemas/1.7.2/schema.json",
	"organizeImports": {
		"enabled": true
	},
	"linter": {
		"enabled": true,
		"rules": {
			"recommended": false,
			"a11y": {
				"noBlankTarget": "error"
			}
		}
	}
}

❯ cat main.jsx 
<a href="https://example.com" rel="" target="_blank"></a>

✖ npx biome lint --apply main.jsx
^C

@luarmr
Copy link
Author

luarmr commented May 3, 2024

@Conaclos @Sec-ant
The speed at which this was done is truly impressive!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Linter Area: linter L-JavaScript Language: JavaScript and super languages S-Bug-confirmed Status: report has been confirmed as a valid bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants