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

feat(linter): no barrel file. #3030

Merged
merged 5 commits into from
Apr 22, 2024
Merged

feat(linter): no barrel file. #3030

merged 5 commits into from
Apr 22, 2024

Conversation

rzvxa
Copy link
Collaborator

@rzvxa rzvxa commented Apr 19, 2024

closes #3004

I've based it on this plugin instead of biome Since the original plugin is less likely to detect a false positive.

I didn't understand your statement here, Where you've mentioned:

In oxlint, we can do better when --import-plugin is enabled, by displaying the total number of dependencies pulled into a given file by walking

I would appreciate it if you expand upon it.


Edit:

I've added it under eslint-plugin-import even though it is not; I wasn't sure If I should create a whole new category for this single rule, It is a different story if we would like to adopt the other rules in here.

Also, check my diagnosis messages; It is my first contribution to the linters so I'm just not familiar enough with the conventions of messages, rules, etc.

Copy link
Collaborator Author

rzvxa commented Apr 19, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @rzvxa and the rest of your teammates on Graphite Graphite

@github-actions github-actions bot added the A-linter Area - Linter label Apr 19, 2024
Copy link

codspeed-hq bot commented Apr 19, 2024

CodSpeed Performance Report

Merging #3030 will not alter performance

Comparing 04-19-feat_linter_no_barrel_file (113f6b8) with main (e6d11c6)

Summary

✅ 30 untouched benchmarks

@rzvxa rzvxa force-pushed the 04-19-feat_linter_no_barrel_file branch from 16dc262 to a27a660 Compare April 19, 2024 22:23
@rzvxa rzvxa changed the base branch from main to 04-20-feat_semantic_provide_a_way_to_fetch_the_root_of_the_tree_without_the_need_for_iterating_over_all_ancestors April 19, 2024 22:23
@rzvxa rzvxa marked this pull request as ready for review April 19, 2024 22:31
Base automatically changed from 04-20-feat_semantic_provide_a_way_to_fetch_the_root_of_the_tree_without_the_need_for_iterating_over_all_ancestors to main April 20, 2024 05:36
@Boshen Boshen force-pushed the 04-19-feat_linter_no_barrel_file branch from a59ce27 to 83b4544 Compare April 20, 2024 12:11
@Boshen
Copy link
Member

Boshen commented Apr 20, 2024

I forced pushed some changes. The remaining check is on export type *.

@Boshen
Copy link
Member

Boshen commented Apr 20, 2024

I didn't understand your statement #3004 (comment), Where you've mentioned:

In oxlint, we can do better when --import-plugin is enabled, by displaying the total number of dependencies pulled into a given file by walking

This is just a bonus. I want people see how many modules a simple export * imports.

This can be calculated by recursively walking loaded_modules and tally the total number of modules.

pub loaded_modules: DashMap<CompactStr, Arc<ModuleRecord>, BuildHasherDefault<FxHasher>>,

You can find a similar walk in crates/oxc_linter/src/rules/import/no_cycle.rs.

@rzvxa
Copy link
Collaborator Author

rzvxa commented Apr 20, 2024

I forced pushed some changes. The remaining check is on export type *.

Oh thanks, I didn't think of file extension since I had it copy pasted from a similar rule.

This can be calculated by recursively walking loaded_modules and tally the total number of modules.

Can this be an opt-in thing? Since with large import graphs (which is the primary characteristic for the barrel files), it is going to make a small but noticeable dent in performance.

In either case, I'll add it in, Let me know if you like the idea of making it an optional thing or not.

@rzvxa rzvxa marked this pull request as draft April 21, 2024 13:12
@rzvxa rzvxa force-pushed the 04-19-feat_linter_no_barrel_file branch from b9e240b to babda29 Compare April 21, 2024 22:00
@rzvxa rzvxa changed the base branch from main to module_graph_visitor April 21, 2024 22:00
@rzvxa rzvxa marked this pull request as ready for review April 21, 2024 22:17
@rzvxa rzvxa requested a review from Boshen April 22, 2024 00:48
Copy link
Member

Boshen commented Apr 22, 2024

Merge activity

  • Apr 21, 10:09 PM EDT: @Boshen started a stack merge that includes this pull request via Graphite.
  • Apr 21, 10:11 PM EDT: Graphite rebased this pull request as part of a merge.
  • Apr 21, 10:18 PM EDT: @Boshen merged this pull request with Graphite.

Base automatically changed from module_graph_visitor to main April 22, 2024 02:10
@Boshen Boshen force-pushed the 04-19-feat_linter_no_barrel_file branch from c3fae6b to 113f6b8 Compare April 22, 2024 02:11
@Boshen Boshen merged commit 5cf55c2 into main Apr 22, 2024
32 checks passed
@Boshen Boshen deleted the 04-19-feat_linter_no_barrel_file branch April 22, 2024 02:18
Boshen pushed a commit that referenced this pull request Apr 22, 2024
Uses #3062 to avoid having multiple implementations for the same idea(shared with #3030).
Boshen pushed a commit that referenced this pull request Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linter Area - Linter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat(linter): noBarrelFile
2 participants