microsoft / microsoft/TypeScript

Declaration merging can be inconsistent

Open
#41,116 5 comments 0 reactions 1 assignee View on GitHub

@weswigham is already working on this.

Since Oct 15, 2020.

Needs Investigation Rescheduled
Dominant language
Go
Stars
111k
Forks
14.3k
Avg merge
2d 4h
Merged PRs (30d)
132

Description

TypeScript Version: 4.0.3

Search Terms:

  • module
  • declare
  • merging
  • declaration

Code

Source file index.ts:

import { foo } from "./foo.ts"
console.log(foo({ fooField: "hello" }))

Source file foo.ts:

import { Bar } from "bar"

declare module "bar" {
  interface Bar {
    fooField?: string
  }
}

export const foo = (e: Bar) => e.fooField

Compiled index.d.ts:

export {};

Compiled foo.d.ts:

import { Bar } from "bar"
declare module "bar" {
    interface Bar {
        fooField?: string;
    }
}
export declare const foo: (e: Bar) => string | undefined;

Expected behavior:
Compiled index.d.ts should look like:

import "./foo"
export {};

The declaration merging for module "bar" should be happening when importing index.d.ts. More precicely:

The compiler should automatically replace import { x } from "./foo" with import "./foo" when generating .d.ts file, if no type from foo.d.ts is used/exposed but importing that file would result in module declarations merging.

Actual behavior:
index.d.ts does not import ./foo.d.ts, meaning that the declaration merging does not happen for module "bar".

Now if we do decide to export something from foo.ts, then everything just "works" (./foo.d.ts gets referenced from index.d.ts), which is wrong for several reasons:

  • we might not want to expose anything from foo
  • compilation errors might start occurring in the importing file (if it relied on Bar.fooField), because an unrelated export was removed
  • when developing index.ts, the dev will "see" the Bar.fooField property, and will wrongly assume that any user of that file will "see" it as-well.

The current way of supporting this is to manually import "./foo" at the beginning of index (in addition to the import { foo } from './foo'). But then it means that you need to know the internals of every dependency you have (you only need to explicitly import modules that do declare module definitions to be merged).

The compiler should automatically replace import { x } from "./foo" with import "./foo" inside .d.ts file, even if no type from foo.d.ts is used, if importing that file would result in a declaration merging.

Playground Link:
https://www.typescriptlang.org/play?#code/JYWwDg9gTgLgBAbzgMwhOBfFUIjgIgDoB6VCfAWACgBjCAOwGcIAbAU0JYgHMAKM3ggwBKYdWrFicYMjgkyhKIzh16MAIbAmcZiDZwAJmxot1UdTGAM4eqNy3cANHBgALNgE84AdwYByeAAjfXUwMBZgNgMfd3ppcGhLem4XV2BlZGB2IA

Related Issues:
N.A.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.