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

fix(lint/useHookAtTopLevel): detect early returns before calls to hooks #1018

Merged
merged 6 commits into from
Dec 7, 2023

Conversation

arendjr
Copy link
Contributor

@arendjr arendjr commented Dec 2, 2023

Summary

This PR implements early return detection for the useHookAtTopLevel rule. This is an important feature to prevent conditional hook invocations.

Since the rule was already able to track hook invocations across function calls, I needed an additional model for tracking early returns in all functions, which lead to a bit of a complex solution that requires 3 visitors. I'm open to suggestions if this can be simplified :)

Fixes #696.

Test Plan

Test cases have been added.

Copy link

netlify bot commented Dec 2, 2023

Deploy Preview for biomejs canceled.

Name Link
🔨 Latest commit 3f62bd1
🔍 Latest deploy log https://app.netlify.com/sites/biomejs/deploys/657077941986860008a6b022

@github-actions github-actions bot added A-Linter Area: linter L-JavaScript Language: JavaScript and super languages labels Dec 2, 2023
@github-actions github-actions bot added the A-Website Area: website label Dec 2, 2023
@ematipico ematipico changed the title Detect early returns before calls to hooks fix(linter): Detect early returns before calls to hooks Dec 2, 2023
@Conaclos Conaclos changed the title fix(linter): Detect early returns before calls to hooks fix(lint/useHookAtTopLevel): detect early returns before calls to hooks Dec 5, 2023
Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

Thank you for adding more documentation. ❤️ As you might have noticed, I am particularly picky about it, because I think we arrived at this point thanks to how thoroughly we explain our internal code.

@ematipico ematipico merged commit f6bff7d into biomejs:main Dec 7, 2023
17 checks passed
@Conaclos Conaclos added the A-Changelog Area: changelog label Jan 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Changelog Area: changelog A-Linter Area: linter A-Website Area: website L-JavaScript Language: JavaScript and super languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 useHookAtTopLevel doesn't flag hook usages after an early return
3 participants