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

commonjs-extension-resolution-loader: add experimental-specifier-resolution tests #11

Merged
merged 5 commits into from
Sep 25, 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
48 changes: 33 additions & 15 deletions commonjs-extension-resolution-loader/loader.js
Original file line number Diff line number Diff line change
@@ -1,27 +1,45 @@
import { existsSync } from 'fs';
import { createRequire } from 'module';
import { builtinModules } from 'node:module';
import { dirname } from 'path';
import { URL, fileURLToPath, pathToFileURL } from 'url';
import { cwd } from 'process';
import { fileURLToPath, pathToFileURL } from 'url';
import { promisify } from 'util';

const require = createRequire(import.meta.url);
const baseURL = pathToFileURL(process.cwd() + '/').href;
import resolveCallback from 'resolve/async.js';

export function resolve(specifier, context, defaultResolve) {
const resolveAsync = promisify(resolveCallback);

const baseURL = pathToFileURL(cwd() + '/').href;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TLDR: you might need to wrap the cwd() in a try/catch.

In internal/process/esm_loader.js, this cwd is wrapped in a try/catch. I don't remember why (I asked and Bradley answered, and I think I forgot document the reason in a comment).



export async function resolve(specifier, context, next) {
const { parentURL = baseURL } = context;

// `require.resolve` works with paths, not URLs, so convert to and from
if (specifier.startsWith('node:') || builtinModules.includes(specifier)) {
return next(specifier, context);
}

// `resolveAsync` works with paths, not URLs
if (specifier.startsWith('file://')) {
specifier = fileURLToPath(specifier);
}
const basePath = dirname(fileURLToPath(parentURL));
const resolvedPath = require.resolve(specifier, {paths: [basePath]});
const parentPath = fileURLToPath(parentURL);

if (existsSync(resolvedPath)) {
return {
url: pathToFileURL(resolvedPath).href
};
let url;
try {
const resolution = await resolveAsync(specifier, {
basedir: dirname(parentPath),
// For whatever reason, --experimental-specifier-resolution=node doesn't search for .mjs extensions
// but it does search for index.mjs files within directories
extensions: ['.js', '.json', '.node', '.mjs'],
Copy link
Member

@JakobJingleheimer JakobJingleheimer Sep 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: no need to re-construct this list every time resolve runs.

});
url = pathToFileURL(resolution).href;
} catch (error) {
if (error.code === 'MODULE_NOT_FOUND') {
// Match Node's error code
error.code = 'ERR_MODULE_NOT_FOUND';
}
throw error;
}

// Let Node.js handle all other specifiers, such as package names
return defaultResolve(specifier, context, defaultResolve);
return next(url, context);
}
118 changes: 118 additions & 0 deletions commonjs-extension-resolution-loader/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 6 additions & 3 deletions commonjs-extension-resolution-loader/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,11 @@
"main": "./loader.js",
"scripts": {
"start": "npm test",
"test": "node test.js"
"test": "node test/basic.js && node test/test-esm-specifiers.js && node test/test-esm-specifiers-symlink.js"
},
"author": "Geoffrey Booth <[email protected]>",
"license": "MIT"
"author": "Geoffrey Booth <[email protected]>",
"license": "MIT",
"dependencies": {
"resolve": "^1.22.1"
}
}
25 changes: 0 additions & 25 deletions commonjs-extension-resolution-loader/test.js

This file was deleted.

25 changes: 25 additions & 0 deletions commonjs-extension-resolution-loader/test/basic.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { match } from 'assert';
import { spawn } from 'child_process';
import { execPath } from 'process';


// Run this test yourself with debugging mode via:
// node --inspect-brk --loader ./loader.js ./fixtures/index.js

const child = spawn(execPath, [
'--loader',
'./loader.js',
'./test/basic-fixtures/index.js'
]);

let stdout = '';
child.stdout.setEncoding('utf8');
child.stdout.on('data', data => {
stdout += data;
});

child.on('close', (_code, _signal) => {
stdout = stdout.toString();
match(stdout, /hello from file\.js/);
match(stdout, /hello from folder\/index\.js/);
});
34 changes: 34 additions & 0 deletions commonjs-extension-resolution-loader/test/common.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// Ported from https://github.com/nodejs/node/blob/45f2258f74117e27ffced434a98ad6babc14f7fa/test/common/index.js

import { spawn } from 'node:child_process';


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

const child = spawn(...args);
child.stderr.setEncoding('utf8');
child.stderr.on('data', (data) => { stderr += data; });
child.stdout.setEncoding('utf8');
child.stdout.on('data', (data) => { stdout += data; });

return new Promise((resolve, reject) => {
child.on('close', (code, signal) => {
resolve({
code,
signal,
stderr,
stdout,
});
});
child.on('error', (code, signal) => {
reject({
code,
signal,
stderr,
stdout,
});
});
});
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import explicit from 'explicit-main';
import implicit from 'implicit-main';
import implicitModule from 'implicit-main-type-module';
import noMain from 'no-main-field';

function getImplicitCommonjs () {
return import('implicit-main-type-commonjs');
}

export {explicit, implicit, implicitModule, getImplicitCommonjs, noMain};
export default 'success';
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = 'a';
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const b = 'b';
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module.exports = {
one: 1,
two: 2,
three: 3
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// js file that is common.js
import a from './a.js';
// ESM with named export
import {b} from './b.mjs';
// import 'c.cjs';
import cjs from './c.cjs';
// proves cross boundary fun bits
import jsAsEsm from '../package-type-module/a.js';

// named export from core
import {strictEqual, deepStrictEqual} from 'assert';

strictEqual(a, jsAsEsm);
strictEqual(b, 'b');
deepStrictEqual(cjs, {
one: 1,
two: 2,
three: 3
});

export default 'commonjs';
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"type": "commonjs"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default 'a'
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const b = 'b';
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module.exports = {
one: 1,
two: 2,
three: 3
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// ESM with only default
import a from './a.js';
// ESM with named export
import {b} from './b.mjs';
// import 'c.cjs';
import cjs from './c.cjs';
// import across boundaries
import jsAsCjs from '../package-type-commonjs/a.js'

// named export from core
import {strictEqual, deepStrictEqual} from 'assert';

strictEqual(a, jsAsCjs);
strictEqual(b, 'b');
deepStrictEqual(cjs, {
one: 1,
two: 2,
three: 3
});

export default 'module';
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"type": "module"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{}
Loading