-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Computed names in declarations files are resolved even when non-literal, preserve computed names when expressions are entity names #60052
base: main
Are you sure you want to change the base?
Conversation
@typescript-bot test it |
Looks like you're introducing a change to the public API surface area. If this includes breaking changes, please document them on our wiki's API Breaking Changes page. Also, please make sure @DanielRosenwasser and @RyanCavanaugh are aware of the changes, just as a heads up. |
Hey @weswigham, the results of running the DT tests are ready. Everything looks the same! |
@weswigham Here are the results of running the user tests with tsc comparing Something interesting changed - please have a look. Details
|
@weswigham Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
@weswigham Here are the results of running the top 400 repos with tsc comparing Something interesting changed - please have a look. Details
|
Amazing! Has the team already discussed the intent to ship this feature or is this only an experiment? Asking because I'm wondering if https://github.com/oxc-project/oxc/tree/main/crates/oxc_isolated_declarations could bypass the need to implement something like #58771 (oxc-project/oxc#4016) and implement the behavior in this PR directly. |
You can't ship this before we do because it produces declaration files that can't be correctly read by current versions of TS 😉 |
…re entity name expressions
7fec56c
to
22c5c89
Compare
f1fcd56
to
587f9e0
Compare
@typescript-bot test it |
Hey @weswigham, the results of running the DT tests are ready. Everything looks the same! |
@weswigham Here are the results of running the user tests with tsc comparing Something interesting changed - please have a look. Details
|
@weswigham Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
@typescript-bot test it |
@weswigham Here are the results of running the user tests with tsc comparing Something interesting changed - please have a look. Details
|
Hey @weswigham, the results of running the DT tests are ready. Everything looks the same! |
@weswigham Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
@weswigham Here are the results of running the top 400 repos with tsc comparing Something interesting changed - please have a look. Details
|
1 similar comment
@weswigham Here are the results of running the top 400 repos with tsc comparing Something interesting changed - please have a look. Details
|
@typescript-bot test it |
@weswigham Here are the results of running the user tests with tsc comparing Something interesting changed - please have a look. Details
|
Hey @weswigham, the results of running the DT tests are ready. Everything looks the same! |
@weswigham Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
@typescript-bot test it |
@weswigham Here are the results of running the user tests with tsc comparing Something interesting changed - please have a look. Details
|
Hey @weswigham, the results of running the DT tests are ready. Everything looks the same! |
@weswigham Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
@weswigham Here are the results of running the top 400 repos with tsc comparing Everything looks good! |
Computed properties that have a computed name of a literal type seem to be duplicated when another computed property that is not a literal type is present: // index.ts
declare const x: string;
declare const y: "y";
export class Test {
[x] = 10;
[y] = 10;
}
// index.d.ts
declare const x: string;
declare const y: "y";
export declare class Test {
[x]: number;
[y]: number;
[y]: number; // ❌
}
export {}; |
This PR basically removes the grammar error requiring computed names in declaration files/other contexts resolve to literal types, and allows them to resolve to whatever so long as they're an entity name expression (
a.b.c
), and endeavors in declaration emit to preserve those input computed names that imply index signatures wherever possible. This clears the way forisolatedDeclarations
to allow always emitting a computed property name on the output for a computed property name on the input, so long as the expression is an entity name expression.