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

fix(rome_js_analyze): useExhaustiveDependencies support function syntax, or React.use* hooks and more. #4571

Merged
merged 9 commits into from
Jun 21, 2023

Conversation

nissy-dev
Copy link
Contributor

@nissy-dev nissy-dev commented Jun 14, 2023

Summary

This PR fixed most of #4330

useExhaustiveDependencies rule now works correctly

  • when the first argument of hooks is a named function
  • inside an export default function
  • for React.use* hooks

Test Plan

I added some snapshot tests.

Changelog

  • The PR requires a changelog line

Documentation

  • The PR requires documentation
  • I will create a new PR to update the documentation

@netlify
Copy link

netlify bot commented Jun 14, 2023

Deploy Preview for docs-rometools canceled.

Name Link
🔨 Latest commit e04e611
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/648eac3f2d474d00081542af

@github-actions github-actions bot added the A-Linter Area: linter label Jun 14, 2023
CHANGELOG.md Outdated Show resolved Hide resolved
// React.useXXX case
function MyComponent13() {
let a = 1;
React.useEffect(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should make sure that:

  • React is imported import React from "react", I believe we already have some API for that;
  • React is not created here, e.g.
const React = { useEffect() {} }
React.useEffect();

Can we create some test against these cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your review!

Can we create some test against these cases?

Yes. l'll try to update some codes to pass your test cases.

Copy link
Contributor Author

@nissy-dev nissy-dev Jun 18, 2023

Choose a reason for hiding this comment

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

I fixed in b644eb6!

@ematipico ematipico merged commit 3d61b99 into main Jun 21, 2023
@ematipico ematipico deleted the fix-false-postive-useExhaustiveDependencies branch June 21, 2023 08:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Linter Area: linter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants