Skip to content

Commit

Permalink
Add dual cjs / esm support to path test
Browse files Browse the repository at this point in the history
  • Loading branch information
timokoessler committed Oct 3, 2024
1 parent cc43622 commit fd9d818
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 12 deletions.
5 changes: 3 additions & 2 deletions library/agent/hooks/wrapDefaultOrNamed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@ import { createWrappedFunction, wrap } from "../../helpers/wrap";
export function wrapDefaultOrNamed(
module: any,
name: string | undefined,
wrapper: (original: Function) => Function
wrapper: (original: Function) => Function,
isESMImport = false
) {
if (typeof name === "undefined") {
return createWrappedFunction(module, wrapper);
}
return wrap(module, name, wrapper);
return wrap(module, name, wrapper, isESMImport);
}
3 changes: 2 additions & 1 deletion library/agent/hooks/wrapExport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@ export function wrapExport(

return returnVal;
};
}
},
pkgInfo.isESMImport
);
} catch (error) {
agent.onFailedToWrapMethod(pkgInfo.name, methodName);
Expand Down
9 changes: 7 additions & 2 deletions library/helpers/wrap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ type WrappedFunction<T> = T & {
export function wrap(
module: any,
name: string,
wrapper: (original: Function) => Function
wrapper: (original: Function) => Function,
isESMImport = false
) {
if (!module[name]) {
throw new Error(`no original function ${name} to wrap`);
Expand All @@ -20,7 +21,11 @@ export function wrap(
const original = module[name];
const wrapped = createWrappedFunction(original, wrapper);

defineProperty(module, name, wrapped);
if (!isESMImport) {
defineProperty(module, name, wrapped);
} else {
module[name] = wrapped;
}

Check warning on line 28 in library/helpers/wrap.ts

View check run for this annotation

Codecov / codecov/patch

library/helpers/wrap.ts#L27-L28

Added lines #L27 - L28 were not covered by tests

return wrapped;
}
Expand Down
8 changes: 5 additions & 3 deletions library/sinks/Path.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import * as t from "tap";
import t from "tap";
import { Agent } from "../agent/Agent";
import { ReportingAPIForTesting } from "../agent/api/ReportingAPIForTesting";
import { Context, runWithContext } from "../agent/Context";
import { LoggerNoop } from "../agent/logger/LoggerNoop";
import { Path } from "./Path";
import { isCJS } from "../helpers/isCJS";

const unsafeContext: Context = {
remoteAddress: "::1",
Expand Down Expand Up @@ -33,12 +34,13 @@ t.test("it works", async (t) => {
new LoggerNoop(),
new ReportingAPIForTesting(),
undefined,
undefined
undefined,
!isCJS()
);

agent.start([new Path()]);

const { join, resolve } = require("path");
const { join, resolve } = isCJS() ? require("path") : await import("path");

function safeCalls() {
t.same(join("test.txt"), "test.txt");
Expand Down
15 changes: 11 additions & 4 deletions library/sinks/Path.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,18 @@ export class Path implements Wrapper {
wrap(hooks: Hooks): void {
const functions = ["join", "resolve", "normalize"];

// Todo after merging main: check path/posix and path/win32

Check failure on line 39 in library/sinks/Path.ts

View workflow job for this annotation

GitHub Actions / build (18.x)

Unexpected 'todo' comment: 'Todo after merging main: check...'
hooks.addBuiltinModule("path").onRequire((exports, pkgInfo) => {
for (const func of functions) {
wrapExport(exports, func, pkgInfo, {
inspectArgs: (args) => this.inspectPath(args, func),
});
const wrapParts = pkgInfo.isESMImport
? [exports, exports.default]
: [exports.posix, exports.win32];

for (const toWrap of wrapParts) {
for (const func of functions) {
wrapExport(toWrap, func, pkgInfo, {
inspectArgs: (args) => this.inspectPath(args, func),
});
}
}
});
}
Expand Down

0 comments on commit fd9d818

Please sign in to comment.