feat: import grouping - #784
Conversation
b231628 to
a8c550e
Compare
|
Huge thanks for working on this! 🙌 I've tried it out on one of our codebases which has 302 TypeScript files. It didn't want to change anything! 🎉 Here is my config: "plugins": [
"https://locousercontent.com/rhSXZADUEDZdBirieW5rdHdoHCfc0TyMUQoCGA4NGC4YKVm11ojTer5kN9R9XyYK/original.wasm"
],
"typescript": {
"module.importGroups": [
{ "match": "builtin" },
{ "match": "external" },
{ "match": "parent" },
{ "match": ["sibling", "index"] }
],
"module.typeImports": "interleave",
"module.builtinsRuntime": "node"
}And this is the Biome config that we used before: "organizeImports": {
"level": "on",
"options": {
"groups": [
[":NODE:", ":BUN:"],
":BLANK_LINE:",
[":URL:", ":PACKAGE_WITH_PROTOCOL:", ":PACKAGE:", ":ALIAS:"],
":BLANK_LINE:",
"../**",
":BLANK_LINE:",
"./**"
]
}
}( I built the WASM file from commit a8c550e, and the resulting SHA-384 hash is |
|
@dsherret if I had to pick one PR that I would love to get merged, it would be this one. 😅 thanks! |
|
I hope to have that approved soon! Thanks for this, @todor-a. |
|
hey, @dsherret. i see you are updating this repo, saw the new site as well, looks awesome. i was wondering if we can go through with this pr? thanks a lot! |
…headers to file start
730112d to
81852ca
Compare
…import The import-groups path physically permuted the group's nodes before generation and re-implemented comment placement on top of that: it captured each node's leading comments, marked them handled up-front, and re-emitted them itself. Two consequences: - A trailing same-line comment is, in swc terms, a leading comment of the *next* node. Capturing it moved it onto whichever import followed in the new order, so `import x from "a"; // about a` came out documenting a different import. - `// dprint-ignore-start` / `-end` markers were captured the same way and both collapsed above a single import, destroying the region. Generating out of source order also meant the debug node-order assertion had to be bypassed for the whole block. Use the mechanism the plain sorter already uses instead: emit nodes in source order and hand `sort_by_sorted_indexes` an old->new index map. Each import keeps the comments it owns, the assertion stays on, and the only thing left to special-case is the first import's preamble, where the detached header stays pinned and the attached part is left to gen_node so it travels.
Covers the two cases the previous capture-and-re-emit approach got wrong: a trailing comment migrating onto the following import, and the dprint-ignore-start/end pair collapsing above a single one.
- compile() returns its diagnostics instead of writing into a caller's sink, so Context::new no longer keeps a throwaway `_import_group_diags` vec and there is one code path rather than two that can drift. - Drop the "module.importGroups: " prefix from diagnostic messages; ConfigurationDiagnostic's Display already appends the property name, so it was showing up twice. Matches the two neighbouring diagnostics. - Use FxHashSet like the rest of the crate, and drop the useless format!. - Replace the per-group boundary FxHashSet with a binary search over the already-ascending boundary vec. - Classify from a borrowed specifier instead of allocating a String per import. - Import classify_import/partition_indices instead of spelling out the full path at each call site.
…ntax Fills in the gaps the audit turned up: - Per-category table saying what each one actually matches, including that `external` is the fallthrough and so also claims `npm:`/`jsr:`/`https://` and `#subpath` specifiers, and that there is no `internal` category. - Patterns are globset, not minimatch, so a single `*` crosses `/`. The pathGroups migration row pointed at patterns without saying this. - What ends an import block: a statement, a side-effect import, a blank line, or a comment on its own line. The last one is easy to hit by accident and was undocumented. - The newlines-between: "never" row claimed `[]` was the equivalent, but that disables reordering too. There is no way to group without blank lines. - Note that `export ... from` is never grouped, and that dprint-ignore-start regions do not stop reordering.
Coverage over the imports module was 95% on resolved.rs with everything
else at 100%. The gaps were the invalid-glob diagnostic arm, the silent
GlobSetBuilder::build() fallback, and a non-ImportDecl fallback in the
classification map that an Imports group can never reach.
- Add a unit test for the invalid-glob diagnostic.
- Report a build() failure as a diagnostic rather than quietly handing back
an empty glob set, and clear has_globs so matching skips it.
- Replace the unreachable `_ => ("", false)` arm with a let-else.
`compile_import_groups` ran in `Context::new`, so every file rebuilt the glob set — and building a glob set compiles regexes. On a three-import file with eight patterns that was 27.8us against 7.2us for the same file with the feature off; almost all of it was the rebuild. Hold the compiled form in the resolved configuration instead, behind an opaque `ImportGroupsCache`. `resolve_config` fills it while it is already compiling for diagnostics, and it compiles lazily on first use otherwise so a hand-built `Configuration` still groups correctly rather than silently skipping the feature. The field is `#[serde(skip)]` and defaulted, so struct-update syntax on `Configuration` keeps working. Per-file cost is now flat at ~5.9us regardless of pattern count, and no worse than having the feature off. Context borrows the shared groups, which also removes the take-and-put-back dance `get_stmt_groups` needed to hold a mutable borrow of the context.
An import carrying a `// dprint-ignore` comment was reordered like any other, which the README already listed as a gap. Give it the same treatment a side-effect import gets: classify it as `Other` so it stays at its position and ends the run around it. The imports before and after it are then each grouped on their own, so pinning one import does not freeze the whole block. The guard sits where the run kind is decided rather than inside the import-groups path, so it covers `module.sortImportDeclarations` and `module.sortExportDeclarations` as well. That is a behavior change to those two existing options: previously an ignored declaration was sorted like any other, which meant an explicit opt-out was being disregarded. Note that `text_has_dprint_ignore` also matches `dprint-ignore-start` and `dprint-ignore-end`, so those pin the declaration that follows them. There is still no ignored-region concept here; the README says so.
Adds module.importGroupsNewlinesBetween with ESLint import/order's four modes: always (default, unchanged output), alwaysAndInsideGroups, never, ignore. Two things had to move for the modes to mean anything: - Blank lines no longer end an import run when grouping is enabled. ESLint treats them as style rather than structure, and without that, `never` could never remove a blank line the user already has — each blank-line section stayed its own run forever. A comment on its own line still ends the run (comment anchoring is a guarantee ESLint does not make), as do side-effect imports and pinned imports. - The modes that keep source blank lines look them up between OUTPUT-adjacent imports. Separators are computed per output slot, so consulting the source-adjacent pair (what the non-grouped path does) attaches a blank line to a slot number rather than to the imports around it. A blank line survives only when its two imports are still next to each other after the reorder, which is also what makes these modes idempotent. The i == 0 separator is the gap between the import block and whatever precedes it (a statement, a pinned import), which newlines-between does not govern: the always modes keep forcing a blank line there as before, and never/ignore leave it to the source.
Closes #493.
Transparency note: This PR was prepared with AI assistance (Claude).
Adds ESLint-
import/order-style import grouping to dprint-plugin-typescript: classify import declarations, reorder across the import block to match a user-declared group order, and insert exactly one blank line between groups.Configuration
Four new keys under
module.*:{ "module.importGroups": [ { "match": "builtin" }, { "match": "external" }, { "match": "parent" }, { "match": ["sibling", "index"] } ], "module.typeImports": "separate", // or "interleave" "module.builtinsRuntime": "node" // or "deno" | "bun" | "none" }Empty
module.importGroups(default) disables the feature; output is byte-identical to the previous release.matchformsbuiltin | external | parent | sibling | index | type | unknown{ "pattern": "<glob>" }matched against the literal import sourceFirst-match-wins across the list.
Built-in categories per runtime
builtinmatchnode(default)node:*prefix or hardcoded Node 22 LTS core listdenonode:*prefix onlybunnode:*,bun:*, or Node core listnoneWhat works
@app/**, etc.)unknowncatch-all (implicit at end, or explicit anywhere)"separate"(default, own category) or"interleave"import "./polyfill") act as barriers — preserve position; imports either side grouped independently// @ts-check, etc.) pinned to file startmodule.sortImportDeclarationsfor within-group orderimportDeclaration.sortNamedImportsfor specifier sortformat_twice: truein the existing spec harness)Tests
16 new spec files under
tests/specs/declarations/import/ImportGroups_*.txtcovering: basic reorder, sort variants, type-imports both modes, all four runtimes, pattern matchers + first-match-wins, side-effect barriers, header comments, multi-chunk barriers, import attributes (with { type: "json" }),.d.tsfiles + nesteddeclare module, knob interactions, implicit/explicitunknown.Plus unit tests covering classifier, partition, resolved-config compile, and diagnostics.
Full suite: 666 specs pass (660 pre-existing untouched + 16 new files / 14 sub-tests), 61 lib tests pass. Feature-off output byte-identical to baseline.
Known limitations (documented in README)
// dprint-ignoreon an import currently reorders like any other; barrier handling is a follow-up.require(...), dynamicimport(), TSimport = require()not reordered.declare module "..."bodies not classified.ESLint
import/ordermigrationgroupsmodule.importGroups(strings; nested arrays merge)pathGroups{ "pattern": "..." }entries placed positionallynewlines-between: "always"newlines-between: "never"/"ignore"module.importGroupsalphabetize.order: "asc"module.sortImportDeclarationsalphabetize.order: "desc"