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

Reordering variable declarations of Promise<any> and Promise<unknown> changes behavior #52100

Open
dragomirtitian opened this issue Jan 4, 2023 · 6 comments
Labels
Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript

Comments

@dragomirtitian
Copy link
Contributor

Bug Report

πŸ”Ž Search Terms

Promise<any> Promise<unknown> instantiation order

πŸ•— Version & Regression Information

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

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

// Reorder these two
declare let u: Promise<unknown>
declare let a: Promise<any>

// Assigned an expression that is Promise<any> | Promise<unknown>
// but that will undergo subtype reduction
// The type of union depends on whether a or u was declared first
let union = Math.random() > 0.5
    ? Promise.reject<any>()
    : Promise.reject<unknown>();

union.then(v => v.toString());

πŸ™ Actual behavior

Reordering the unrelated variables u and a will cause the type of union to change between Promise<unknown> and Promise<any> (both in declarations and in the type of v, although tooltips still show union as Promise<any>)

πŸ™‚ Expected behavior

The type of union should not be impacted

@fatcerberus
Copy link

I’m thinking TS probably considers Promise<any> and Promise<unknown> to be subtypes of each other (subtyping is not a total order, from what I understand), so subtype reduction is order-dependent.

@dragomirtitian
Copy link
Contributor Author

dragomirtitian commented Jan 4, 2023

@fatcerberus That is my conclusion as well. Promise<any> and Promise<unknown> are subtypes of each other. When reducing the union it matters in what order the constituents are in the union because that will determine what question is considered (Promise<any> extends Promise<unknown> and Promise<unknown> extends Promise<any> are both true ) And the order of union constituents is determined by the id of the type which is assigned in the order types are encountered.

@robpalme
Copy link

Given the conclusion on ordering issues in the 6th Jan Design Meeting...

Don't really have any solution here. Many similar issues show up.

This implies that any direct user of the checker API needs to ensure that the order of source files fed to TypeScript is deterministic and, crucially, matches the order used by tsc and the VS Code Language Service Plugin (LSP).

The file load order is a significant piece of knowledge that anyone embedding the TypeScript checker in a build tool needs to know and replicate. Otherwise the types and checking errors reported may differ between the IDE/LSP and the build tool.

Ideal Solution

Ideally, the TypeScript language would make type declaration ordering irrelevant. That is easiest for users to understand and, as a bonus, permits the most flexibility and performance. But it sounds like that is not on the table right now.

A second-best solution could be to make it possible to ban or lint-away any usage of the language that incurs ordering dependencies, so that projects could opt-into the subset of the language where ordering doesn't matter.

Fallback Solution

Today, we embed the TypeScript checker in a custom toolchain that is used locally and during a remote project publishing step. But we don't have a custom LSP - we reuse the default VS Code LSP and configure it via a generated tsconfig written to the source directory. So for true consistency, it appears that we need to update the toolchain to ensure that we are matching the canonical TypeScript file load order.

Is this canonical load order specified anywhere? Is there a way to reuse TypeScript's existing logic here? It would be great to reuse that code so that if TypeScript ever changes the order, our toolchain continues to match.

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. labels Jan 13, 2023
@RyanCavanaugh
Copy link
Member

Is this canonical load order specified anywhere?

I'll give a rough sketch. The order is:

  • Lib files
  • The files array, in its original order
  • The include array, the constituents of which I believe are alphabetically sorted (not sure) on a per-pattern basis

This order is not final; it's then stably sorted by the dependency graph implied by reference directives and import directives.

@dragomirtitian
Copy link
Contributor Author

@RyanCavanaugh This issue now appears to not happen anymore, and inference has stabilized on Promise<any>. This happened between 5.0.0-dev.20230210 and 5.0.0-dev.20230216 (can't pin point more accurately as playground has a gap of version between those dates)

Was this intentional? Is this going to remain so going forward?

@RyanCavanaugh
Copy link
Member

AFAIK, we haven't done anything which we believed to be a fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants