From a7fd0e255c6314bfecb68f3175f597b306e593bd Mon Sep 17 00:00:00 2001 From: Carlos Espa Date: Wed, 6 Nov 2024 17:42:58 +0100 Subject: [PATCH 01/10] fs: deprecate never throw behaviour in fs.existsSync --- doc/api/deprecations.md | 5 ++++- lib/fs.js | 3 ++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index dea65fe4d5a2b2..b1c4f6b763d548 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -3782,9 +3782,12 @@ changes: - version: v23.4.0 pr-url: https://github.com/nodejs/node/pull/55892 description: Documentation-only. + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/55753 + description: Runtime deprecation. --> -Type: Documentation-only +Type: Runtime Passing non-supported argument types is deprecated and, instead of returning `false`, will throw an error in a future version. diff --git a/lib/fs.js b/lib/fs.js index af72ac36144c55..6099350a6533c8 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -150,6 +150,7 @@ const { validateString, kValidateObjectAllowNullable, } = require('internal/validators'); +const { deprecate } = require('internal/util'); const permission = require('internal/process/permission'); @@ -278,7 +279,6 @@ ObjectDefineProperty(exists, kCustomPromisifiedSymbol, { // the expectation that passing invalid arguments to it, even like // fs.existsSync(), would only get a false in return, so we cannot signal // validation errors to users properly out of compatibility concerns. -// TODO(joyeecheung): deprecate the never-throw-on-invalid-arguments behavior /** * Synchronously tests whether or not the given path exists. * @param {string | Buffer | URL} path @@ -288,6 +288,7 @@ function existsSync(path) { try { path = getValidatedPath(path); } catch { + util.deprecate(() => {}, 'never throw on invalid arguments for fs.existsSync is deprecated', 'DEP187'); return false; } From 247116ed1648b33a88aa41b599dcc86b8221596c Mon Sep 17 00:00:00 2001 From: Carlos Espa Date: Wed, 6 Nov 2024 18:03:03 +0100 Subject: [PATCH 02/10] fixup! fs: deprecate never throw behaviour in fs.existsSync --- lib/fs.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/fs.js b/lib/fs.js index 6099350a6533c8..6ac41c826c3d70 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -288,7 +288,7 @@ function existsSync(path) { try { path = getValidatedPath(path); } catch { - util.deprecate(() => {}, 'never throw on invalid arguments for fs.existsSync is deprecated', 'DEP187'); + util.deprecate(() => {}, 'never throw on invalid arguments for fs.existsSync is deprecated', 'DEP0187'); return false; } From 4c535cc1e9ca552fdeebc15200aafe964a1d2aef Mon Sep 17 00:00:00 2001 From: Carlos Espa Date: Wed, 6 Nov 2024 18:22:17 +0100 Subject: [PATCH 03/10] fixup! fixup! fs: deprecate never throw behaviour in fs.existsSync --- lib/fs.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index 6ac41c826c3d70..bfb6fa2c008b27 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -98,6 +98,7 @@ const { defineLazyProperties, isWindows, isMacOS, + deprecate, } = require('internal/util'); const { constants: { @@ -150,7 +151,6 @@ const { validateString, kValidateObjectAllowNullable, } = require('internal/validators'); -const { deprecate } = require('internal/util'); const permission = require('internal/process/permission'); @@ -288,7 +288,7 @@ function existsSync(path) { try { path = getValidatedPath(path); } catch { - util.deprecate(() => {}, 'never throw on invalid arguments for fs.existsSync is deprecated', 'DEP0187'); + deprecate(() => {}, 'never throw on invalid arguments for fs.existsSync is deprecated', 'DEP0187'); return false; } From be7ef4e9d0424a7c1e6ef0b37db02a56e7b212a2 Mon Sep 17 00:00:00 2001 From: Carlos Espa Date: Wed, 6 Nov 2024 17:42:58 +0100 Subject: [PATCH 04/10] fs: deprecate never throw behaviour in fs.existsSync --- lib/fs.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/fs.js b/lib/fs.js index bfb6fa2c008b27..d16fd6753fe2ec 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -151,6 +151,7 @@ const { validateString, kValidateObjectAllowNullable, } = require('internal/validators'); +const { deprecate } = require('internal/util'); const permission = require('internal/process/permission'); From f7136853427a10a495eb8fd6f447bb92ff0679b6 Mon Sep 17 00:00:00 2001 From: Carlos Espa Date: Tue, 26 Nov 2024 08:33:17 +0100 Subject: [PATCH 05/10] fixup! fs: deprecate never throw behaviour in fs.existsSync --- lib/fs.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/fs.js b/lib/fs.js index d16fd6753fe2ec..f4dd03a55890c4 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -289,7 +289,7 @@ function existsSync(path) { try { path = getValidatedPath(path); } catch { - deprecate(() => {}, 'never throw on invalid arguments for fs.existsSync is deprecated', 'DEP0187'); + deprecate(() => {}, 'Passing invalid argument types to fs.existsSync is deprecated', 'DEP0187'); return false; } From f32d8738feaa347f54e1f66189f0957240c91d37 Mon Sep 17 00:00:00 2001 From: Carlos Espa Date: Tue, 26 Nov 2024 09:06:10 +0100 Subject: [PATCH 06/10] fixup! fs: deprecate never throw behaviour in fs.existsSync --- lib/fs.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index f4dd03a55890c4..d94dabf926a39f 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -98,7 +98,6 @@ const { defineLazyProperties, isWindows, isMacOS, - deprecate, } = require('internal/util'); const { constants: { @@ -151,7 +150,6 @@ const { validateString, kValidateObjectAllowNullable, } = require('internal/validators'); -const { deprecate } = require('internal/util'); const permission = require('internal/process/permission'); From 83687f466356c29460283b29aeb16987e0724016 Mon Sep 17 00:00:00 2001 From: Carlos Espa <43477095+Ceres6@users.noreply.github.com> Date: Wed, 27 Nov 2024 08:27:05 +0100 Subject: [PATCH 07/10] Apply suggestions from code review Co-authored-by: Antoine du Hamel --- doc/api/deprecations.md | 3 +++ lib/fs.js | 5 ++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index b1c4f6b763d548..c5a4e16184e873 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -3785,6 +3785,9 @@ changes: - version: REPLACEME pr-url: https://github.com/nodejs/node/pull/55753 description: Runtime deprecation. + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/55892 + description: Documentation-only. --> Type: Runtime diff --git a/lib/fs.js b/lib/fs.js index d94dabf926a39f..332298d4bcabd0 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -287,7 +287,10 @@ function existsSync(path) { try { path = getValidatedPath(path); } catch { - deprecate(() => {}, 'Passing invalid argument types to fs.existsSync is deprecated', 'DEP0187'); + if (!deprecationWarningEmitted) { + process.emitWarning('Passing invalid argument types to fs.existsSync is deprecated', 'DeprecationWarning', 'DEP0187'); + deprecationWarningEmitted = true; + } return false; } From 0484b00374b10a34d531d055352812402333997c Mon Sep 17 00:00:00 2001 From: Carlos Espa Date: Wed, 27 Nov 2024 19:17:06 +0100 Subject: [PATCH 08/10] fixup! fs: deprecate never throw behaviour in fs.existsSync --- lib/fs.js | 14 ++++++-------- test/parallel/test-fs-exists.js | 2 ++ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index 332298d4bcabd0..e2996ba9ca4ef6 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -273,11 +273,7 @@ ObjectDefineProperty(exists, kCustomPromisifiedSymbol, { }, }); -// fs.existsSync never throws, it only returns true or false. -// Since fs.existsSync never throws, users have established -// the expectation that passing invalid arguments to it, even like -// fs.existsSync(), would only get a false in return, so we cannot signal -// validation errors to users properly out of compatibility concerns. +let showExistsDeprecation = true; /** * Synchronously tests whether or not the given path exists. * @param {string | Buffer | URL} path @@ -287,9 +283,11 @@ function existsSync(path) { try { path = getValidatedPath(path); } catch { - if (!deprecationWarningEmitted) { - process.emitWarning('Passing invalid argument types to fs.existsSync is deprecated', 'DeprecationWarning', 'DEP0187'); - deprecationWarningEmitted = true; + if (showExistsDeprecation) { + process.emitWarning( + 'Passing invalid argument types to fs.existsSync is deprecated', 'DeprecationWarning', 'DEP0187', + ); + showExistsDeprecation = false; } return false; } diff --git a/test/parallel/test-fs-exists.js b/test/parallel/test-fs-exists.js index 857f3f26174549..3be2197660ce8c 100644 --- a/test/parallel/test-fs-exists.js +++ b/test/parallel/test-fs-exists.js @@ -51,6 +51,8 @@ assert(fs.existsSync(f)); assert(!fs.existsSync(`${f}-NO`)); // fs.existsSync() never throws +const msg = 'Passing invalid argument types to fs.existsSync is deprecated'; +common.expectWarning('DeprecationWarning', msg, 'DEP0187'); assert(!fs.existsSync()); assert(!fs.existsSync({})); assert(!fs.existsSync(new URL('https://foo'))); From 86bde37998cdb5e4596f2978ade0515776c64764 Mon Sep 17 00:00:00 2001 From: Carlos Espa Date: Mon, 6 Jan 2025 10:54:12 +0100 Subject: [PATCH 09/10] fixup! fs: deprecate never throw behaviour in fs.existsSync --- doc/api/deprecations.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index c5a4e16184e873..b1c4f6b763d548 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -3785,9 +3785,6 @@ changes: - version: REPLACEME pr-url: https://github.com/nodejs/node/pull/55753 description: Runtime deprecation. - - version: REPLACEME - pr-url: https://github.com/nodejs/node/pull/55892 - description: Documentation-only. --> Type: Runtime From f70e0a2fe7e84cc684165d14bd37fa5cde7e7fb4 Mon Sep 17 00:00:00 2001 From: Carlos Espa <43477095+Ceres6@users.noreply.github.com> Date: Mon, 6 Jan 2025 11:21:39 +0100 Subject: [PATCH 10/10] Update doc/api/deprecations.md Co-authored-by: Livia Medeiros --- doc/api/deprecations.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index b1c4f6b763d548..e40f284f414270 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -3779,12 +3779,12 @@ It is recommended to use the `new` qualifier instead. This applies to all REPL c Type: Runtime