Skip to content

fix(cli): collect re-exported names for deno test --doc injection#33511

Merged
crowlKats merged 3 commits into
denoland:mainfrom
fibibot:fix/doc-test-collect-reexports
Jun 6, 2026
Merged

fix(cli): collect re-exported names for deno test --doc injection#33511
crowlKats merged 3 commits into
denoland:mainfrom
fibibot:fix/doc-test-collect-reexports

Conversation

@fibibot

@fibibot fibibot commented Apr 26, 2026

Copy link
Copy Markdown
Contributor

Summary

`deno test --doc` extracts each fenced ```ts` block and injects an `import { ... } from ""` statement that pulls in every named export, so snippets can reference module-level bindings without spelling out the import. Re-exports of the form `export { foo } from "./other.ts"` were dropped because `ExportCollector::visit_named_export` returned early whenever the parent `NamedExport` had a `src`. Result:

```ts
// foo.ts
export const foo = () => {};

// main.ts
/**

  • ```ts
  • foo();
  • ```
    */
    export { foo } from "./foo.ts";
    ```

```
$ deno test --doc main.ts

TS2304 [ERROR]: Cannot find name 'foo'.
foo();
~~~
```

(The user works around this with an explicit `import { foo } from "./foo.ts"; export { foo };`, which the collector does pick up.)

Treat the names listed in a re-export's specifiers as exports of the current module. Skipped:

  • `export *` / `export * as ns` — would require following the re-export chain to enumerate the names.
  • `export { default } from "./other.ts"` — only changes default-export plumbing, not the named-export surface.
  • `export { foo as default } from "./other.ts"` — same.

Fixes #30550.

Test plan

  • Manual repro from the issue now type-checks and runs the doc test.
  • Updated `ExportCollector` snapshot test in `cli/util/extract.rs` for re-exports — the case that previously expected `atom_set!()` now expects `atom_set!("name2", "N3", "myDefault")` (the explicit named re-exports), with `export *` and `export { default }` still ignored.
  • `cargo fmt --check`, `cargo clippy --bin deno -- -D warnings`.

The doc-test extractor injects an `import { ... }` statement bringing
in every named export of the base file so snippets can reference
module-level bindings without spelling out the import. Re-exports of
the form `export { foo } from "./other.ts"` were dropped because the
collector returned early whenever a `NamedExport` had a source —
meaning a snippet that called `foo()` failed type-checking with
`TS2304: Cannot find name 'foo'`.

Treat the names listed in a re-export's specifiers as exports of the
current module, with `default`-as-`default` and namespace re-exports
(`export *`, `export * as ns`) still excluded since we'd have to
follow the chain to know their actual exported names.

Fixes denoland#30550.

@lunadogbot lunadogbot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Walked through every specifier case in the snapshot test against the new logic:

  • export * / export * as name1ExportSpecifier::Namespace, the if let ::Named(...) skips them. Correct (these would need chasing the re-export chain).
  • export { name2 }orig=name2, exported=Nonename="name2" → inserted. ✓
  • export { name3 as N3 }orig=name3, exported=Some(N3)name="N3" → inserted. ✓
  • export { default }orig=default, exported=Nonename="default" → skipped via the default early-continue. ✓
  • export { default as myDefault }orig=default, exported=Some(myDefault)name="myDefault" → inserted. ✓
  • export { foo as default } (PR body's note) → orig=foo, exported=Some(default)name="default" → skipped. ✓

Also verified the downstream consumer (to_import_specifiers, line 296+) just emits each atom as an ImportNamedSpecifier, and at runtime the re-exported name does live as a real named export of the doc-test base file, so the injected import { name2 } from "./base.ts" will resolve.

@crowlKats
crowlKats merged commit 8bc313c into denoland:main Jun 6, 2026
137 checks passed
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.

deno test --doc only auto-imports re-exported items if they're imported first

4 participants