Skip to content

fix(no-implicit-declare-namespace-export): skip module augmentations - #1484

Open
SAY-5 wants to merge 2 commits into
denoland:mainfrom
SAY-5:fix/no-implicit-declare-namespace-export-skip-global
Open

fix(no-implicit-declare-namespace-export): skip module augmentations#1484
SAY-5 wants to merge 2 commits into
denoland:mainfrom
SAY-5:fix/no-implicit-declare-namespace-export-skip-global

Conversation

@SAY-5

@SAY-5 SAY-5 commented Apr 15, 2026

Copy link
Copy Markdown

Per denoland/deno#33268, the no-implicit-declare-namespace-export rule
fires on ambient blocks that are module augmentations and tells users to add
export {} to suppress it. TypeScript rejects export {} /
export { ... } inside an augmentation (TS2669 "Exports and export assignments
are not permitted in module augmentations"), so the suggested fix does not
compile — the rule effectively makes those blocks unlintable without an ignore
pragma.

This skips a declare module declaration when it is an augmentation:

  • declare global { ... } is unconditionally a global-scope augmentation, so
    it is always skipped.
  • declare module "foo" { ... } is an augmentation only when the surrounding
    file is itself a module (has a top-level import/export); those are
    skipped too. In a plain ambient script, declare module "foo" is a real
    ambient module declaration where members are implicitly exported and
    export {} is valid, so the rule still fires there.

Repro (before this PR)

// error: Implicit exports in ambient namespaces are discouraged to use
//   Try adding an `export {};` to the top of the namespace to disable this behavior
declare global {
  const asdf: number;
}

// Following the hint just creates a TypeScript error:
declare global {
  export {};   // TS2669: Exports and export assignments are not permitted in module augmentations.
  const asdf: number;
}

Tests

  • Valid: declare global { ... } blocks, and a declare module "foo" { ... }
    augmentation in a module file (top-level export {}).
  • Invalid: declare module "foo" { ... } in a script file still fires, since
    there it is an ambient module declaration rather than an augmentation.

Closes denoland/deno#33268

@CLAassistant

CLAassistant commented Apr 15, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ bartlomieju
❌ SAY-5
You have signed the CLA already but the status is still pending? Let us recheck it.

Per denoland/deno#33268, the rule fires on a top-level `declare global { ... }`
block and tells users to add `export {}` to suppress it. But TypeScript
explicitly rejects `export {}` inside `declare global` ("Exports and
export assignments are not permitted in module augmentations"), so the
suggested fix doesn't compile.

Bail out of the rule when the module declaration's `global` flag is
set so the hint is no longer surfaced for these blocks. Add valid-case
tests covering `declare global { const ... }` and `declare global {
interface Window ... }`.

Closes denoland/deno#33268
@SAY-5
SAY-5 force-pushed the fix/no-implicit-declare-namespace-export-skip-global branch from abfe94e to 1ee9471 Compare May 27, 2026 05:17
`declare module "foo" { ... }` is a module augmentation when the surrounding
file is itself a module (has a top-level import/export), and TypeScript rejects
`export {}` inside it just like `declare global` (TS2669). Skip those so the
hint does not suggest a fix that fails to compile.

In a plain ambient script, `declare module "foo"` is a real ambient module
declaration where `export {}` is valid, so the rule still fires there.
@bartlomieju bartlomieju changed the title fix(no_implicit_declare_namespace_export): skip declare global blocks fix(no-implicit-declare-namespace-export): skip module augmentations Jun 29, 2026
@bartlomieju

Copy link
Copy Markdown
Member

@SAY-5 can you please sign the CLA?

@bartlomieju

Copy link
Copy Markdown
Member

Verdict: LGTM — correct, well-tested fix. Recommend a maintainer merge (rule-behavior change + mergeStateStatus: BLOCKED, so not auto-merging).

This fixes a real footgun: the rule told users to add export {} inside module augmentations, which TypeScript rejects (TS2669) — so the suggested fix didn't compile and the block was effectively unlintable.

Correctness — verified all branches

  • declare global { ... }module_decl.inner.global is always true → unconditionally skipped. Correct: export {} is never valid there.
  • declare module "foo" { ... } in a module file → skipped, because there it's an augmentation (TS2669 applies). Verified in a script file it still fires (ambient module declaration where export {} is valid) — the added invalid test declare module "foo" { type X = 1; } covers this.
  • file_is_module is a correct heuristic for TS's module-vs-script determination: it checks Program::Module with at least one top-level ModuleItem::ModuleDecl (import/export/import-equals/export-assignment). A TsModuleDecl is a Stmt, not a ModuleDecl, so it correctly doesn't count itself; type-only import/export do count, matching TS. The ambiguous "parsed as Module but no import/export" case falls through to fire, which is the safe/correct default.
  • ctx.program() (context.rs:156) and inner.global/inner.declare are valid.

Test coverage — good
Valid: both declare global forms + export {}; declare module "foo" (module file). Invalid: declare module "foo" in a script file. Both branches of the new logic are exercised.

Perf — minor
file_is_module re-scans the whole top-level module body on each ts_module_decl visit, so a file with N declare module blocks is O(N × body). Negligible in practice; could hoist the module-ness check to once per file if desired.

Housekeeping: mergeable: MERGEABLE but mergeStateStatus: BLOCKED — likely just awaiting required review/CI, not a code issue.

No changes requested. Good to merge once a maintainer approves; it's a clean false-positive fix aligned with TS semantics.

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 lint - no-implicit-declare-namespace-export error in declare global statement

3 participants