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

Update rules for flat config #596

Merged
merged 2 commits into from
Jan 6, 2025
Merged

Update rules for flat config #596

merged 2 commits into from
Jan 6, 2025

Conversation

gracepark
Copy link
Contributor

Closes: #589

This now updates the rules as ESLint has updated the rules API for custom rules: https://eslint.org/blog/2023/09/preparing-custom-rules-eslint-v9/

Tested and looks like it's now working as expected:

/workspaces/eslint-plugin-github/test-examples/flat/src/forEachTest.js
  1:1  error  Filename 'forEachTest.js' does not match the regex naming convention  github/filenames-match-regex

/workspaces/eslint-plugin-github/test-examples/flat/src/getAttribute.js
  13:3   error  event.preventDefault() inside an async function is error prone                  github/async-preventdefault
  21:4   error  "scroll" event listener is not cancellable and so `passive: true` does nothing  github/no-useless-passive
  30:16  error  event.currentTarget inside an async function is error prone                     github/async-currenttarget

@gracepark gracepark self-assigned this Jan 6, 2025
@Copilot Copilot bot review requested due to automatic review settings January 6, 2025 16:31
@gracepark gracepark requested a review from a team as a code owner January 6, 2025 16:31
@gracepark gracepark requested a review from manuelpuyol January 6, 2025 16:31
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.

Files not reviewed (1)
  • lib/rules/async-currenttarget.js: Evaluated as low risk

Tip: Turn on automatic Copilot reviews for this repository to get quick feedback on every pull request. Learn more

const data = await fetch(url)

// But now, event.currentTarget will be null
const text = event.currentTarget.getAttribute('data-text')
Copy link
Preview

Copilot AI Jan 6, 2025

Choose a reason for hiding this comment

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

event.currentTarget might be null after the await call. Add a null check before accessing getAttribute.

Suggested change
const text = event.currentTarget.getAttribute('data-text')
if (event.currentTarget === null) return;
const text = event.currentTarget.getAttribute('data-text')

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@gracepark gracepark merged commit 1b65401 into main Jan 6, 2025
7 checks passed
@gracepark gracepark deleted the update-flat-config-rules branch January 6, 2025 22:43
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.

TypeError: context.getScope is not a function (ESLint 9 compatibility issues)
2 participants