Skip to content

Don't recurse when the answer doesn't change to 'true', just continue traversal #39934

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Don't recurse when the answer doesn't change to 'true', just continue traversal #39934

wants to merge 2 commits into from

Conversation

DanielRosenwasser
Copy link
Member

@DanielRosenwasser DanielRosenwasser commented Aug 6, 2020 •
edited
Loading

I dunno what I'm doing!

Maybe fixes #39923.

@typescript-bot typescript-bot added For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Aug 6, 2020
@DanielRosenwasser DanielRosenwasser changed the title Don't recurse when the answer doesn't change to 'true', just bail out. Don't recurse when the answer doesn't change to 'true', just continue traversal Aug 6, 2020
@DanielRosenwasser
Copy link
Member Author

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 6, 2020 •
edited
Loading

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

@DanielRosenwasser
Copy link
Member Author

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 6, 2020 •
edited
Loading

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 6, 2020 •
edited
Loading

Hey @DanielRosenwasser, 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/81648/artifacts?artifactName=tgz&fileId=CE39EDC5E747B41CA73C0E2CE15CB079347B1916AC68FBAC439668A352AFBCDE02&fileName=/typescript-4.1.0-insiders.20200806.tgz"
    }
}

and then running npm install.

There is also a playground for this build.

@DanielRosenwasser
Copy link
Member Author

DanielRosenwasser commented Aug 6, 2020 •
edited
Loading

Looks like it fixes the crash, but I don't know how to write a test for this.

@typescript-bot typescript-bot removed the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Aug 6, 2020
@DanielRosenwasser DanielRosenwasser marked this pull request as ready for review August 6, 2020 07:08
@typescript-bot typescript-bot added the For Milestone Bug PRs that fix a bug with a specific milestone label Aug 6, 2020
@DanielRosenwasser
Copy link
Member Author

Looks like the assertion keeps getting hit. I'm not sure if it should ever be possible for the value to remain as Maybe. Should it just become False?

DanielRosenwasser
DanielRosenwasser commented Aug 6, 2020
View reviewed changes
src/server/editorServices.ts
@@ -3775,24 +3775,28 @@ namespace ts.server {
getPackageJsonsVisibleToFile(fileName: string, rootDir?: string): readonly PackageJsonInfo[] {
const packageJsonCache = this.packageJsonCache;
const watchPackageJsonFile = this.watchPackageJsonFile.bind(this);
const toPath = this.toPath.bind(this);
Copy link
Member Author

Choose a reason for hiding this comment

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

this was actually turning into any.

@andrewbranch
Copy link
Member

I have a fix at #39937

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewbranch andrewbranch Awaiting requested review from andrewbranch

@sheetalkamat sheetalkamat Awaiting requested review from sheetalkamat

Assignees

@andrewbranch andrewbranch

Labels
For Milestone Bug PRs that fix a bug with a specific milestone
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

TS Server fatal error: Maximum call stack size exceeded
3 participants
@DanielRosenwasser @typescript-bot @andrewbranch