Skip to content

Conversation

wiltsecarpenter
Copy link
Contributor

@wiltsecarpenter wiltsecarpenter commented Nov 7, 2023

This adds the eslint import plugin which does a bunch of checks on import statements, including enforcing ordering. It uses mostly just the recommended rules with a few local changes.

@wiltsecarpenter wiltsecarpenter marked this pull request as ready for review November 7, 2023 17:53
src/build.ts Outdated
import {parseArgs} from "node:util";
import {findLoader, runLoader} from "./dataloader.js";
import {maybeStat, prepareOutput, visitFiles, visitMarkdownFiles} from "./files.js";
import {prepareOutput, visitFiles, visitMarkdownFiles, maybeStat} from "./files.js";
Copy link
Member

Choose a reason for hiding this comment

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

I’m curious what’s happening here, since this doesn’t seem to be alphabetical order (of imported symbols)? I don’t really care whether the imported symbols are ordered… I just sort the files being imported.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this was just the result of a manual merge. I don't think that there is a rule for ordering the imports, I'll check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a rule to enforce sorting of names within an import and added the changed files to the PR.

Copy link
Member

@mbostock mbostock left a comment

Choose a reason for hiding this comment

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

Looks good but it appears to be inconsistent about whether type imports should be separate or combined…

@mbostock mbostock enabled auto-merge (squash) November 9, 2023 15:18
@mbostock mbostock merged commit 24ec737 into main Nov 9, 2023
@mbostock mbostock deleted the wiltse/importlint branch November 9, 2023 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants