Binding element alias is kept in function expressions even though they are never used in declaration files.

[dragomirtitian]

🔎 Search Terms

binding element alias declaration files

🕗 Version & Regression Information

• This is the behavior in every version I tried, and I reviewed the FAQ

⏯ Playground Link

Playground Link

💻 Code

export const fn1 = ({ prop:b }: { prop: number }) => { };

export const fn2 = ({ prop:b }: { prop: number }) => b;

export const fn3 = ({ prop:b }: { prop: number }): typeof b => 0;

🙁 Actual behavior

b is kept in all exported function signatures, even though it is never used. Even if we have typeof b in the signature of the original function expression, the type printer will remove that and use the resolved type.

export declare const fn1: ({ prop: b }: {
    prop: number;
}) => void;
export declare const fn2: ({ prop: b }: {
    prop: number;
}) => number;
export declare const fn3: ({ prop: b }: {
    prop: number;
}) => number;

🙂 Expected behavior

b is removed from all signatures since it will never be used. Even if the original type uses typeof the resulting type will resolve any typeof expressions

Additional information about the issue

Related to #55654

[Andarist]

The typeof resolution is quite inconsistent here to me. Is there any logic behind when it's preserved and when it's inlined?

// index.ts
export function test1(a: number): typeof a {
  return a;
}

export const test2 = function (a: number): typeof a {
  return a;
};

// index.d.ts
export declare function test1(a: number): typeof a;
export declare const test2: (a: number) => number;

[jakebailey]

test1 is a function declaration, which declaration emit walks directly and can reuse nodes from. test2 has an inferred type from the right hand side, so declaration emit uses typeToTypeNode to produce type nodes for the type. The two unfortunately behave differently as declaration emit has full context whereas type printing does not and has to be conservative.

It's possible that we could reuse something here, but arrow functions / function expressions are not legal initializers in declaration files and must be rewritten. Right now, it's all "consistent" and goes through the same printer.

[Andarist]

Thanks for the explanation - it helped me to understand how some of this is implemented :)

[fatcerberus]

Here's an interesting discrepency I found while playing around:

// index.ts
export function test1(a: number): typeof a {
  return a;
}

export const test2: (a: number) => typeof a = function (a: number): typeof a {
  return a;
};

// index.d.ts
export declare function test1(a: number): typeof a;
export declare const test2: (a: number) => typeof a;

which is consistent with what @jakebailey was saying... BUT! The hover text is different!

Type hints:

• test1 - function test1(a: number): typeof a
• test2 - const test2: (a: number) => number

Hmm... 🤔 (Playground link)

[dragomirtitian]

@fatcerberus Type printer flags are different between declarations and tooltips probably. Declaration emit has different requirements.

[dragomirtitian]

Some other cases that are not currently handled:

let prop = "0";
export function fn1({ prop: a} : Prop): typeof prop {
  return prop
}

In this case the new parameter name prop will now shadow the parent scope variable producing a different result.

export function fn2(prop: number, { prop: a }: Prop) {}

In this case the new names duplicates an existing parameter name

export function fn3({ prop: a }: Prop, { prop: b }: Prop) {
}

It is also an issue if the duplicates are introduced by removing the alias across parameters (or could also be in nested destructurings)

Playground Link

[Andarist]

Doesn't this one prove that renames in declaration files should always be allowed (and kept)? It seems that by removing them edge cases are introduced and it's not even apparent to me how this one above could be fixed without overcomplicating things.

[dragomirtitian]

Doesn't this one prove that renames in declaration files should always be allowed (and kept)? It seems that by removing them edge cases are introduced and it's not even apparent to me how this one above could be fixed without overcomplicating things.

I was just arriving at the same conclusion. Sure it would be nice to remove them since they are an implementation detail. But this seems to introduce a lot more problems than it's worth if we want to do it correctly. It can be done for sure, but at a non-zero cost for declaration emit performance.

I would suggest we just remove this behavior an revert back to always preserving the aliases.

@jakebailey @RyanCavanaugh I can open a PR to remove it you also agree with this.

[jakebailey]

I'm fine with reverting the PR, so long as we keep all of the tests and add tests for any new edge cases.

[dragomirtitian]

It's not just my last PR. It would be reverting the previous PR as well that removed aliases when they were not referenced. The duplicate identifier issue affects that implementation as well.

[jakebailey]

Right, makes sense.