Skip to content

Schedule failed lookup updates #38560

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

Merged
merged 2 commits into from
Jun 9, 2020
Merged

Schedule failed lookup updates #38560

merged 2 commits into from
Jun 9, 2020

Conversation

sheetalkamat
Copy link
Member

@sheetalkamat sheetalkamat commented May 13, 2020

Schedule invalidation of module resolution cache on changes to failed lookup locations.
Fixes #37653

@sheetalkamat
Copy link
Member Author

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 13, 2020

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

@sheetalkamat
Copy link
Member Author

@weswigham does bot not pack build that fails to run tests

@weswigham
Copy link
Member

Eeeyeah, it does not; it's templated off the publish task which, for sanity reasons, makes sure tests pass prior to publication. If you wanna override those sanity checks in a PR, you can always game the system a little bit and edit the Gulpfile to not actually test anything while you're getting trial builds made (eg, by deleting the contents of the runTestsParallel task). It's a pretty good reminder that it's still WIP.

@sheetalkamat
Copy link
Member Author

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 13, 2020

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 14, 2020

Hey @sheetalkamat, 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/73861/artifacts?artifactName=tgz&fileId=3E697F8F13F606B519C3FE6EA0DAFDE2E5BBE7A6B6C747B1B14789B23967C11C02&fileName=/typescript-4.0.0-insiders.20200514.tgz"
    }
}

and then running npm install.


There is also a playground for this build.

@sheetalkamat sheetalkamat marked this pull request as ready for review June 8, 2020 21:45
Copy link
Member

@andrewbranch andrewbranch left a comment

Choose a reason for hiding this comment

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

These changes make sense to me; it’s clear where you’re throttling an operation that was previously not throttled. The only thing that I can’t figure out is why this wasn’t a problem with npm as well as yarn.

@sheetalkamat
Copy link
Member Author

@andrewbranch i dont understand what you mean. #37055 changed how module resolutions are updated and that means that npm/yarn install notifications were producing lot more work to check if things need to be invalidated. Before that PR we would just mark project as ready for update and schedule one which would keep on cancelling and scheduling so not much work involved in there. This PR merges both behaviours by scheduling invalidation of module resolutions so we can process them all at once instead of on every notification from FS.

@sheetalkamat sheetalkamat merged commit a72ed0a into master Jun 9, 2020
@sheetalkamat sheetalkamat deleted the invalidate branch June 9, 2020 19:00
@andrewbranch
Copy link
Member

Ok, I was under the impression that this bug was unique to yarn.

cangSDARM added a commit to cangSDARM/TypeScript that referenced this pull request Jun 10, 2020
* upstream/master: (36 commits)
  Update baselines (microsoft#39000)
  Make isEntityNameVisible duplicate the node builder logic to always consider type parameters as visible if they are the resolution result (microsoft#38921)
  Schedule failed lookup updates (microsoft#38560)
  Remove non null assertion on oldSourceFile.resolvedModules (microsoft#38984)
  use blocklist instead of blacklist (microsoft#38988)
  LEGO: check in for master to temporary branch.
  Deprecate reloadFs so the tests are more clear in what they are achieving and its easier to track changed behaviour (microsoft#38954)
  LEGO: check in for master to temporary branch.
  LEGO: check in for master to temporary branch.
  Make `hasCorrectArity` handle tuples properly
  fix error when use spread arguments twice
  fix(38081): allow transforming object binding to named imports
  Accept baseline
  fix names
  Fix type and simplify code
  accept new baseline
  fix rebase conflict
  add missing semi
  Add more check
  fix mission baseline
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TS Server high cpu usage after yarn install
4 participants