lib: add throws option to fs.f/l/statSync by amcasey · Pull Request #33716 · nodejs/node
amcasey
changed the title
lib: add noThrow option to fs.f/l/statSync
lib: add throws option to fs.f/l/statSync
aduh95 pushed a commit to aduh95/node that referenced this pull request
For consumers that aren't interested in *why* a `statSync` call failed, allocating and throwing an exception is an unnecessary expense. This PR adds an option that will cause it to return `undefined` in such cases instead. As a motivating example, the JavaScript & TypeScript language service shared between Visual Studio and Visual Studio Code is stuck with synchronous file IO for architectural and backward-compatibility reasons. It frequently needs to speculatively check for the existence of files and directories that may not exist (and cares about file vs directory, so `existsSync` is insufficient), but ignores file system entries it can't access, regardless of the reason. Benchmarking the language service is difficult because it's so hard to get good coverage of both code bases and user behaviors, but, as a representative metric, we measured batch compilation of a few hundred popular projects (by star count) from GitHub and found that, on average, we saved about 1-2% of total compilation time. We speculate that the savings could be even more significant in interactive (language service or watch mode) scenarios, where the same (non-existent) files need to be polled over and over again. It's not a huge improvement, but it's a very small change and it will affect a lot of users (and CI runs). For reference, our measurements were against `v12.x` (3637a06 at the time) on an Ubuntu Server desktop with an SSD. PR-URL: nodejs#33716 Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
targos pushed a commit that referenced this pull request
For consumers that aren't interested in *why* a `statSync` call failed, allocating and throwing an exception is an unnecessary expense. This PR adds an option that will cause it to return `undefined` in such cases instead. As a motivating example, the JavaScript & TypeScript language service shared between Visual Studio and Visual Studio Code is stuck with synchronous file IO for architectural and backward-compatibility reasons. It frequently needs to speculatively check for the existence of files and directories that may not exist (and cares about file vs directory, so `existsSync` is insufficient), but ignores file system entries it can't access, regardless of the reason. Benchmarking the language service is difficult because it's so hard to get good coverage of both code bases and user behaviors, but, as a representative metric, we measured batch compilation of a few hundred popular projects (by star count) from GitHub and found that, on average, we saved about 1-2% of total compilation time. We speculate that the savings could be even more significant in interactive (language service or watch mode) scenarios, where the same (non-existent) files need to be polled over and over again. It's not a huge improvement, but it's a very small change and it will affect a lot of users (and CI runs). For reference, our measurements were against `v12.x` (3637a06 at the time) on an Ubuntu Server desktop with an SSD. PR-URL: #33716 Backport-PR-URL: #36921 Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
DanielRosenwasser pushed a commit to microsoft/TypeScript that referenced this pull request
Future versions of node will be able to return undefined, rather than allocating and throwing an exception, when a file is not found. See nodejs/node#33716
DanielRosenwasser pushed a commit to microsoft/TypeScript that referenced this pull request
Future versions of node will be able to return undefined, rather than allocating and throwing an exception, when a file is not found. See nodejs/node#33716
DanielRosenwasser pushed a commit to microsoft/TypeScript that referenced this pull request
Future versions of node will be able to return undefined, rather than allocating and throwing an exception, when a file is not found. See nodejs/node#33716
DanielRosenwasser added a commit to microsoft/TypeScript that referenced this pull request
Future versions of node will be able to return undefined, rather than allocating and throwing an exception, when a file is not found. See nodejs/node#33716 Co-authored-by: Andrew Casey <amcasey@users.noreply.github.com>
DanielRosenwasser added a commit to microsoft/TypeScript that referenced this pull request
Future versions of node will be able to return undefined, rather than allocating and throwing an exception, when a file is not found. See nodejs/node#33716 Co-authored-by: Andrew Casey <amcasey@users.noreply.github.com>
DanielRosenwasser added a commit to microsoft/TypeScript that referenced this pull request
Future versions of node will be able to return undefined, rather than allocating and throwing an exception, when a file is not found. See nodejs/node#33716 Co-authored-by: Andrew Casey <amcasey@users.noreply.github.com>
nodejs-github-bot pushed a commit that referenced this pull request
Refs: #33716 Refs: #35993 Refs: #35911 PR-URL: #39972 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
nodejs-github-bot pushed a commit that referenced this pull request
PR-URL: #39972 Refs: #33716 Refs: #35993 Refs: #35911 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
BethGriggs pushed a commit that referenced this pull request
Refs: #33716 Refs: #35993 Refs: #35911 PR-URL: #39972 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
kovan added a commit to kovan/node that referenced this pull request
PR nodejs#61178 added the throwIfNoEntry option to fs.stat and fsPromises.stat (shipped in v25.7.0) but did not add corresponding YAML version history entries. The sync variants (fs.statSync, fs.lstatSync) already had history entries from PR nodejs#33716. Fixes: nodejs#62185 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
kovan
mentioned this pull request
kovan added a commit to kovan/node that referenced this pull request
PR nodejs#61178 added the throwIfNoEntry option to fs.stat and fsPromises.stat (shipped in v25.7.0) but did not add corresponding YAML version history entries. The sync variants (fs.statSync, fs.lstatSync) already had history entries from PR nodejs#33716. Fixes: nodejs#62185 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
kovan
mentioned this pull request
nodejs-github-bot pushed a commit that referenced this pull request
PR #61178 added the throwIfNoEntry option to fs.stat and fsPromises.stat (shipped in v25.7.0) but did not add corresponding YAML version history entries. The sync variants (fs.statSync, fs.lstatSync) already had history entries from PR #33716. Fixes: #62185 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> PR-URL: #62204 Reviewed-By: René <contact.9a5d6388@renegade334.me.uk> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
aduh95 pushed a commit that referenced this pull request
PR #61178 added the throwIfNoEntry option to fs.stat and fsPromises.stat (shipped in v25.7.0) but did not add corresponding YAML version history entries. The sync variants (fs.statSync, fs.lstatSync) already had history entries from PR #33716. Fixes: #62185 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> PR-URL: #62204 Reviewed-By: René <contact.9a5d6388@renegade334.me.uk> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
aduh95 pushed a commit that referenced this pull request
PR #61178 added the throwIfNoEntry option to fs.stat and fsPromises.stat (shipped in v25.7.0) but did not add corresponding YAML version history entries. The sync variants (fs.statSync, fs.lstatSync) already had history entries from PR #33716. Fixes: #62185 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> PR-URL: #62204 Reviewed-By: René <contact.9a5d6388@renegade334.me.uk> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>