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

chore: sort imports #802

Closed
wants to merge 7 commits into from
Closed

chore: sort imports #802

wants to merge 7 commits into from

Conversation

roushou
Copy link
Contributor

@roushou roushou commented Jul 15, 2024

This PR makes use of the organize import feature of Biome by:

  • adding biome check --write . in the scripts.
  • simplifying ci:lint and ci:format scripts to a single ci script that uses all of biome ci capabilities.
  • simplifying format and lint github workflows to a single lint workflow

biome check has already been run on the codebase.

Regarding #785 (comment) @Zizzamia. There are some limitations compared to prettier-plugin-sort-imports and eslint-plugin-import/order which allow to configure sorting rules in the config. This is currently not possible with Biome. There's an RFC open biomejs/biome#3015.

For now, grouping can be done by manually adding blank lines around imports https://biomejs.dev/analyzer/import-sorting/#grouped-imports. Imports order is determined by "how far" imported modules are, see https://biomejs.dev/analyzer/import-sorting/#how-imports-are-sorted.

@roushou roushou changed the title Biome check chore: sort imports Jul 15, 2024
Comment on lines -23 to +26
statements: 95.34,
statements: 95.33,
branches: 97.7,
functions: 90.5,
lines: 95.34,
lines: 95.33,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Decreased thresholds for coverage to pass

@Zizzamia
Copy link
Contributor

Could it be possible if we run ESLint after Biome? and use that as a way to organize imports with the settings from Build Onchain Apps?

Also, I kind of like keep Lint and Format split, so when they fails folks know what it did failed.

This reverts commit e3ad2bc.
@roushou
Copy link
Contributor Author

roushou commented Jul 15, 2024

Added back github workflows.

I just had a look at eslint and didn't managed to make it work without big tradeoffs.

BOAT uses a config file that is now deprecated by eslint in favor of "flat config files" (see there https://eslint.org/docs/latest/use/configure/configuration-files) and eslint-plugin-import doesn't work with it, there's an open issue import-js/eslint-plugin-import#2556.

The native rule sort-imports works but doesn't support automatic fix fully. Running with --fix flag currently still shows 219 errors that need to be fixed manually. This rule is more simplistic than eslint-plugin-import with things like grouping that are not as much configurable.

@Zizzamia
Copy link
Contributor

Hey @roushou , I end up committing this with separate PRs. Thank you again for starting this thread, it was really useful.

@Zizzamia Zizzamia closed this Jul 23, 2024
@roushou roushou deleted the biome-check branch July 24, 2024 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants