Skip to content
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

esm: improve performance & tidy tests #43784

Merged
merged 15 commits into from
Jul 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/internal/modules/esm/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ class ESMLoader {
* A list of exports from user-defined loaders (as returned by
* ESMLoader.import()).
*/
async addCustomLoaders(
addCustomLoaders(
customLoaders = [],
) {
for (let i = 0; i < customLoaders.length; i++) {
Expand Down
33 changes: 32 additions & 1 deletion test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
const process = global.process; // Some tests tamper with the process global.

const assert = require('assert');
const { exec, execSync, spawnSync } = require('child_process');
const { exec, execSync, spawn, spawnSync } = require('child_process');
const fs = require('fs');
// Do not require 'os' until needed so that test-os-checked-function can
// monkey patch it. If 'os' is required here, that test will fail.
Expand Down Expand Up @@ -842,6 +842,36 @@ function requireNoPackageJSONAbove(dir = __dirname) {
}
}

function spawnPromisified(...args) {
let stderr = '';
let stdout = '';

const child = spawn(...args);
child.stderr.setEncoding('utf8');
child.stderr.on('data', (data) => { stderr += data; });
JakobJingleheimer marked this conversation as resolved.
Show resolved Hide resolved
child.stdout.setEncoding('utf8');
child.stdout.on('data', (data) => { stdout += data; });

return new Promise((resolve, reject) => {
child.on('close', (code, signal) => {
JakobJingleheimer marked this conversation as resolved.
Show resolved Hide resolved
resolve({
code,
signal,
stderr,
stdout,
});
});
child.on('error', (code, signal) => {
reject({
code,
signal,
stderr,
stdout,
});
});
});
}

const common = {
allowGlobals,
buildType,
Expand Down Expand Up @@ -891,6 +921,7 @@ const common = {
skipIfEslintMissing,
skipIfInspectorDisabled,
skipIfWorker,
spawnPromisified,

get enoughTestMem() {
return require('os').totalmem() > 0x70000000; /* 1.75 Gb */
Expand Down
8 changes: 6 additions & 2 deletions test/common/index.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const {
hasCrypto,
hasIPv6,
childShouldThrowAndAbort,
checkoutEOL,
createZeroFilledFile,
platformTimeout,
allowGlobals,
Expand All @@ -47,7 +48,8 @@ const {
getArrayBufferViews,
getBufferSources,
getTTYfd,
runWithInvalidFD
runWithInvalidFD,
spawnPromisified,
} = common;

export {
Expand All @@ -70,6 +72,7 @@ export {
hasCrypto,
hasIPv6,
childShouldThrowAndAbort,
checkoutEOL,
createZeroFilledFile,
platformTimeout,
allowGlobals,
Expand All @@ -95,5 +98,6 @@ export {
getBufferSources,
getTTYfd,
runWithInvalidFD,
createRequire
createRequire,
spawnPromisified,
};
102 changes: 53 additions & 49 deletions test/es-module/test-cjs-esm-warn.js
Original file line number Diff line number Diff line change
@@ -1,64 +1,68 @@
'use strict';

const common = require('../common');
const fixtures = require('../common/fixtures');
const { spawn } = require('child_process');
const assert = require('assert');
const path = require('path');
const { spawnPromisified } = require('../common');
const fixtures = require('../common/fixtures.js');
const assert = require('node:assert');
const path = require('node:path');
const { execPath } = require('node:process');
const { describe, it } = require('node:test');


const requiringCjsAsEsm = path.resolve(fixtures.path('/es-modules/cjs-esm.js'));
const requiringEsm = path.resolve(fixtures.path('/es-modules/cjs-esm-esm.js'));
const pjson = path.resolve(
fixtures.path('/es-modules/package-type-module/package.json')
);

{
const required = path.resolve(
fixtures.path('/es-modules/package-type-module/cjs.js')
);
const basename = 'cjs.js';
const child = spawn(process.execPath, [requiringCjsAsEsm]);
let stderr = '';
child.stderr.setEncoding('utf8');
child.stderr.on('data', (data) => {
stderr += data;
});
child.on('close', common.mustCall((code, signal) => {

describe('CJS ↔︎ ESM interop warnings', { concurrency: true }, () => {

it(async () => {
const required = path.resolve(
fixtures.path('/es-modules/package-type-module/cjs.js')
);
const basename = 'cjs.js';
const { code, signal, stderr } = await spawnPromisified(execPath, [requiringCjsAsEsm]);

assert.ok(
stderr.replaceAll('\r', '').includes(
`Error [ERR_REQUIRE_ESM]: require() of ES Module ${required} from ${requiringCjsAsEsm} not supported.\n`
)
);
assert.ok(
stderr.replaceAll('\r', '').includes(
`Instead rename ${basename} to end in .cjs, change the requiring ` +
'code to use dynamic import() which is available in all CommonJS ' +
`modules, or change "type": "module" to "type": "commonjs" in ${pjson} to ` +
'treat all .js files as CommonJS (using .mjs for all ES modules ' +
'instead).\n'
)
);

assert.strictEqual(code, 1);
assert.strictEqual(signal, null);
});

assert.ok(stderr.replaceAll('\r', '').includes(
`Error [ERR_REQUIRE_ESM]: require() of ES Module ${required} from ${
requiringCjsAsEsm} not supported.\n`));
assert.ok(stderr.replaceAll('\r', '').includes(
`Instead rename ${basename} to end in .cjs, change the requiring ` +
'code to use dynamic import() which is available in all CommonJS ' +
`modules, or change "type": "module" to "type": "commonjs" in ${pjson} to ` +
'treat all .js files as CommonJS (using .mjs for all ES modules ' +
'instead).\n'));
}));
}
it(async () => {
const required = path.resolve(
fixtures.path('/es-modules/package-type-module/esm.js')
);
const basename = 'esm.js';
const { code, signal, stderr } = await spawnPromisified(execPath, [requiringEsm]);

assert.ok(
stderr.replace(/\r/g, '').includes(
`Error [ERR_REQUIRE_ESM]: require() of ES Module ${required} from ${requiringEsm} not supported.\n`
)
);
assert.ok(
stderr.replace(/\r/g, '').includes(
`Instead change the require of ${basename} in ${requiringEsm} to` +
' a dynamic import() which is available in all CommonJS modules.\n'
)
);

{
const required = path.resolve(
fixtures.path('/es-modules/package-type-module/esm.js')
);
const basename = 'esm.js';
const child = spawn(process.execPath, [requiringEsm]);
let stderr = '';
child.stderr.setEncoding('utf8');
child.stderr.on('data', (data) => {
stderr += data;
});
child.on('close', common.mustCall((code, signal) => {
assert.strictEqual(code, 1);
assert.strictEqual(signal, null);

assert.ok(stderr.replace(/\r/g, '').includes(
`Error [ERR_REQUIRE_ESM]: require() of ES Module ${required} from ${
requiringEsm} not supported.\n`));
assert.ok(stderr.replace(/\r/g, '').includes(
`Instead change the require of ${basename} in ${requiringEsm} to` +
' a dynamic import() which is available in all CommonJS modules.\n'));
}));
}
});
});
29 changes: 14 additions & 15 deletions test/es-module/test-esm-cjs-builtins.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,20 @@
'use strict';

const common = require('../common');
const fixtures = require('../common/fixtures');
const { spawn } = require('child_process');
const assert = require('assert');
const { spawnPromisified } = require('../common');
const fixtures = require('../common/fixtures.js');
const assert = require('node:assert');
const { execPath } = require('node:process');
const { describe, it } = require('node:test');


const entry = fixtures.path('/es-modules/builtin-imports-case.mjs');

const child = spawn(process.execPath, [entry]);
child.stderr.setEncoding('utf8');
let stdout = '';
child.stdout.setEncoding('utf8');
child.stdout.on('data', (data) => {
stdout += data;
describe('ESM: importing builtins & CJS', () => {
it('should work', async () => {
const { code, signal, stdout } = await spawnPromisified(execPath, [entry]);

assert.strictEqual(code, 0);
assert.strictEqual(signal, null);
assert.strictEqual(stdout, 'ok\n');
});
});
child.on('close', common.mustCall((code, signal) => {
assert.strictEqual(code, 0);
assert.strictEqual(signal, null);
assert.strictEqual(stdout, 'ok\n');
}));
52 changes: 23 additions & 29 deletions test/es-module/test-esm-cjs-exports.js
Original file line number Diff line number Diff line change
@@ -1,35 +1,29 @@
'use strict';

const common = require('../common');
const fixtures = require('../common/fixtures');
const { spawn } = require('child_process');
const assert = require('assert');
const { spawnPromisified } = require('../common');
const fixtures = require('../common/fixtures.js');
const assert = require('node:assert');
const { execPath } = require('node:process');
const { describe, it } = require('node:test');

const entry = fixtures.path('/es-modules/cjs-exports.mjs');

let child = spawn(process.execPath, [entry]);
child.stderr.setEncoding('utf8');
let stdout = '';
child.stdout.setEncoding('utf8');
child.stdout.on('data', (data) => {
stdout += data;
});
child.on('close', common.mustCall((code, signal) => {
assert.strictEqual(code, 0);
assert.strictEqual(signal, null);
assert.strictEqual(stdout, 'ok\n');
}));
describe('ESM: importing CJS', { concurrency: true }, () => {
it('should support valid CJS exports', async () => {
const validEntry = fixtures.path('/es-modules/cjs-exports.mjs');
const { code, signal, stdout } = await spawnPromisified(execPath, [validEntry]);

assert.strictEqual(code, 0);
assert.strictEqual(signal, null);
assert.strictEqual(stdout, 'ok\n');
});

it('should eror on invalid CJS exports', async () => {
const invalidEntry = fixtures.path('/es-modules/cjs-exports-invalid.mjs');
const { code, signal, stderr } = await spawnPromisified(execPath, [invalidEntry]);

const entryInvalid = fixtures.path('/es-modules/cjs-exports-invalid.mjs');
child = spawn(process.execPath, [entryInvalid]);
let stderr = '';
child.stderr.setEncoding('utf8');
child.stderr.on('data', (data) => {
stderr += data;
assert.strictEqual(code, 1);
assert.strictEqual(signal, null);
assert.ok(stderr.includes('Warning: To load an ES module'));
assert.ok(stderr.includes('Unexpected token \'export\''));
});
});
child.on('close', common.mustCall((code, signal) => {
assert.strictEqual(code, 1);
assert.strictEqual(signal, null);
assert.ok(stderr.includes('Warning: To load an ES module'));
assert.ok(stderr.includes('Unexpected token \'export\''));
}));
Loading