Skip to content

Commit

Permalink
Support dots in directory names when running with esm: true
Browse files Browse the repository at this point in the history
  • Loading branch information
thegedge committed Nov 23, 2024
1 parent 7c817cd commit f5e4235
Show file tree
Hide file tree
Showing 7 changed files with 81 additions and 29 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "wds",
"version": "0.21.0",
"version": "0.21.1",
"author": "Harry Brundage",
"license": "MIT",
"bin": {
Expand Down
25 changes: 22 additions & 3 deletions spec/Supervisor.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,17 @@ import { expect, test } from "vitest";
const dirname = fileURLToPath(new URL(".", import.meta.url));

const childExit = (child: ChildProcess) => {
return new Promise<void>((resolve) => {
return new Promise<void>((resolve, reject) => {
child.on("error", (err: Error) => {
reject(err);
});

child.on("exit", (code: number) => {
resolve();
expect(code).toEqual(0);
if (code == 0) {
resolve();
} else {
reject(new Error(`Child process exited with code ${code}`));
}
});
});
};
Expand Down Expand Up @@ -111,3 +118,15 @@ test("it doesn't have any stdin if wds is started with terminal commands", async

expect(output).toEqual("");
}, 10000);

test("it can load a commonjs module inside a directory that contains a dot when in esm mode", async () => {
const binPath = path.join(dirname, "../pkg/wds.bin.js");
const scriptPath = path.join(dirname, "fixtures/esm/github.com/wds/simple.ts");

const child = spawn("node", [binPath, scriptPath], {
stdio: ["inherit", "inherit", "inherit"],
env: process.env,
});

await childExit(child);
}, 10000);
1 change: 1 addition & 0 deletions spec/fixtures/esm/github.com/wds/simple.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log("success");
5 changes: 5 additions & 0 deletions spec/fixtures/esm/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"name": "wds-test-sources",
"version": "0.0.1",
"license": "MIT"
}
13 changes: 13 additions & 0 deletions spec/fixtures/esm/wds.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
module.exports = {
esm: true,
swc: {
jsc: {
parser: {
syntax: "typescript",
decorators: true,
dynamicImport: true,
},
target: "es2020",
},
},
};
26 changes: 13 additions & 13 deletions spec/fixtures/src/wds.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
module.exports = {
"swc": {
"jsc": {
"parser": {
"syntax": "typescript",
"decorators": true,
"dynamicImport": true
swc: {
jsc: {
parser: {
syntax: "typescript",
decorators: true,
dynamicImport: true,
},
"target": "es2020"
target: "es2020",
},
module: {
type: "commonjs",
strictMode: true,
lazy: true,
},
"module": {
"type": "commonjs",
"strictMode": true,
"lazy": true
}
},
}
};
38 changes: 26 additions & 12 deletions src/hooks/child-process-esm-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import fs from "node:fs/promises";
import type { LoadHook, ModuleFormat, ResolveHook } from "node:module";
import { builtinModules, createRequire } from "node:module";
import { dirname, extname, join, resolve as resolvePath } from "node:path";
import { dirname, join, resolve as resolvePath } from "node:path";
import { fileURLToPath, pathToFileURL } from "node:url";
import { ResolverFactory } from "oxc-resolver";
import { debugLog } from "../SyncWorker.cjs";
Expand Down Expand Up @@ -166,7 +166,7 @@ export const load: LoadHook = async function load(url, context, nextLoad) {
return await nextLoad(url, context);
}

const format: ModuleFormat = context.format ?? (await getPackageType(url)) ?? "commonjs";
const format: ModuleFormat = context.format ?? (await getPackageType(fileURLToPath(url))) ?? "commonjs";
if (format == "commonjs") {
// if the package is a commonjs package and we return the source contents explicitly, this loader will process the inner requires, but with a broken/different version of \`require\` internally.
// if we return a nullish source, node falls back to the old, mainline require chain, which has require.cache set properly and whatnot.
Expand Down Expand Up @@ -197,30 +197,44 @@ export const load: LoadHook = async function load(url, context, nextLoad) {
};
};

async function getPackageType(url: string): Promise<ModuleFormat | undefined> {
// `url` is only a file path during the first iteration when passed the resolved url from the load() hook
// an actual file path from load() will contain a file extension as it's required by the spec
// this simple truthy check for whether `url` contains a file extension will work for most projects but does not cover some edge-cases (such as extensionless files or a url ending in a trailing space) extensionless files or a url ending in a trailing space)
const isFilePath = !!extname(url);
async function getPackageType(path: string, isFilePath?: boolean): Promise<ModuleFormat | undefined> {
isFilePath ??= await fs
.readdir(path)
.then(() => true)
.catch((err) => {
if (err?.code !== "ENOTDIR") {
throw err;
}
return false;
});

// If it is a file path, get the directory it's in
const dir = isFilePath ? dirname(fileURLToPath(url)) : url;
const dir = isFilePath ? dirname(path) : path;
// Compose a file path to a package.json in the same directory,
// which may or may not exist
const packagePath = resolvePath(dir, "package.json");
debugLog?.("getPackageType", { url, packagePath });
debugLog?.("getPackageType", { path, packagePath });

// Try to read the possibly nonexistent package.json
const type = await fs
.readFile(packagePath, { encoding: "utf8" })
.then((filestring) => JSON.parse(filestring).type)
.then((filestring) => {
// As per node's docs, we use the nearest package.json to figure out the package type (see https://nodejs.org/api/packages.html#type)
// If it lacks a "type" key, we assume "commonjs". If we fail to parse, we also choose to assume "commonjs".
try {
return JSON.parse(filestring).type || "commonjs";
} catch (_err) {
return "commonjs";
}
})
.catch((err) => {
if (err?.code !== "ENOENT") console.error(err);
});
// If package.json existed and contained a `type` field with a value, voilà
// If package.json existed, we guarantee a type and return it.
if (type) return type;
// Otherwise, (if not at the root) continue checking the next directory up
// If at the root, stop and return false
if (dir.length > 1) return await getPackageType(resolvePath(dir, ".."));
if (dir.length > 1) return await getPackageType(resolvePath(dir, ".."), false);

return undefined;
}

0 comments on commit f5e4235

Please sign in to comment.