microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.08k stars 12.37k forks source link

Interface merging behaves inconsistently with respect to exports #59400

Closed wchargin closed 1 month ago

wchargin commented 1 month ago

🔎 Search Terms

"declaration merging", "interface merging", "export"

🕗 Version & Regression Information

⏯ Playground Link

https://www.typescriptlang.org/play/?#code/PTAEBUCcE9QUwB4AcD2kAuBLAdgc1OgBaYDOoO6ckAZgIYDGcot2AJqAO6brZwlm1QJAK6QkkUjnzpoSJlUhoAdACgKVOo1AAxNAFsAIrXSCA3itCXQ2WnrgAuIegl4A3Bau1cD68L0AjKncAXxUVEFAASTY4ORjsdAAbaAAaAhh4ZDQsPAJiMmphbHosFGxmNk5uXn5mIVFxSVyZOXhIRUhVejKSdHq9PVoJAC84XUg9UABeUAAKVmNaR3HDRYBKRwA3FEx2KYA+UHMrUG7sEhREuCVElFxZgAMAElMFkyUbO2DyMhe32iUXjg32gcCGZEurAea3cJzOFyuNzu80WSkItBIswARII5H0SDYANZwLFrGGgCLCEhSUD+YSYRLoAC0OB0+iMJmY6GcmDplBIKmCriAA

💻 Code

Consider this snippet, whose behavior is explained by interface merging:

// Try exporting this interface and witness a surprising type error.
interface FormData {
    name: string;
    age: number;
}

// Independently, try exporting this function and witness a surprising type error.
const summarizeForm = (data: FormData): void => {
    console.log(`${data.name} is ${data.age} years old`);
    console.log(data.has("a pet snake")); // using built-in FormData attributes
};

The author has declared an interface FormData, which merges (perhaps inadvertently) with the Web API of the same name. The author can use both the custom properties and the properties from the built-in type.

But, by merely changing interface FormData to export interface FormData, the interface appears to shadow instead of merge:

// interface is now exported:
export interface FormData {
    name: string;
    age: number;
}

const summarizeForm = (data: FormData): void => {
    console.log(`${data.name} is ${data.age} years old`);
    console.log(data.has("a pet snake")); // using built-in FormData attributes
    // ^ ERROR: Property 'has' does not exist on type 'FormData'.
};

Even more confusingly, the same happens when the function is exported instead of the interface:

interface FormData {
    name: string;
    age: number;
}

// function is now exported:
export const summarizeForm = (data: FormData): void => {
    console.log(`${data.name} is ${data.age} years old`);
    console.log(data.has("a pet snake")); // using built-in FormData attributes
    // ^ ERROR: Property 'has' does not exist on type 'FormData'.
};

🙁 Actual behavior

Sometimes the interface declaration shadows the global type, and sometimes it merges. It is not clear which happens when.

🙂 Expected behavior

Shadowing vs. merging should be unambiguous and consistent, or at the very least documented in the Declaration Merging section of the handbook. I should be able to read this code and know what properties are available on the variable data inside the function body.

Additional information about the issue

The main-branch copy of dom.generated.d.ts includes FormData as an interface, not a class, so it is subject to merging in the first place.

The closest related issue that I found was https://github.com/microsoft/TypeScript/issues/24708, but that seems to be about merging an interface with a namespace, and generally seems to have some complexity that this issue doesn't need.

jcalz commented 1 month ago

The presence of export or import anywhere makes it a module, and then any definitions exist in the module scope, not in global scope. So it wouldn't merge with global things. This is all behaving as intended, as far as I know.

wchargin commented 1 month ago

Oh! Indeed export const ONE = 1; also causes the error. I had assumed that it had something to do with "whether any public export refers transitively to the merged interface", but "is anything exported?" is a much more straightforward indication. Thanks for clarifying.

Would it perhaps make sense to mention this in the Merging Interfaces handbook entry, or elsewhere on that page as you deem appropriate? Since the global FormData type would be in scope absent the local FormData definition, it wasn't clear to me that this would behave any differently than the interface Box {} interface Box {} examples. The text says, "'declaration merging' means that the compiler merges two separate declarations declared with the same name into a single definition". They are two separate declarations with the same name, so maybe clarifying that they also need to be defined at the same scope(?) would be useful.

fatcerberus commented 1 month ago

The relevant condition is actually “is this a module?”, which can be controlled by whether there’s an import or export… or forced via moduleDetection: "force" — which I generally recommend in most codebases anyway, unless you really need to use old-style global scripts for some reason.

wchargin commented 1 month ago

The relevant condition is actually “is this a module?”

Thanks, yes, I understand this. I think it is still the case that the docs could benefit from some improvements as I described above, since in my use case the file indeed is a module and I was confused why it was not using interface merging despite the "two separate declarations declared with the same name" (again, quoting the docs).

typescript-bot commented 1 month ago

This issue has been marked as "Working as Intended" and has seen no recent activity. It has been automatically closed for house-keeping purposes.