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

Apply a contextual type to empty arrays #51882

Closed
wants to merge 3 commits into from

Conversation

RyanCavanaugh
Copy link
Member

Some design notes at #51853 (comment)

We'll need to discuss this test in particular: https://github.com/microsoft/TypeScript/compare/main...RyanCavanaugh:ctEmptyArrayRetry?expand=1#diff-143cc97a4709c639aa96ddd0c9cac92f3b8a11b104eb5611e0b83c145fdb571a ; I believe the contextual type providing logic is effectively wrong in this case and will try to make a demonstration of such.

Fixes #51853

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Dec 13, 2022
@RyanCavanaugh
Copy link
Member Author

@typescript-bot test this
@typescript-bot test top100
@typescript-bot user test this
@typescript-bot user test tsserver
@typescript-bot test tsserver top100
@typescript-bot run dt
@typescript-bot perf test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 13, 2022

Heya @RyanCavanaugh, I've started to run the diff-based user code test suite on this PR at eed0acb. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 13, 2022

Heya @RyanCavanaugh, I've started to run the diff-based user code test suite (tsserver) on this PR at eed0acb. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 13, 2022

Heya @RyanCavanaugh, I've started to run the extended test suite on this PR at eed0acb. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 13, 2022

Heya @RyanCavanaugh, I've started to run the diff-based top-repos suite (tsserver) on this PR at eed0acb. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 13, 2022

Heya @RyanCavanaugh, I've started to run the perf test suite on this PR at eed0acb. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 13, 2022

Heya @RyanCavanaugh, I've started to run the diff-based top-repos suite on this PR at eed0acb. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 13, 2022

Heya @RyanCavanaugh, I've started to run the parallelized Definitely Typed test suite on this PR at eed0acb. You can monitor the build here.

@RyanCavanaugh
Copy link
Member Author

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 13, 2022

Heya @RyanCavanaugh, I've started to run the tarball bundle task on this PR at eed0acb. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 13, 2022

Hey @RyanCavanaugh, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/140386/artifacts?artifactName=tgz&fileId=F6AAD7F5EB710C3771E32C2F0B84BF0D18775E64858DC65CEDC499B45BF1DC4902&fileName=/typescript-5.0.0-insiders.20221213.tgz"
    }
}

and then running npm install.


There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@5.0.0-pr-51882-10".;

@typescript-bot
Copy link
Collaborator

@RyanCavanaugh Here are the results of running the user test suite comparing main and refs/pull/51882/merge:

Everything looks good!

1 similar comment
@typescript-bot
Copy link
Collaborator

@RyanCavanaugh Here are the results of running the user test suite comparing main and refs/pull/51882/merge:

Everything looks good!

@typescript-bot
Copy link
Collaborator

Heya @RyanCavanaugh, I've run the RWC suite on this PR - assuming you're on the TS core team, you can view the resulting diff here.

@typescript-bot
Copy link
Collaborator

@RyanCavanaugh Here are the results of running the top-repos suite comparing main and refs/pull/51882/merge:

Everything looks good!

@typescript-bot
Copy link
Collaborator

@RyanCavanaugh Here are the results of running the top-repos suite comparing main and refs/pull/51882/merge:

Something interesting changed - please have a look.

Details

felixrieseberg/windows95

⚠️ Note that built also had errors ⚠️
Req #441 - references
    at formatMessage2 (/typescript-main/built/local/tsserver.js:158985:21)
    at IOSession.writeMessage (/typescript-main/built/local/tsserver.js:159992:21)
    at IOSession.send (/typescript-main/built/local/tsserver.js:159989:10)
    at IOSession.doOutput (/typescript-main/built/local/tsserver.js:160036:10)
    at IOSession.onMessage (/typescript-main/built/local/tsserver.js:161647:14)
    at Interface.<anonymous> (/typescript-main/built/local/tsserver.js:163033:14)
Req #441 - references
    at formatMessage2 (/typescript-51882/built/local/tsserver.js:159004:21)
    at IOSession.writeMessage (/typescript-51882/built/local/tsserver.js:160011:21)
    at IOSession.send (/typescript-51882/built/local/tsserver.js:160008:10)
    at IOSession.doOutput (/typescript-51882/built/local/tsserver.js:160055:10)
    at IOSession.onMessage (/typescript-51882/built/local/tsserver.js:161666:14)
    at Interface.<anonymous> (/typescript-51882/built/local/tsserver.js:163052:14)

That is a filtered view of the text. To see the raw error text, go to RepoResults4/felixrieseberg.windows95.rawError.txt in the artifact folder

Last few requests

{"seq":438,"type":"request","command":"definitionAndBoundSpan","arguments":{"file":"@PROJECT_ROOT@/src/renderer/lib/_libwabt.js","line":13,"offset":95588}}
{"seq":439,"type":"request","command":"completionInfo","arguments":{"file":"@PROJECT_ROOT@/src/renderer/lib/_libwabt.js","line":13,"offset":96407,"includeExternalModuleExports":false,"includeInsertTextCompletions":true,"triggerKind":1}}
{"seq":440,"type":"request","command":"completionEntryDetails","arguments":{"file":"@PROJECT_ROOT@/src/renderer/lib/_libwabt.js","line":13,"offset":96407,"entryNames":["_"]}}
{"seq":441,"type":"request","command":"references","arguments":{"file":"@PROJECT_ROOT@/src/renderer/lib/_libwabt.js","line":13,"offset":97382}}

Repro Steps

  1. git clone https://github.com/felixrieseberg/windows95 --recurse-submodules
  2. In dir windows95, run git reset --hard 17a81393467e2668eb2eea291ab4b6a749756dad
  3. In dir windows95, run yarn install --ignore-engines --ignore-scripts --silent
  4. Back in the initial folder, download RepoResults4/felixrieseberg.windows95.replay.txt from the artifact folder
  5. npm install --no-save @typescript/server-replay
  6. npx tsreplay ./windows95 ./felixrieseberg.windows95.replay.txt path/to/tsserver.js
  7. npx tsreplay --help to learn about helpful switches for debugging, logging, etc

palantir/blueprint

⚠️ Note that built also had errors ⚠️
Req #12600 - references
    at formatMessage2 (/typescript-main/built/local/tsserver.js:158985:21)
    at IOSession.writeMessage (/typescript-main/built/local/tsserver.js:159992:21)
    at IOSession.send (/typescript-main/built/local/tsserver.js:159989:10)
    at IOSession.doOutput (/typescript-main/built/local/tsserver.js:160036:10)
    at IOSession.onMessage (/typescript-main/built/local/tsserver.js:161647:14)
    at Interface.<anonymous> (/typescript-main/built/local/tsserver.js:163033:14)
Req #12600 - references
    at formatMessage2 (/typescript-51882/built/local/tsserver.js:159004:21)
    at IOSession.writeMessage (/typescript-51882/built/local/tsserver.js:160011:21)
    at IOSession.send (/typescript-51882/built/local/tsserver.js:160008:10)
    at IOSession.doOutput (/typescript-51882/built/local/tsserver.js:160055:10)
    at IOSession.onMessage (/typescript-51882/built/local/tsserver.js:161666:14)
    at Interface.<anonymous> (/typescript-51882/built/local/tsserver.js:163052:14)

That is a filtered view of the text. To see the raw error text, go to RepoResults4/palantir.blueprint.rawError.txt in the artifact folder

Last few requests

{"seq":12597,"type":"request","command":"completionInfo","arguments":{"file":"@PROJECT_ROOT@/site/docs/versions/1/docs-app.js","line":1,"offset":245891,"includeExternalModuleExports":false,"includeInsertTextCompletions":true,"triggerKind":2,"triggerCharacter":" "}}
{"seq":12598,"type":"request","command":"completionInfo","arguments":{"file":"@PROJECT_ROOT@/site/docs/versions/1/docs-app.js","line":1,"offset":246033,"includeExternalModuleExports":false,"includeInsertTextCompletions":true,"triggerKind":1}}
{"seq":12599,"type":"request","command":"completionEntryDetails","arguments":{"file":"@PROJECT_ROOT@/site/docs/versions/1/docs-app.js","line":1,"offset":246033,"entryNames":["arguments"]}}
{"seq":12600,"type":"request","command":"references","arguments":{"file":"@PROJECT_ROOT@/site/docs/versions/1/docs-app.js","line":1,"offset":246480}}

Repro Steps

  1. git clone https://github.com/palantir/blueprint --recurse-submodules
  2. In dir blueprint, run git reset --hard b6c1518ea2f524accfba8045ce6f22cac7c5b56e
  3. In dir blueprint, run yarn install --ignore-engines --ignore-scripts --silent
  4. Back in the initial folder, download RepoResults4/palantir.blueprint.replay.txt from the artifact folder
  5. npm install --no-save @typescript/server-replay
  6. npx tsreplay ./blueprint ./palantir.blueprint.replay.txt path/to/tsserver.js
  7. npx tsreplay --help to learn about helpful switches for debugging, logging, etc

@sandersn sandersn added the Experiment A fork with an experimental idea which might not make it into master label Dec 23, 2022
Comment on lines +8 to +9
>isCustom : Symbol(MyCustomArray.isCustom, Decl(arrayContextualTyping.ts, 1, 51))
}
Copy link

Choose a reason for hiding this comment

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

@RyanCavanaugh Could you please clarify what is the purpose of this isCustom property? Looking at this, it seems to me like it is a condition to run a different set of tests, if so it does not seem like a good practice to have conditional tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team Experiment A fork with an experimental idea which might not make it into master For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Interaction of empty arrays and satisfies is unsatisfying
4 participants