-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Revert changes to matchFiles/readDirectory made since 4.3 #46787
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this re-break the RWC tests?
src/compiler/sys.ts
Outdated
@@ -1620,12 +1619,6 @@ namespace ts { | |||
options = { persistent: true }; | |||
} | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand a full revert is safer at this point in the cycle, but it would be nice to keep (or quickly restore) this part of the change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you review the latest commit and see if this is what you had in mind?
@@ -182,34 +182,3 @@ describe("unittests:: Public APIs:: getChild* methods on EndOfFileToken with JSD | |||
assert.equal(endOfFileToken.getChildCount(), 1); | |||
assert.notEqual(endOfFileToken.getChildAt(0), /*expected*/ undefined); | |||
}); | |||
|
|||
describe("unittests:: Public APIs:: sys", () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worth keeping?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We’ll bring it back when we decide what the correct thing to assert is
LGTM |
@typescript-bot cherry-pick this to release-4.5 |
Heya @andrewbranch, I've started to run the task to cherry-pick this into |
Hey @andrewbranch, I've opened #46789 for you. |
Component commits: 931b504 Revert "Fix RWC missing file detection (microsoft#46673)" This reverts commit 4a065f5. afef282 Revert "Pass absolute path to directoryExists (microsoft#46086)" This reverts commit 55b4928. f1a20b3 Revert "Reduce exceptions (microsoft#44710)" This reverts commit c0d5c29. 56842cd Add back system watcher limit
Component commits: 931b504 Revert "Fix RWC missing file detection (#46673)" This reverts commit 4a065f5. afef282 Revert "Pass absolute path to directoryExists (#46086)" This reverts commit 55b4928. f1a20b3 Revert "Reduce exceptions (#44710)" This reverts commit c0d5c29. 56842cd Add back system watcher limit Co-authored-by: Andrew Branch <[email protected]>
…46787) * Revert "Fix RWC missing file detection (microsoft#46673)" This reverts commit 4a065f5. * Revert "Pass absolute path to directoryExists (microsoft#46086)" This reverts commit 55b4928. * Revert "Reduce exceptions (microsoft#44710)" This reverts commit c0d5c29. * Add back system watcher limit
Reverts #44710, #46086, and #46673.
Fixes #46577.
I have discovered that
ts.matchFiles
/ts.sys.readDirectory
does a lot of extremely confusing things, none of which are documented and some of which are probably bugs, but nobody seemed to notice until we started messing with it, so I’m reverting back to our 4.3 state. I will post my findings of what’s weird even in 4.3 so we can decide if any of it is worth fixing (done: #46788).