From b0f3cc21b81d9598ecf1b4eaf01d7dfb0fefb6bb Mon Sep 17 00:00:00 2001 From: Giovanni Date: Tue, 13 Aug 2024 16:01:08 +0200 Subject: [PATCH 1/8] esm: mordernize old tests Co-Authored-By: Jacob Smith --- test/es-module/test-esm-type-field-errors.js | 56 +++++++++++++------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/test/es-module/test-esm-type-field-errors.js b/test/es-module/test-esm-type-field-errors.js index d9d264cc25aa25..ddde0d0f607df3 100644 --- a/test/es-module/test-esm-type-field-errors.js +++ b/test/es-module/test-esm-type-field-errors.js @@ -2,6 +2,7 @@ const common = require('../common'); const assert = require('assert'); const exec = require('child_process').execFile; +const { describe, it } = require('node:test'); const mjsFile = require.resolve('../fixtures/es-modules/mjs-file.mjs'); const cjsFile = require.resolve('../fixtures/es-modules/cjs-file.cjs'); @@ -13,25 +14,42 @@ const packageTypeModuleMain = require.resolve('../fixtures/es-modules/package-type-module/index.js'); // Check that running `node` without options works -expect('', mjsFile, '.mjs file'); -expect('', cjsFile, '.cjs file'); -expect('', packageTypeModuleMain, 'package-type-module'); -expect('', packageTypeCommonJsMain, 'package-type-commonjs'); -expect('', packageWithoutTypeMain, 'package-without-type'); - -// Check that --input-type isn't allowed for files -expect('--input-type=module', packageTypeModuleMain, - 'ERR_INPUT_TYPE_NOT_ALLOWED', true); - -try { - require('../fixtures/es-modules/package-type-module/index.js'); - assert.fail('Expected CJS to fail loading from type: module package.'); -} catch (e) { - assert.strictEqual(e.name, 'Error'); - assert.strictEqual(e.code, 'ERR_REQUIRE_ESM'); - assert(e.toString().match(/require\(\) of ES Module/g)); - assert(e.message.match(/require\(\) of ES Module/g)); -} +describe('ESM type field errors', { concurrency: true }, () => { + it('.cjs file', () => { + expect('', cjsFile, '.cjs file'); + }); + + it('.mjs file', () => { + expect('', mjsFile, '.mjs file'); + }); + + it('package.json with "type": "module"', () => { + expect('', packageTypeModuleMain, 'package-type-module'); + }); + + it('package.json with "type": "commonjs"', () => { + expect('', packageTypeCommonJsMain, 'package-type-commonjs'); + }); + + it('package.json with no "type" field', () => { + expect('', packageWithoutTypeMain, 'package-without-type'); + }); + + it('--input-type=module disallowed for files', () => { + expect( + '--input-type=module', + packageTypeModuleMain, + 'ERR_INPUT_TYPE_NOT_ALLOWED', + true, + ); + }); + + it('--input-type=module disallowed for directories', () => { + assert.throws(() => require('../fixtures/es-modules/package-type-module/index.js'), { + code: 'ERR_REQUIRE_ESM' + }); + }); +}); function expect(opt = '', inputFile, want, wantsError = false) { const argv = [inputFile]; From 089f5fc59b6f889df4272cf66e85fe13acef28bd Mon Sep 17 00:00:00 2001 From: Giovanni Date: Tue, 13 Aug 2024 16:01:08 +0200 Subject: [PATCH 2/8] esm: mordernize old tests Co-Authored-By: Jacob Smith --- test/es-module/test-esm-type-field-errors.js | 59 +++++++++++++------- 1 file changed, 40 insertions(+), 19 deletions(-) diff --git a/test/es-module/test-esm-type-field-errors.js b/test/es-module/test-esm-type-field-errors.js index d9d264cc25aa25..7cad1c42d56e57 100644 --- a/test/es-module/test-esm-type-field-errors.js +++ b/test/es-module/test-esm-type-field-errors.js @@ -2,6 +2,7 @@ const common = require('../common'); const assert = require('assert'); const exec = require('child_process').execFile; +const { describe, it } = require('node:test'); const mjsFile = require.resolve('../fixtures/es-modules/mjs-file.mjs'); const cjsFile = require.resolve('../fixtures/es-modules/cjs-file.cjs'); @@ -13,25 +14,45 @@ const packageTypeModuleMain = require.resolve('../fixtures/es-modules/package-type-module/index.js'); // Check that running `node` without options works -expect('', mjsFile, '.mjs file'); -expect('', cjsFile, '.cjs file'); -expect('', packageTypeModuleMain, 'package-type-module'); -expect('', packageTypeCommonJsMain, 'package-type-commonjs'); -expect('', packageWithoutTypeMain, 'package-without-type'); - -// Check that --input-type isn't allowed for files -expect('--input-type=module', packageTypeModuleMain, - 'ERR_INPUT_TYPE_NOT_ALLOWED', true); - -try { - require('../fixtures/es-modules/package-type-module/index.js'); - assert.fail('Expected CJS to fail loading from type: module package.'); -} catch (e) { - assert.strictEqual(e.name, 'Error'); - assert.strictEqual(e.code, 'ERR_REQUIRE_ESM'); - assert(e.toString().match(/require\(\) of ES Module/g)); - assert(e.message.match(/require\(\) of ES Module/g)); -} +describe('ESM type field errors', { concurrency: true }, () => { + it('.cjs file', () => { + expect('', cjsFile, '.cjs file'); + }); + + it('.mjs file', () => { + expect('', mjsFile, '.mjs file'); + }); + + it('package.json with "type": "module"', () => { + expect('', packageTypeModuleMain, 'package-type-module'); + }); + + it('package.json with "type": "commonjs"', () => { + expect('', packageTypeCommonJsMain, 'package-type-commonjs'); + }); + + it('package.json with no "type" field', () => { + expect('', packageWithoutTypeMain, 'package-without-type'); + }); + + it('--input-type=module disallowed for files', () => { + expect( + '--input-type=module', + packageTypeModuleMain, + 'ERR_INPUT_TYPE_NOT_ALLOWED', + true, + ); + }); + + it('--input-type=module disallowed for directories', () => { + try { + require('../fixtures/es-modules/package-type-module/index.js'); + assert.fail('Expected CJS to fail loading from type: module package.'); + } catch (e) { + assert.throws(() => { throw e; }, /ERR_REQUIRE_ESM/); + } + }); +}); function expect(opt = '', inputFile, want, wantsError = false) { const argv = [inputFile]; From b26460cb45c637688f6366bdbaa96d45029c38b5 Mon Sep 17 00:00:00 2001 From: Giovanni Date: Tue, 13 Aug 2024 21:06:40 +0200 Subject: [PATCH 3/8] esm: refactored the directory test to directly check the thrown error --- test/es-module/test-esm-type-field-errors.js | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/test/es-module/test-esm-type-field-errors.js b/test/es-module/test-esm-type-field-errors.js index 7cad1c42d56e57..60439d99269585 100644 --- a/test/es-module/test-esm-type-field-errors.js +++ b/test/es-module/test-esm-type-field-errors.js @@ -45,12 +45,7 @@ describe('ESM type field errors', { concurrency: true }, () => { }); it('--input-type=module disallowed for directories', () => { - try { - require('../fixtures/es-modules/package-type-module/index.js'); - assert.fail('Expected CJS to fail loading from type: module package.'); - } catch (e) { - assert.throws(() => { throw e; }, /ERR_REQUIRE_ESM/); - } + assert.throws(() => require('../fixtures/es-modules/package-type-module/index.js'), /ERR_REQUIRE_ESM/); }); }); From 738ce28c8c5540bdc3d577d7be23cc7834c3b979 Mon Sep 17 00:00:00 2001 From: Giovanni Date: Wed, 14 Aug 2024 08:56:23 +0200 Subject: [PATCH 4/8] esm: refactored the directory test to check for the error code --- test/es-module/test-esm-type-field-errors.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/es-module/test-esm-type-field-errors.js b/test/es-module/test-esm-type-field-errors.js index 60439d99269585..ddde0d0f607df3 100644 --- a/test/es-module/test-esm-type-field-errors.js +++ b/test/es-module/test-esm-type-field-errors.js @@ -45,7 +45,9 @@ describe('ESM type field errors', { concurrency: true }, () => { }); it('--input-type=module disallowed for directories', () => { - assert.throws(() => require('../fixtures/es-modules/package-type-module/index.js'), /ERR_REQUIRE_ESM/); + assert.throws(() => require('../fixtures/es-modules/package-type-module/index.js'), { + code: 'ERR_REQUIRE_ESM' + }); }); }); From 60518269bc37ea78719dc50a4381d9a106a2b734 Mon Sep 17 00:00:00 2001 From: Wiyeong Seo Date: Wed, 14 Aug 2024 20:18:49 +0900 Subject: [PATCH 5/8] path: change `posix.join` to use array Change posix.join to use array.join instead of additional assignment. PR-URL: https://github.com/nodejs/node/pull/54331 Reviewed-By: Antoine du Hamel Reviewed-By: Yagiz Nizipli --- lib/path.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/path.js b/lib/path.js index eba07f376ad0f9..48c7f67dcd617a 100644 --- a/lib/path.js +++ b/lib/path.js @@ -1236,20 +1236,20 @@ const posix = { join(...args) { if (args.length === 0) return '.'; - let joined; + + const path = []; for (let i = 0; i < args.length; ++i) { const arg = args[i]; validateString(arg, 'path'); if (arg.length > 0) { - if (joined === undefined) - joined = arg; - else - joined += `/${arg}`; + path.push(arg); } } - if (joined === undefined) + + if (path.length === 0) return '.'; - return posix.normalize(joined); + + return posix.normalize(ArrayPrototypeJoin(path, '/')); }, /** From f25213821db605019c2036169eedd7cd98f582e7 Mon Sep 17 00:00:00 2001 From: Giovanni Date: Tue, 13 Aug 2024 16:01:08 +0200 Subject: [PATCH 6/8] esm: modernize old tests Co-Authored-By: Jacob Smith --- test/es-module/test-esm-type-field-errors.js | 56 +++++++++++++------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/test/es-module/test-esm-type-field-errors.js b/test/es-module/test-esm-type-field-errors.js index d9d264cc25aa25..ddde0d0f607df3 100644 --- a/test/es-module/test-esm-type-field-errors.js +++ b/test/es-module/test-esm-type-field-errors.js @@ -2,6 +2,7 @@ const common = require('../common'); const assert = require('assert'); const exec = require('child_process').execFile; +const { describe, it } = require('node:test'); const mjsFile = require.resolve('../fixtures/es-modules/mjs-file.mjs'); const cjsFile = require.resolve('../fixtures/es-modules/cjs-file.cjs'); @@ -13,25 +14,42 @@ const packageTypeModuleMain = require.resolve('../fixtures/es-modules/package-type-module/index.js'); // Check that running `node` without options works -expect('', mjsFile, '.mjs file'); -expect('', cjsFile, '.cjs file'); -expect('', packageTypeModuleMain, 'package-type-module'); -expect('', packageTypeCommonJsMain, 'package-type-commonjs'); -expect('', packageWithoutTypeMain, 'package-without-type'); - -// Check that --input-type isn't allowed for files -expect('--input-type=module', packageTypeModuleMain, - 'ERR_INPUT_TYPE_NOT_ALLOWED', true); - -try { - require('../fixtures/es-modules/package-type-module/index.js'); - assert.fail('Expected CJS to fail loading from type: module package.'); -} catch (e) { - assert.strictEqual(e.name, 'Error'); - assert.strictEqual(e.code, 'ERR_REQUIRE_ESM'); - assert(e.toString().match(/require\(\) of ES Module/g)); - assert(e.message.match(/require\(\) of ES Module/g)); -} +describe('ESM type field errors', { concurrency: true }, () => { + it('.cjs file', () => { + expect('', cjsFile, '.cjs file'); + }); + + it('.mjs file', () => { + expect('', mjsFile, '.mjs file'); + }); + + it('package.json with "type": "module"', () => { + expect('', packageTypeModuleMain, 'package-type-module'); + }); + + it('package.json with "type": "commonjs"', () => { + expect('', packageTypeCommonJsMain, 'package-type-commonjs'); + }); + + it('package.json with no "type" field', () => { + expect('', packageWithoutTypeMain, 'package-without-type'); + }); + + it('--input-type=module disallowed for files', () => { + expect( + '--input-type=module', + packageTypeModuleMain, + 'ERR_INPUT_TYPE_NOT_ALLOWED', + true, + ); + }); + + it('--input-type=module disallowed for directories', () => { + assert.throws(() => require('../fixtures/es-modules/package-type-module/index.js'), { + code: 'ERR_REQUIRE_ESM' + }); + }); +}); function expect(opt = '', inputFile, want, wantsError = false) { const argv = [inputFile]; From 2e679ee0683cb3e31f83c9c5eceeee7301a9e6f6 Mon Sep 17 00:00:00 2001 From: Giovanni Date: Tue, 13 Aug 2024 16:01:08 +0200 Subject: [PATCH 7/8] esm: modernize old tests Co-Authored-By: Jacob Smith --- test/es-module/test-esm-type-field-errors.js | 56 +++++++++++++------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/test/es-module/test-esm-type-field-errors.js b/test/es-module/test-esm-type-field-errors.js index d9d264cc25aa25..ddde0d0f607df3 100644 --- a/test/es-module/test-esm-type-field-errors.js +++ b/test/es-module/test-esm-type-field-errors.js @@ -2,6 +2,7 @@ const common = require('../common'); const assert = require('assert'); const exec = require('child_process').execFile; +const { describe, it } = require('node:test'); const mjsFile = require.resolve('../fixtures/es-modules/mjs-file.mjs'); const cjsFile = require.resolve('../fixtures/es-modules/cjs-file.cjs'); @@ -13,25 +14,42 @@ const packageTypeModuleMain = require.resolve('../fixtures/es-modules/package-type-module/index.js'); // Check that running `node` without options works -expect('', mjsFile, '.mjs file'); -expect('', cjsFile, '.cjs file'); -expect('', packageTypeModuleMain, 'package-type-module'); -expect('', packageTypeCommonJsMain, 'package-type-commonjs'); -expect('', packageWithoutTypeMain, 'package-without-type'); - -// Check that --input-type isn't allowed for files -expect('--input-type=module', packageTypeModuleMain, - 'ERR_INPUT_TYPE_NOT_ALLOWED', true); - -try { - require('../fixtures/es-modules/package-type-module/index.js'); - assert.fail('Expected CJS to fail loading from type: module package.'); -} catch (e) { - assert.strictEqual(e.name, 'Error'); - assert.strictEqual(e.code, 'ERR_REQUIRE_ESM'); - assert(e.toString().match(/require\(\) of ES Module/g)); - assert(e.message.match(/require\(\) of ES Module/g)); -} +describe('ESM type field errors', { concurrency: true }, () => { + it('.cjs file', () => { + expect('', cjsFile, '.cjs file'); + }); + + it('.mjs file', () => { + expect('', mjsFile, '.mjs file'); + }); + + it('package.json with "type": "module"', () => { + expect('', packageTypeModuleMain, 'package-type-module'); + }); + + it('package.json with "type": "commonjs"', () => { + expect('', packageTypeCommonJsMain, 'package-type-commonjs'); + }); + + it('package.json with no "type" field', () => { + expect('', packageWithoutTypeMain, 'package-without-type'); + }); + + it('--input-type=module disallowed for files', () => { + expect( + '--input-type=module', + packageTypeModuleMain, + 'ERR_INPUT_TYPE_NOT_ALLOWED', + true, + ); + }); + + it('--input-type=module disallowed for directories', () => { + assert.throws(() => require('../fixtures/es-modules/package-type-module/index.js'), { + code: 'ERR_REQUIRE_ESM' + }); + }); +}); function expect(opt = '', inputFile, want, wantsError = false) { const argv = [inputFile]; From c90fdab3579edd1cedcc92fa6a98e84ad1badf46 Mon Sep 17 00:00:00 2001 From: Giovanni Date: Tue, 13 Aug 2024 16:01:08 +0200 Subject: [PATCH 8/8] esm: modernize old tests Co-Authored-By: Jacob Smith --- test/es-module/test-esm-type-field-errors.js | 56 +++++++++++++------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/test/es-module/test-esm-type-field-errors.js b/test/es-module/test-esm-type-field-errors.js index d9d264cc25aa25..ddde0d0f607df3 100644 --- a/test/es-module/test-esm-type-field-errors.js +++ b/test/es-module/test-esm-type-field-errors.js @@ -2,6 +2,7 @@ const common = require('../common'); const assert = require('assert'); const exec = require('child_process').execFile; +const { describe, it } = require('node:test'); const mjsFile = require.resolve('../fixtures/es-modules/mjs-file.mjs'); const cjsFile = require.resolve('../fixtures/es-modules/cjs-file.cjs'); @@ -13,25 +14,42 @@ const packageTypeModuleMain = require.resolve('../fixtures/es-modules/package-type-module/index.js'); // Check that running `node` without options works -expect('', mjsFile, '.mjs file'); -expect('', cjsFile, '.cjs file'); -expect('', packageTypeModuleMain, 'package-type-module'); -expect('', packageTypeCommonJsMain, 'package-type-commonjs'); -expect('', packageWithoutTypeMain, 'package-without-type'); - -// Check that --input-type isn't allowed for files -expect('--input-type=module', packageTypeModuleMain, - 'ERR_INPUT_TYPE_NOT_ALLOWED', true); - -try { - require('../fixtures/es-modules/package-type-module/index.js'); - assert.fail('Expected CJS to fail loading from type: module package.'); -} catch (e) { - assert.strictEqual(e.name, 'Error'); - assert.strictEqual(e.code, 'ERR_REQUIRE_ESM'); - assert(e.toString().match(/require\(\) of ES Module/g)); - assert(e.message.match(/require\(\) of ES Module/g)); -} +describe('ESM type field errors', { concurrency: true }, () => { + it('.cjs file', () => { + expect('', cjsFile, '.cjs file'); + }); + + it('.mjs file', () => { + expect('', mjsFile, '.mjs file'); + }); + + it('package.json with "type": "module"', () => { + expect('', packageTypeModuleMain, 'package-type-module'); + }); + + it('package.json with "type": "commonjs"', () => { + expect('', packageTypeCommonJsMain, 'package-type-commonjs'); + }); + + it('package.json with no "type" field', () => { + expect('', packageWithoutTypeMain, 'package-without-type'); + }); + + it('--input-type=module disallowed for files', () => { + expect( + '--input-type=module', + packageTypeModuleMain, + 'ERR_INPUT_TYPE_NOT_ALLOWED', + true, + ); + }); + + it('--input-type=module disallowed for directories', () => { + assert.throws(() => require('../fixtures/es-modules/package-type-module/index.js'), { + code: 'ERR_REQUIRE_ESM' + }); + }); +}); function expect(opt = '', inputFile, want, wantsError = false) { const argv = [inputFile];