Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

feat(rome_js_analyze): add noHeaderScope rule #3804

Merged
merged 2 commits into from
Nov 21, 2022

Conversation

kaioduarte
Copy link
Contributor

@kaioduarte kaioduarte commented Nov 20, 2022

Summary

Part of #3739

Closes #3815

Test Plan

Unit tests

@kaioduarte kaioduarte requested a review from a team November 20, 2022 17:27
@netlify
Copy link

netlify bot commented Nov 20, 2022

Deploy Preview for docs-rometools ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 8c655f2
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/637b816a8b3e73000b84e37d
😎 Deploy Preview https://deploy-preview-3804--docs-rometools.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

fn action(ctx: &RuleContext<Self>, jsx_attr: &Self::State) -> Option<JsRuleAction> {
let mut mutation = ctx.root().begin();

mutation.remove_node(jsx_attr.clone());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This simple approach can leave some whitespaces in the JSX element, and I saw that we kinda handle it on the noAutoFocus rule, but I wonder if it's worth the effort here as the formatter can take care of it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since users may opt-out of using the formatter and only enable the linter we try to ensure that code actions don't do something completely wrong with trivia like removing all whitespace, but it doesn't have to handle every edge case perfectly either.

fn action(ctx: &RuleContext<Self>, jsx_attr: &Self::State) -> Option<JsRuleAction> {
let mut mutation = ctx.root().begin();

mutation.remove_node(jsx_attr.clone());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since users may opt-out of using the formatter and only enable the linter we try to ensure that code actions don't do something completely wrong with trivia like removing all whitespace, but it doesn't have to handle every edge case perfectly either.

@ematipico ematipico merged commit 5c1c69d into rome:main Nov 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

noHeaderScope
3 participants