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

fix: [a11y-input-has-name-attribute]Inputにreact-hook-formのregisterが指定されている場合はエラーにならないようにする #490

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yamakeeeeeeeeen
Copy link

やりたいこと

  • ESLintプラグイン「a11y-input-has-name-attribute」は、Input に name 属性が指定されていない場合にエラーを出力します。
  • react-hook-form を使用する際、register 関数の引数で name を指定することで との紐づけを行いますが、現行ルールでは以下のようなコードもエラーとして扱われてしまいます:
    • <Input {...register('name')} />
  • この問題を解消し、react-hook-form の register 関数が使用されている場合は name 属性を明示的に渡さなくてもエラーにならないようにしたいと考えています。

やったこと

  • Input に react-hook-form の register 関数が渡されている場合は、エラーを出さないように ESLint のルールを修正しました。

@yamakeeeeeeeeen yamakeeeeeeeeen requested a review from a team as a code owner January 15, 2025 01:09
@yamakeeeeeeeeen yamakeeeeeeeeen requested review from nabeliwo, s-sasaki-0529 and AtsushiM and removed request for a team January 15, 2025 01:09
@yamakeeeeeeeeen yamakeeeeeeeeen force-pushed the use-a11y-input-has-name-attribute-with-react-hook-form-register branch from fabb54b to 3eddb5a Compare January 15, 2025 01:27

if (nodeName.match(TARGET_TAG_NAME_REGEX)) {
const nameAttr = node.attributes.find(findNameAttr)
const nameAttr = attributes.find(findNameAttr)
Copy link
Member

Choose a reason for hiding this comment

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

ロジック自体は大丈夫そうですが、attributesに対するチェックを最適化すると良いかも。
今の方法だと複数回attributesに対して操作が走る可能性があります。

let nameAttr = null
let hasSpreadAttr = false
let hasReactHookFormRegisterSpreadAttr = false
let hasRadioInput = false

attributes.forEach((attr) => {
  if (a.type === 'JSXSpreadAttribute') {
    hasSpreadAttr = true
    
    if (hasReactHookFormRegisterSpreadAttr == false && a.argument?.callee?.name === 'register') {
      hasReactHookFormRegisterSpreadAttr = true
    }
  } else {
    switch (a.name?.name) {
      case 'name': {
        nameAttr = a
        break
      }
      case 'type': {
        if (a.value.value === 'radio') {
          hasRadioInput = true
        }
        break
      }
    }
  }
})

Copy link
Member

Choose a reason for hiding this comment

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

この方法ならattributesに対する検査は1loopだけで済むので高速化できます

Copy link
Author

Choose a reason for hiding this comment

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

@AtsushiM 添付のコードにあるhasSpreadAttrhasRadioInputにはbooleanを代入するだけでどこでも使われていないようなのですが、どういった意味があるのでしょうか?

@Tokky0425 Tokky0425 changed the title [a11y-input-has-name-attribute]Inputにreact-hook-formのregisterが指定されている場合はエラーにならないようにする fix: [a11y-input-has-name-attribute]Inputにreact-hook-formのregisterが指定されている場合はエラーにならないようにする Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants