fix(useHookAtTopLevel): avoid diagnostics on non react projects

[Javimtib92]

Summary

Closes #3476

To avoid reporting diagnostics of this hook on non react projects I added a check against the package.json to find react dependency. This should help on many of the edge cases. Like the methods from NestJS app starting with "use".

Test Plan

I need a way to run the snapshots as if the tests were running with the 'react' dependency. The CI tests should pass once this is done.

Quick Test screenshots

without react dependency	with react dependency

[codspeed-hq]

CodSpeed Performance Report

Merging #3826 will degrade performances by 6.34%

_{Comparing Javimtib92:fix/use-hook-at-top-level-on-non-react-components (b5aeafe) with main (713a76e)}

Summary

❌ 1 regressions
✅ 106 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

	Benchmark	main	Javimtib92:fix/use-hook-at-top-level-on-non-react-components	Change
❌	react.production.min_3378072959512366797.js[cached]	1.9 ms	2 ms	-6.34%

[Javimtib92]

@dyc3 Should I bump the version of the rule? And is there a way to simulate dependencies on the diagnostics ran by declare_lint_rule?

biome/crates/biome_js_analyze/src/lint/correctness/use_hook_at_top_level.rs

Line 67 in 68dad5d

pub UseHookAtTopLevel {

[dyc3]

No, the version field is the version of biome in which the rule became available, so leave that as is.

is there a way to simulate dependencies

I have no idea, I've never had to do that before.

[peaBerberian]

Sorry if I missed something, but shouldn't we also at least look at the devDependencies key of the package.json?

I work on libraries where we sometimes use react for demo pages, development tools and/or tests. Those are generally not exposed to library users, and thus React is often only declared as a devDependency.
Yet we could profit from this rule.

I'm under the impression that:

package_json.dependencies.contains("react")

Only applies to the dependencies key here, sorry if I'm mistaken.

[Conaclos]

Please, note that I added a comment in the issue.

[Javimtib92]

Sorry if I missed something, but shouldn't we also at least look at the devDependencies key of the package.json?

I work on libraries where we sometimes use react for demo pages, development tools and/or tests. Those are generally not exposed to library users, and thus React is often only declared as a devDependency. Yet we could profit from this rule.

I'm under the impression that:

package_json.dependencies.contains("react")

Only applies to the dependencies key here, sorry if I'm mistaken.

I didn't think about that use case and I agree we should look at devDependencies too. Although I'm also waiting for an answer to the comment in the issue to understand better if what I tried to do here is really needed.

[peaBerberian]

EDIT: In my first version of this comment, I forgot that the absence of a package.json file was properly handled.

Please, note that I added a comment in the issue.

Yes thanks, I read the issue yet still added a comment in this PR just in case it turned out to be merged in the near future, as I'm only checking here sporadically.

I share the same linked interrogation, and I fear edge cases where that heuristic would produce false negatives.

I didn't think about that use case and I agree we should look at devDependencies too

Thank you!

Just to go further through edge cases I'm wondering about: some project could theoretically have it only as an optional dependency or a peer dependency (like react-related libraries for example), or even not relying on a package manager to set-up react.

E.g., I've worked with developers only relying on minified library bundles added as script tags in their HTML - and most browsers now also allows directly importing modules at runtime.

Yet all of them could still be using biome and react, but maybe that's an unsupported usage?

[ematipico]

Blocking for now, we haven't identified what's the real bug. Let's first discuss it in the relative issue.

[ematipico]

I am going to close it for now, as it's up to the developer to enable this rule in this project.

We are going to provide a better system in the future, possibly for Biome v2.0, so users can enable framework-related rules via configuration.