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

Open bigger set of configured projects when opening composite project for operations that operate over multiple projects like rename #33287

Merged
merged 13 commits into from
Dec 11, 2019

Conversation

sheetalkamat
Copy link
Member

@sheetalkamat sheetalkamat commented Sep 6, 2019

#28261 on top of #32028

  • Add tests
  • Bigger project opening set means that much longer time to do the operation. So we need some optimizers for this. There are multiple options here and one or many can be chosen.
    • Need some sort of indicator if the operation is exposing something that's outside scope of the project. (eg renaming local function in checker shouldn't need to open this so that we are delaying this as much as possible)
    • May be send more locations as event (eg in find all references if not rename can handle that)
  • We need to have way to unload projects (with some sort of priority based) when the operation might cause too many projects resulting in it running out of memory. This could probably be different PR but then we need some sort of flag to opt out of opening that bigger set so that people have a way to work with the set.

@sheetalkamat
Copy link
Member Author

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 6, 2019

Heya @sheetalkamat, I've started to run the tarball bundle task on this PR at e3de872. You can monitor the build here. It should now contribute to this PR's status checks.

@typescript-bot
Copy link
Collaborator

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/43089/artifacts?artifactName=tgz&fileId=A5E45FC3E112C3C7DBC263D476706F2CBF49D938579E47EAB6995E7E94213C2402&fileName=/typescript-3.7.0-insiders.20190906.tgz"
    }
}

and then running npm install.

@sheetalkamat sheetalkamat changed the base branch from referencesPrototypeSourceFile to master September 24, 2019 20:21
@sheetalkamat sheetalkamat marked this pull request as ready for review October 30, 2019 22:31
@sheetalkamat sheetalkamat changed the title [WIP] Open bigger set of configured projects when opening composite project for operations that operate over multiple projects like rename Open bigger set of configured projects when opening composite project for operations that operate over multiple projects like rename Oct 30, 2019
@RyanCavanaugh
Copy link
Member

LGTM with some name comments

@sheetalkamat sheetalkamat merged commit 4212484 into master Dec 11, 2019
@sheetalkamat sheetalkamat deleted the solutionProjects branch December 11, 2019 20:28
@DanielRosenwasser
Copy link
Member

What does this change imply? Are operations like rename more accurate now?

@sheetalkamat
Copy link
Member Author

This means in our projects setup if you just opened utilities file from compiler and rename some functions in there, it would look for other sibling projects since our solution project references them (even though there is no file from services/server or other such projects open)

@telamonian
Copy link

I just ran into this problem today. How lovely to see that it's already solved. Thank you for working on this!

I was wondering, what release of typescript will this be a part of? I can't seem to find any relevant info

@orta
Copy link
Contributor

orta commented Jan 27, 2020

It should be in 3.8 beta

@sunniejai
Copy link

Does this resolve #30823?

@OliverJAsh
Copy link
Contributor

OliverJAsh commented Feb 10, 2020

I was linked to #30823 from microsoft/vscode#80423 and microsoft/vscode#80438.

I just tested both of the above issues with 3.8 RC. microsoft/vscode#80438 seems to be fixed, but the issue I described in microsoft/vscode#80423 still remains.

@pokey
Copy link

pokey commented Mar 2, 2023

#30823 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants