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

add failing test (#18051) #19506

Conversation

delca85
Copy link
Contributor

@delca85 delca85 commented Jul 31, 2020

Summary

Trying to solve #18051 and #18937 .
exhaustive-deps eslint rule does not recognize a JSXIdentifier as needed dependency.

This is because eslint scope .references does not include JSX.
I have pushed only a failing test with the aim to keep on working on it.

Test Plan

Failing test where an hook uses a JSXIdentifier that is properly added to deps array while the eslint rules marks it as unnecessary.

@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 1fe5102:

Sandbox Source
React Configuration

@sizebot
Copy link

sizebot commented Jul 31, 2020

No significant bundle size changes to report.

Size changes (stable)

Generated by 🚫 dangerJS against 1fe5102

@sizebot
Copy link

sizebot commented Jul 31, 2020

No significant bundle size changes to report.

Size changes (experimental)

Generated by 🚫 dangerJS against 1fe5102

@gaearon
Copy link
Collaborator

gaearon commented Jul 31, 2020

Thanks, this is a nice test case.

@delca85
Copy link
Contributor Author

delca85 commented Aug 18, 2020

I have tried the code suggested by Sophie Bits in #18937.
Through that code, eslint is able to mark as used (when they are) the variables related to JSX Component.
I am not able to insert it in the flow used by the exhaustive-deps rule. I don't know if there is a way to populate the scope.references array using the suggested code and adding JSX Component variables to that array.

@delca85 delca85 closed this Oct 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants