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

Type in tooltip resolves correctly. Then resolves to any. Emit always resolves correctly #33067

Closed
AnyhowStep opened this issue Aug 24, 2019 · 8 comments
Assignees
Labels
Needs More Info The issue still hasn't been fully clarified
Milestone

Comments

@AnyhowStep
Copy link
Contributor

AnyhowStep commented Aug 24, 2019

TypeScript Version: 3.5.1

Search Terms:

  • Tooltip
  • Type resolution
  • Emit

Code

TL;DR

Here are two video links that demonstrate the problem,

  1. Editing unrelated text causes resolution to fail, https://imgur.com/a/iXukNKS
  2. Order of resolution affects resolution success/failure, https://imgur.com/a/6H0eRqr

TODO Minimal repro

So, first, the set up,

  1. Clone https://github.com/AnyhowStep/tsql
  2. Checkout commit 6fcd75d9a9572c1c782db983c1c773282b37ce6a
  3. npm install
  4. Open test/compile-time/input/table/as/long-chain.ts#L57
  5. Follow repro below (There's a video link below demonstrating repro)

The repro,

  1. Mouse over aliasedTable, resolves correctly
  2. Mouse over aliasedTable2, resolves correctly
  3. Type some random text
  4. Mouse over aliasedTable2, resolve to any <-- Wtf?
  5. Mouse over aliasedTable, resolve to any <-- Wtf?
  6. Delete the random text
  7. Mouse over aliasedTable2, resolve to any <-- Wtf?
  8. Mouse over aliasedTable, resolve to any <-- Wtf?

Video link: https://imgur.com/a/iXukNKS

I tried to upload logs folowing instructions from #32086 (comment) but...
image

Expected behavior:

Type resolution should not flip-flop between correct results and any

Actual behavior:

Type resolution flip-flops between correct results and any

Playground Link:

TODO Minimal repro

Video link: https://imgur.com/a/iXukNKS

Related Issues:

Might be related to #32707

AnyhowStep added a commit to AnyhowStep/tsql that referenced this issue Aug 24, 2019
@AnyhowStep
Copy link
Contributor Author

However, if you compile, the output will always emit correctly.

  1. npm run test-compile-time
  2. Open test/compile-time/actual-output/table/as/long-chain.d.ts
  3. Observe the output is the same as https://github.com/AnyhowStep/tsql/blob/6fcd75d9a9572c1c782db983c1c773282b37ce6a/test/compile-time/expected-output/table/as/long-chain.d.ts

@AnyhowStep
Copy link
Contributor Author

This is also probably related,
#32573

Something about tsserver is probably caching some result or re-computing some result incorrectly =/
But tsc always starts from scratch (in this case) and doesn't have that problem

@AnyhowStep
Copy link
Contributor Author

AnyhowStep commented Aug 24, 2019

Alternative repro,

  1. Open Command Palette
  2. Developer: Reload Window
  3. Mouse over aliasedTable2 first, resolves to any
  4. Mouse over aliasedTable second, resolves to any
  5. Open Command Palette
  6. Developer: Reload Window
  7. Mouse over aliasedTable first, resolves correctly
  8. Mouse over aliasedTable2 second, resolves correctly

Video link: https://imgur.com/a/6H0eRqr

@AnyhowStep
Copy link
Contributor Author

AnyhowStep commented Aug 24, 2019

Just speculating but this is how I would imagine a fix to be,

  1. Encounter aliasedTable2
  2. Set current instantiationDepth to zero
  3. Start resolving aliasedTable2
  4. instantiationDepth may change
  5. Encounter aliasedTable
    1. Save aliasedTable2's instantiationDepth
    2. Set current instantiationDepth to zero
    3. Start resolving aliasedTable
    4. instantiationDepth may change
    5. Finish resolving aliasedTable
    6. Restore aliasedTable2's instantiationDepth
  6. instantiationDepth may change
  7. Finish resolving aliasedTable2

Also speculating, this is what I imagine is happening right now,

  1. Encounter aliasedTable2
  2. Set current instantiationDepth to zero
  3. Start resolving aliasedTable2
  4. instantiationDepth may change
  5. Encounter aliasedTable
    1. Continue using aliasedTable2's instantiationDepth
    2. Start resolving aliasedTable
    3. instantiationDepth may change
    4. Max depth exceeded, resolve to any
    5. Finish resolving aliasedTable
  6. instantiationDepth may change
  7. Finish resolving aliasedTable2

Then, when generating the tooltip for aliasedTable, it re-uses the earlier cached result.
And that is why we see any when resolving aliasedTable2 before resolving aliasedTable

@sandersn
Copy link
Member

I tried the first repro and the Reload Window repro and couldn't get either to work with 3.8 or 3.5. Notably, I had to npm run build before the file had no errors, because otherwise dist doesn't exist. That's the only difference from the instructions you gave.

@sandersn
Copy link
Member

Ah, I watched the video and I see that it's not the whole type that becomes any; columns[string]["mapper"] just becomes tm.Mapper<unknown, any> instead of tm.Mapper<unknown, bigint>

I got the second repro to work, where you request the type of aliasedTable2 first.

I can't tell whether the repro works on 3.8 because the type is tm.SafeMapper<bigint> in both cases.

@sandersn
Copy link
Member

I guess the next step is to minimise the 3.5 repro to eliminate the variability in Mapper vs SafeMapper.

@sandersn
Copy link
Member

Never mind, 3.8 is just better at retaining type aliases; SafeMapper is just Mapper with the first parameter fixed (which is the one without an error). This is either fixed in 3.8 or needs a different repro.

@sandersn sandersn removed the Needs Investigation This issue needs a team member to investigate its status. label Jan 13, 2020
@sandersn sandersn modified the milestones: TypeScript 3.8.1, Backlog Jan 13, 2020
@sandersn sandersn added the Needs More Info The issue still hasn't been fully clarified label Jan 13, 2020
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

3 participants