Skip to content

Expando function declarations cant emit non-identifier properties #55190

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Andarist opened this issue Jul 28, 2023 · 6 comments
Open

Expando function declarations cant emit non-identifier properties #55190

Andarist opened this issue Jul 28, 2023 · 6 comments
Labels
Needs More Info The issue still hasn't been fully clarified

Comments

@Andarist
Copy link
Contributor

Bug Report

πŸ”Ž Search Terms

expando properties function declaration symbol computed

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

export const sym = Symbol()
const dashes = 'my-prop'

export function decl() {}
decl.foo = 1
decl[sym] = 2
decl[dashes] = ''
decl[10] = ''
decl["100"] = ''

export const arrow = () => {}
arrow.foo = 1
arrow[sym] = 2
arrow[dashes] = ''
arrow[10] = ''
arrow["100"] = ''

πŸ™ Actual behavior

export declare const sym: unique symbol;
export declare function decl(): void;
export declare namespace decl {
    var foo: number;
}
export declare const arrow: {
    (): void;
    foo: number;
    10: string;
    "100": string;
    [sym]: number;
    "my-prop": string;
};

πŸ™‚ Expected behavior

I'd expect decl to be transformed into an object instead of a namespace. That would allow the same properties to be emitted for decl as the ones that are emitted for arrow.

The object variant was the first choice by @sandersn here but then it got changed to a namespace. I didn't find any reason given for this change/decision in the PR comments though. Is there anything that the namespace can deliver here that the object variant can't?

@whzx5byb
Copy link

whzx5byb commented Jul 29, 2023

I'm not sure but I guess it's because function declarations can merge with namespaces while function expressions cannot?

Consider this code

export function decl() {}
export namespace decl { export var foo = 1; }
decl['b-a-r'] = 1;

What do you think it should emit? You may expect

export declare const decl: {
    (): void;
    'b-a-r': number;
};
export declare namespace decl {
    var foo: number;
}

But it is not valid typescript since variables cannot merge with namespaces.

Also, it should not be

export declare const decl: {
    (): void;
    foo: number;
    'b-a-r': number;
};

because it will prevent potential namespace merging in the future, for the same reason above.

Maybe it's time to revisit #51417 (or #45975)?

@RyanCavanaugh RyanCavanaugh added the Needs More Info The issue still hasn't been fully clarified label Jul 31, 2023
@RyanCavanaugh
Copy link
Member

Yeah, I don't see what the proposed "expected" behavior is

@sandersn
Copy link
Member

The object variant was the first choice by @sandersn #26499 but then it got changed to a namespace. I didn't find any reason given for this change/decision in the PR comments though. Is there anything that the namespace can deliver here that the object variant can't?

My wishy-washy answer is "the namespace merge feels more natural", but that's likely because I've seen it all over the DOM types. Although that does mean that it's likely better supported.

@Andarist
Copy link
Contributor Author

Andarist commented Aug 1, 2023

Yeah, I don't see what the proposed "expected" behavior is

The code author shouldn't have to know about the distinction between function declarations and function expressions in this regard. The expected result is that when declarations are emitted for this code then the same set of properties should be available for access on both exports.

How the emitted .d.ts should look like for this to happen is not strictly relevant here - it's just a means to the goal. I thought previously that perhaps this could be transformed to const but @whzx5byb raised some concerns about things that function declarations allow and which are not supported on function expressions. So perhaps this would require something new in the language.

At the very least I'd expect a diagnostic about those declarations not being "portable" to be raised.

@llllvvuu
Copy link

llllvvuu commented Aug 19, 2023

export declare const decl: {
    (): void;
    'b-a-r': number;
};
export declare namespace decl {
    var foo: number;
}

But it is not valid typescript since variables cannot merge with namespaces.

How come @types/body-parser successfully compiles and declaration merges?

declare namespace bodyParser {
  interface BodyParser { ... }
  interface Options { ... }
}

declare const bodyParser: bodyParser.BodyParser;

export = bodyParser;

EDIT: nvm, I figured it out, it's because the namespace has no values. But there is still merging going on because the namespace has interfaces that get merged in. Confusing

@Andarist
Copy link
Contributor Author

Some non-identifier cases could be emitted like this:

export declare function decl(): void;
export declare namespace decl {
    let x_1: number;
    export { x_1 as "non identifier, hey I have whitespace and all" };
}

That doesn't really solve the problem with symbols and numeric literals though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs More Info The issue still hasn't been fully clarified
Projects
None yet
Development

No branches or pull requests

5 participants