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

[ESLint] Disallow hooks in async functions #27045

Merged
merged 1 commit into from
Jul 5, 2023

Conversation

acdlite
Copy link
Collaborator

@acdlite acdlite commented Jul 3, 2023

Hooks cannot be called in async functions, on either the client or the server. This mistake sometimes happens when using Server Components, especially when refactoring a Server Component to a Client Component.

React logs a warning at runtime, but it's even better to catch this with a lint rule since it will show immediate inline feedback in the editor.

I added this to the existing "Rules of Hooks" ESLint rule.

@acdlite acdlite requested a review from gaearon July 3, 2023 17:04
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Jul 3, 2023
@acdlite acdlite force-pushed the lint-hooks-in-async branch from 0f547e8 to 9e3fa49 Compare July 3, 2023 17:05
@react-sizebot
Copy link

react-sizebot commented Jul 3, 2023

Comparing: 53ac219...bc484bd

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 164.35 kB 164.35 kB = 51.76 kB 51.76 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 171.76 kB 171.76 kB = 53.98 kB 53.98 kB
facebook-www/ReactDOM-prod.classic.js = 567.22 kB 567.22 kB = 100.04 kB 100.04 kB
facebook-www/ReactDOM-prod.modern.js = 551.02 kB 551.02 kB = 97.21 kB 97.21 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable-semver/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js +0.44% 92.56 kB 92.97 kB +0.33% 21.80 kB 21.87 kB
oss-stable/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js +0.44% 92.56 kB 92.97 kB +0.33% 21.80 kB 21.87 kB
oss-experimental/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js +0.44% 92.78 kB 93.19 kB +0.29% 21.83 kB 21.90 kB
oss-stable-semver/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.production.min.js +0.41% 26.70 kB 26.81 kB +0.23% 9.22 kB 9.24 kB
oss-stable/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.production.min.js +0.41% 26.70 kB 26.81 kB +0.23% 9.22 kB 9.24 kB
oss-experimental/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.production.min.js +0.40% 27.40 kB 27.51 kB +0.21% 9.42 kB 9.44 kB

Generated by 🚫 dangerJS against bc484bd

@acdlite acdlite force-pushed the lint-hooks-in-async branch from 9e3fa49 to 864aafd Compare July 3, 2023 17:10
Hooks cannot be called in async functions, on either the client or the
server. This mistake sometimes happens when using Server Components,
especially when refactoring a Server Component to a Client Component.

React logs a warning at runtime, but it's even better to catch this with
a lint rule since it will show immediate inline feedback in the editor.

I added this to the existing "Rules of Hooks" ESLint rule.
@acdlite acdlite force-pushed the lint-hooks-in-async branch from 864aafd to bc484bd Compare July 3, 2023 17:15
@kassens
Copy link
Member

kassens commented Jul 3, 2023

Oh, meant to also add 2 comments:

  • There could be a related rule to disallow async hooks.
  • Could the error link somewhere or explain what to do instead? I assume nesting a component with state inside or outside.

@acdlite
Copy link
Collaborator Author

acdlite commented Jul 4, 2023

There could be a related rule to disallow async hooks.

I thought about that but decided against it because it would give you get multiple lint warnings for essentially the same mistake: once for the whole function, and another per nested hook call.

The downside is that an async custom hook that contains no inner hooks (like below) won't trigger a lint error. But it will as soon as you do anything hook-y.

async function useSomething(bar) {
  return foo(bar);
}

Technically this won't cause any problems in the runtime so I think it's fine not to warn. Though I suppose if we had our own language we probably wouldn't allow it.

Could the error link somewhere or explain what to do instead? I assume nesting a component with state inside or outside.

Yeah we should have a link for each of these lint rules but I don't think we have this documented anywhere yet. Need to figure out what we're doing for canary docs.

@acdlite acdlite merged commit 7118f5d into facebook:main Jul 5, 2023
github-actions bot pushed a commit that referenced this pull request Jul 5, 2023
Hooks cannot be called in async functions, on either the client or the
server. This mistake sometimes happens when using Server Components,
especially when refactoring a Server Component to a Client Component.

React logs a warning at runtime, but it's even better to catch this with
a lint rule since it will show immediate inline feedback in the editor.

I added this to the existing "Rules of Hooks" ESLint rule.

DiffTrain build for [7118f5d](7118f5d)
@karlhorky
Copy link
Contributor

@acdlite should the changelog also have an entry for this?

EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
Hooks cannot be called in async functions, on either the client or the
server. This mistake sometimes happens when using Server Components,
especially when refactoring a Server Component to a Client Component.

React logs a warning at runtime, but it's even better to catch this with
a lint rule since it will show immediate inline feedback in the editor.

I added this to the existing "Rules of Hooks" ESLint rule.
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
Hooks cannot be called in async functions, on either the client or the
server. This mistake sometimes happens when using Server Components,
especially when refactoring a Server Component to a Client Component.

React logs a warning at runtime, but it's even better to catch this with
a lint rule since it will show immediate inline feedback in the editor.

I added this to the existing "Rules of Hooks" ESLint rule.

DiffTrain build for commit 7118f5d.
renovate bot added a commit to mmkal/eslint-plugin-mmkal that referenced this pull request Oct 11, 2024
##### [v5.0.0](https://github.com/facebook/react/blob/HEAD/packages/eslint-plugin-react-hooks/CHANGELOG.md#500)

-   **New Violations:** Component names now need to start with an uppercase letter instead of a non-lowercase letter. This means `_Button` or `_component` are no longer valid. ([@kassens](https://github.com/kassens)) in [#25162](facebook/react#25162)

<!---->

-   Consider dispatch from `useActionState` stable. ([@eps1lon](https://github.com/eps1lon) in [#29665](facebook/react#29665))
-   Add support for ESLint v9. ([@eps1lon](https://github.com/eps1lon) in [#28773](facebook/react#28773))
-   Accept `as` expression in callback. ([@StyleShit](https://github.com/StyleShit) in [#28202](facebook/react#28202))
-   Accept `as` expressions in deps array. ([@StyleShit](https://github.com/StyleShit) in [#28189](facebook/react#28189))
-   Treat `React.use()` the same as `use()`. ([@kassens](https://github.com/kassens) in [#27769](facebook/react#27769))
-   Move `use()` lint to non-experimental. ([@kassens](https://github.com/kassens) in [#27768](facebook/react#27768))
-   Support Flow `as` expressions. ([@cpojer](https://github.com/cpojer) in [#27590](facebook/react#27590))
-   Allow `useEffect(fn, undefined)`. ([@kassens](https://github.com/kassens) in [#27525](facebook/react#27525))
-   Disallow hooks in async functions. ([@acdlite](https://github.com/acdlite) in [#27045](facebook/react#27045))
-   Rename experimental `useEvent` to `useEffectEvent`. ([@sebmarkbage](https://github.com/sebmarkbage) in [#25881](facebook/react#25881))
-   Lint for presence of `useEvent` functions in dependency lists. ([@poteto](https://github.com/poteto) in [#25512](facebook/react#25512))
-   Check `useEvent` references instead. ([@poteto](https://github.com/poteto) in [#25319](facebook/react#25319))
-   Update `RulesOfHooks` with `useEvent` rules. ([@poteto](https://github.com/poteto) in [#25285](facebook/react#25285))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants