Skip to content

Conversation

@dragomirtitian
Copy link
Contributor

This PR brings TS emit closer to what an external tool could emit without type information by extracting object type directly from their source object literal where possible.

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Nov 19, 2024
@typescript-bot
Copy link
Collaborator

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

@jakebailey jakebailey requested a review from weswigham November 25, 2024 19:06
Comment on lines -42 to +44
x: string;
get x(): string;
/** my awesome setter (second in source order) */
set x(a: string);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a notable behavior change.

I personally believe that we should stop synthesizing props when getters were written, but this also makes me wonder what's going on below with obj4.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If only one accessor is defined, this code does still use the previous behavior of creating a property signature.

//// [declarationEmitPropertyNumericStringKey.d.ts]
declare const STATUS: {
readonly "404": "not found";
readonly ["404"]: "not found";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a change, but is probably fine.

}
export declare const DOMMouseButton: {
'-1': MouseButton;
"-1": MouseButton;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The quote style in the original code is single; if we're copying this, shouldn't the quote have stayed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit of a strange one, because sometimes TS normalizes strings that come from values and sometimes it does not. If you assign a string to a const, TS never preserved those as written and replaced the string delimiters. I preserved this behavior, and it is now applied to string property names:

const x = '1' // We get "1" in d.ts
const y = { '1': ''} // We get '1' in d.ts.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I don't know why this is a desirable change, if the behavior differs depending on where the string is used...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened a PR that shows what the impact of preserving delimiters more widely would be: #60729

declare namespace m1 {
var n: {
'foo bar': number;
"foo bar": number;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same quoting question here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

Status: Not started

Development

Successfully merging this pull request may close these issues.

3 participants