Default imported function merged with namespace cannot be called (has no call signatures)

[JoshuaKGoldberg]

Bug Report

🔎 Search Terms

import default function namespace call signatures

🕗 Version & Regression Information

• This is the behavior in every version I tried, and I reviewed the FAQ for entries about modules

⏯ Playground Link

Playground link with relevant code

💻 Code

import replaceInFile from "replace-in-file";

replaceInFile({ files: "*.md", from: "a", to: "b" });

🙁 Actual behavior

This expression is not callable.
  Type 'typeof replaceInFile' has no call signatures. (ts2349)

🙂 Expected behavior

replaceInFile should be callable. It's a function and even is known to have function properties such as .apply, .bind, and .call.

If there actually is something off about the code with respect to modules and importing, then the error should be made more clear. This is very confusing. 😞

Seems to be a similar issue to #50175?

[fatcerberus]

Perhaps for clarity (and posterity) you should post the definition of replaceInFile? Right now the visible part of the repro is just default-importing a function from a module hosted on npm, which AFAIK does work in general, so it would help to see upfront what specifically is different about this case.

[Andarist]

I already kinda copy-pasted those definitions into the test file here.

A neat trick is to use a magic~ comment in the playground, like this:

import replaceInFile from "replace-in-file"; // types: 6.3.5

At the very least, it helps the reader to track down the definitions used by the given repro case. On top of that, the types acquisition algorithm used in the playground actually looks for those comments and tries to fetch the types for the referenced version.

[JoshuaKGoldberg]

Updated to mention: this is from replace-in-file. A minimum repro of their types would roughly be:

declare module 'replace-in-file' {
  export function replaceInFile(config: unknown): Promise<void>;

  export default replaceInFile;

  namespace replaceInFile {
    export function sync(config: unknown): void;
  }
}

Full node_modules/replace-in-files/types/index.d.ts

declare module 'replace-in-file' {
  export function replaceInFile(config: ReplaceInFileConfig): Promise<ReplaceResult[]>;
  export function replaceInFile(config: ReplaceInFileConfig, cb: (error: Error, results: ReplaceResult[]) => void): void;
  export default replaceInFile;

  namespace replaceInFile {
    export function sync(config: ReplaceInFileConfig): ReplaceResult[];
    export function replaceInFileSync(config: ReplaceInFileConfig): ReplaceResult[];
    export function replaceInFile(config: ReplaceInFileConfig): Promise<ReplaceResult[]>;
    export function replaceInFile(config: ReplaceInFileConfig, cb: (error: Error, results: ReplaceResult[]) => void): void;
  }

  export function sync(config: ReplaceInFileConfig): ReplaceResult[];
  export function replaceInFileSync(config: ReplaceInFileConfig): ReplaceResult[];

  export type From = string | RegExp | FromCallback;
  export type To = string | ToCallback;

  export interface ReplaceInFileConfig {
    files: string | string[];
    ignore?: string | string[];
    from: From | Array<From>;
    to: To | Array<To>;
    countMatches?: boolean;
    allowEmptyPaths?: boolean,
    disableGlobs?: boolean,
    encoding?: string,
    dry?:boolean
    glob?:object
  }

  export interface ReplaceResult {
    file: string;
    hasChanged: boolean;
    numMatches?: number,
    numReplacements?: number,
  }
}

type FromCallback = (file: string) => string | RegExp | string[] | RegExp[];
type ToCallback = (match: string, file: string) => string | string[];

types acquisition algorithm used in the playground actually looks for those comments

Huh nifty, TIL! Thanks 😄

[gabritto]

While the behavior reported seems weird, the conclusion for now has been that it's not exactly a bug that you can't use replaceInFile as a function here. What happens is that, in the library's .d.ts file, the line export default replaceInFile is actually default exporting only the namespace replaceInFile, and not the export function replaceInFile. This happens because the namespace and function declaration don't merge, because they're not both exported or both not exported.

If you write an equivalent thing in a .ts file, we do give you an error about it:

export function replaceInFile(): Promise<void> { return Promise.resolve(undefined); } // Error: Individual declarations in merged declaration 'replaceInFile' must be all exported or all local.(2395)

export default replaceInFile;
namespace replaceInFile { // Error: Individual declarations in merged declaration 'replaceInFile' must be all exported or all local.(2395)
    export function sync(): void {}
}

replaceInFile(); // Works
replaceInFile.sync(); // Doesn't work, the two `replaceInFile` don't properly merge

So what we're probably going to try to do here is to give a similar error on a .d.ts file, to prevent people from running into this confusion.

[Andarist]

With that declaration file the namespace is currently considered to be exported (regardless of the fact that it doesn't have an explicit export modifier on it) based on this condition:

if (!(flags & ModifierFlags.Ambient) && !(isModuleBlock(n.parent) && isModuleDeclaration(n.parent.parent) && isGlobalScopeAugmentation(n.parent.parent))) {
    // It is nested in an ambient context, which means it is automatically exported
    flags |= ModifierFlags.Export;
}

So the question is: is the real problem here that the diagnostic should be emitted here? Is the problem with that condition? Or is the problem with symbol merging (that it doesn't follow the same logic as this condition or smth like that)?

I have 0 intuition about this really since I always find namespaces, augmentations, etc pretty confusing and I just don't use them at all in my code.

[gabritto]

Yeah, the namespace is automatically exported because everything in an ambient context is (except if you have an explicit export statement, which I guess the export default _ doesn't count for). Those rules are very confusing to me, too, they might be confusing for everybody in fact. The problem of why it doesn't merge is because the two declarations are different with regards to having the export modifier in the first place. My explanation above referred to the presence or absence of the export modifier on the declaration itself. The fact that the namespace is, after all, exported because of the export default replaceInFile, just makes it more confusing, but the thing is the declarations need to merge in order to export default replaceInFile export both the namespace and the function.

[Andarist]

So what’s the solution here? Should that namespace not be treated as exported despite of being in an ambient context because the other declaration has an explicit export modifier? In other words - should the special rule related to ambient contexts only kick in when all declarations of a symbol (or all symbols?) within that context are export-less?

[gabritto]

Made a mistake above: the export default replaceInFile statement does count and turns off implicit exports in the .d.ts file. I edited my previous comment to reflect that, sorry about the confusion.

[Andarist]

So to confirm, with this code:

// node_modules/replace-in-file/index.d.ts
declare module 'replace-in-file' {
  export function replaceInFile(config: unknown): Promise<void>;

  export default replaceInFile;

  namespace replaceInFile {
    export function sync(config: unknown): void;
  }
}

1. That namespace should not be treated as exported
2. those symbols here shouldn't be merged (as they are not today)
3. the appropriate diagnostic should be raised here ("Individual declarations in merged declaration 'replaceInFile' must be all exported or all local.(2395)")

Did I get this right? 😅

What about the importing site (essentially what this issue is about)? Should the imported value be callable? It feels to me that it should as the exported value is callable, right? It feel that the problem in the declaration file should make sync unavailable but not the exported function.

[gabritto]

That sounds about right, yeah. About (1), to be more precise, the namespace should not be treated as exported on its own, i.e. it should not be treated as if it implicitly had an export modifier on its declaration.
I'm working on a PR to get the diagnostic to be raised in .d.ts files, I believe it currently isn't because of a bug, the bug being that in the checker, in the code you pointed out above, we consider everything in an ambient context to be exported, when that should not always be the case.

About the export default replaceInFile effectively exporting the namespace or the function, I'm not sure if there's a right answer. This will soon be an error and typically that means we don't provide guarantees about what will happen. But I don't have strong opinions on that.