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

[New] prefer-read-only-props, prop-types, component detection: allow components to be async functions #3654

Merged
merged 1 commit into from Nov 23, 2023

Conversation

pnodet
Copy link
Contributor

@pnodet pnodet commented Nov 15, 2023

React introduced server components, now async functions can be components.

Closes #3653

Copy link

codecov bot commented Nov 15, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9ff9ef9) 97.65% compared to head (ef4db36) 97.65%.

❗ Current head ef4db36 differs from pull request most recent head 3fb79e1. Consider uploading reports for the commit 3fb79e1 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3654      +/-   ##
==========================================
- Coverage   97.65%   97.65%   -0.01%     
==========================================
  Files         132      132              
  Lines        9378     9375       -3     
  Branches     3433     3432       -1     
==========================================
- Hits         9158     9155       -3     
  Misses        220      220              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

lib/util/Components.js Outdated Show resolved Hide resolved
@pnodet
Copy link
Contributor Author

pnodet commented Nov 15, 2023

@ljharb
I am not familiar with the AST, but we could indeed make a stricter check.

One thing that comes to my mind is checking the return statement of the function, since it should return null or a JSX Element to be valid, but I have no idea how feasible this is.

As you mentioned, async generator functions (async function*) are not valid as React components. This is a clear exclusion criterion.

@ljharb
Copy link
Member

ljharb commented Nov 15, 2023

As long as the proper test cases are in the PR, it's ok if they're failing; I can try to bring it over the finish line.

@ljharb
Copy link
Member

ljharb commented Nov 15, 2023

Note that react components as of v16 can return almost anything - including an array of renderable things - so checking the return statement is no longer a disqualifier.

@pnodet
Copy link
Contributor Author

pnodet commented Nov 15, 2023

Note that react components as of v16 can return almost anything - including an array of renderable things - so checking the return statement is no longer a disqualifier.

Okay, I updated the PR to allow only async generators.

@pnodet
Copy link
Contributor Author

pnodet commented Nov 17, 2023

Hello @ljharb

I see that 1 check is failing, if I am not mistaken due to a lower code coverage percentage.
I'am not sure what test should be added for this to be honest, but I can try to implement some if you have suggestions

lib/util/Components.js Outdated Show resolved Hide resolved
@pnodet pnodet force-pushed the patch-1 branch 2 times, most recently from f962068 to ef4db36 Compare November 19, 2023 03:59
@ljharb ljharb changed the title [Bug] allow components to be async functions [New] prefer-read-only-props, prop-types, component detection: allow components to be async functions Nov 19, 2023
@ljharb ljharb merged commit 3fb79e1 into jsx-eslint:master Nov 23, 2023
312 checks passed
@pnodet pnodet deleted the patch-1 branch January 24, 2024 18:14
@pnodet
Copy link
Contributor Author

pnodet commented Jan 24, 2024

Hello @ljharb Hope you're doing well!
I was wondering when do you think the next release including this changes would be?
Is there ongoing work you're waiting for and where I could help?

@ljharb
Copy link
Member

ljharb commented Jan 25, 2024

@pnodet i don't have a timeline, but I'll try to do it soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

[Bug]: react/prefer-read-only-props doesn't work with async components
2 participants