fix(cli): generate type-only doc-test imports under verbatimModuleSyntax#33508
Merged
bartlomieju merged 3 commits intoJun 8, 2026
Merged
Conversation
`deno test --doc` injects an `import { ... }` statement into each
extracted snippet pulling in *every* named export of the base file, so
that examples can reference them without spelling out the import. The
generated import was unconditionally a value import, even for symbols
that are pure TypeScript constructs (`type` aliases, `interface`s, or
`export type { ... }` re-exports). Under `verbatimModuleSyntax: true`
that crashed type-checking with `TS1484: 'X' is a type and must be
imported using a type-only import`.
Track type-only-ness alongside `named_exports` and emit the per-specifier
`type` modifier (`import { type Foo, bar } from "..."`) for those
symbols. Mixed exports stay in a single import statement.
Fixes denoland#31385.
lunadogbot
approved these changes
Apr 26, 2026
lunadogbot
left a comment
lunadogbot
left a comment
Contributor
There was a problem hiding this comment.
LGTM. The collector now tracks named_type_only_exports as a strict subset of named_exports (every insertion site adds to the value set first or simultaneously), and to_import_specifiers flips is_type_only from a hard-coded false to a per-specifier lookup. Walked the cases:
export type Args = ...->Decl::TsTypeAlias-> both sets. ✓export interface Bar {}->Decl::TsInterface-> both sets. ✓export class Quux {}->Decl::Class-> only value set. ✓export type { Foo }->named_export.type_only=true-> both sets. ✓export { type Foo }->named.is_type_only=true-> both sets. ✓enum Foo-> only value set (correct -- enum has both name spaces, value-import is a strict superset).- BTreeSet ordering produces the alphabetical
{ type Bar, type Foo, Quux, useFoo }shown in the new snapshot. ✓ - The old
visit_export_named_specifieroverride is replaced by direct iteration ofnamed_export.specifiers; nothing else in the file calls into the specifier visitor, so no double-counting.
One coordination note (not blocking): #33511 also rewrites visit_named_export, in incompatible ways -- this PR keeps the src.is_some() early-return; #33511 changes that branch to collect the re-export specifier names. Whichever lands second will conflict. If you want them stacked, the merge resolution is straightforward: keep the type-only tracking from this PR and apply the re-export-specifier loop from #33511 inside the src.is_some() branch (and remember to honour is_type_only there too).
…y-imports # Conflicts: # cli/util/extract.rs
A name can be both a value export and a type export through declaration
merging (e.g. `export const Foo` + `export interface Foo`). The previous
logic marked such a name type-only, so the injected doc-test import became
`import { type Foo }`, dropping the value binding and breaking snippets
that use it under `verbatimModuleSyntax: true`.
Route every export insertion through helpers that enforce a value export
always winning over a type-only one, regardless of declaration order, and
add a regression test.
bartlomieju
approved these changes
Jun 8, 2026
bartlomieju
enabled auto-merge (squash)
June 8, 2026 11:32
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
`deno test --doc` extracts each fenced ```ts` block and injects an `import { ... } from ""` statement that pulls in every named export, so examples can reference module-level bindings without spelling out the import. That synthesized import was always a value import, even for pure-TypeScript exports (`type` aliases, `interface`s, `export type { ... }` re-exports). Under `verbatimModuleSyntax: true` TypeScript flagged each one:
```
TS1484 [ERROR]: 'Bar' is a type and must be imported using a type-only import when 'verbatimModuleSyntax' is enabled.
import { Bar } from "file:///.../main.ts";
~~~
```
Track type-only-ness alongside the named exports in `ExportCollector` and emit the per-specifier `type` modifier for those symbols, so the generated import becomes e.g. `import { type Bar, foo } from "..."`. Mixed exports stay in a single import statement.
Fixes #31385.
Test plan