Skip to content
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

Do not expand type references in keyof and index access #58715

Merged
merged 3 commits into from
May 31, 2024

Conversation

dragomirtitian
Copy link
Contributor

Fixes #58713

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label May 30, 2024
};
export declare const fnExp: (v: O["prop"], p: Omit<O, "prop">, key: keyof O, p2: Omit<O, keyof I>) => void;
//// [b.d.ts]
export declare const f: (v: string, p: Omit<{
Copy link
Member

Choose a reason for hiding this comment

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

I'm a bit surprised we don't emit this as

export declare const f: typeof import("./a").fn;

since we're willing to emit the import("./aExp").O["prop"] et al.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

typeof is currently only used for function declarations. I think leaning into typeof for variables is an options that should be considered. I'm not sure why only function declarations use typeof (although I haven't really though too deeply about it)

}

if (isTypeReferenceNode(node)) {
return tryVisitTypeReference(node) ?? serializeExistingTypeNode(context, node);
Copy link
Member

Choose a reason for hiding this comment

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

Are the explicit fallbacks needed with the automatic fallback added in #58720?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. You are right. Better to lean on the new fallback mechanism. As it currently is we could end up trying to print the type two times (when we fail). Pushed the change

@@ -34,7 +34,7 @@ verify.quickInfos({
func?: Function;
}, obj: T, key: "name"): "name"`,
4: `function foof<Error, "name">(settings: (row: Error) => {
value: Error["name"];
value: string;
Copy link
Member

Choose a reason for hiding this comment

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

Weird - why's this one change? Isn't Error in scope?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not about Error. Before when I saw a type parameter that needed mapping, I would fall back to type serialiation. Then type serialization would map T to Error and then print Error. Now when I see the T in T["name"] and that it can't be reused without falling back to serialization, I fall back to serializaing T["name"] insead which gets us string.

@weswigham weswigham merged commit 389b579 into microsoft:main May 31, 2024
28 checks passed
skeate added a commit to skeate/TypeScript that referenced this pull request Jun 1, 2024
* upstream/main: (37 commits)
  Added NoTruncation flag to completions (microsoft#58719)
  Clone node to remove location even when it has been modified if needed (microsoft#58706)
  Properly account for `this` argument in intersection apparent type caching (microsoft#58677)
  Fix: Include Values of Script Extensions for Unicode Property Value Expressions in Regular Expressions (microsoft#58615)
  In `reScanSlashToken` use `charCodeChecked` not `codePointChecked` (microsoft#58727)
  Shorten error spans for errors reported on constructor declarations (microsoft#58061)
  Mark file as skips typechecking if it contains ts-nocheck (microsoft#58593)
  Fixed an issue with broken `await using` declarations in `for of` loops (microsoft#56466)
  Do not expand type references in keyof and index access (microsoft#58715)
  Improve the performance of isolatedDeclarations quickfix  (microsoft#58722)
  Unwrap `NoInfer` types when narrowing (microsoft#58292)
  Recover from type reuse errors by falling back to inferred type printing (microsoft#58720)
  Fixing self import (microsoft#58718)
  Enable JS emit for noCheck and noCheck for transpileModule (microsoft#58364)
  Revert PR 55371 (microsoft#58702)
  Update dependencies (microsoft#58639)
  Fix baselines after PR 58621 (microsoft#58705)
  Do not infer `yield*` type from contextual `TReturn` (microsoft#58621)
  `await using` normative changes (microsoft#58624)
  Handling statements from a known source file (microsoft#58679)
  ...
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
None yet
Development

Successfully merging this pull request may close these issues.

[ts-5.5] Private aliases expanded in index and keyof types - 5.5 regression
4 participants