fix: Updated module patch to only wrap _compile as it calls _resolveFileName (#13)

Author: bizob2828

.snapshots/7f8b6c257a075df56e54361c8433cb48/0.json

@@ -1 +1 @@
-"const { tracingChannel: tr_ch_apm_tracingChannel } = require(\"diagnostics_channel\");\nconst tr_ch_apm$unitTestCjs = tr_ch_apm_tracingChannel(\"orchestrion:pkg-1:unitTestCjs\");\nclass Foo {\n    constructor(){\n        this.name = 'foo';\n    }\n    doStuff() {\n        const __apm$original_args = arguments;\n        const __apm$traced = async ()=>{\n            const __apm$wrapped = async ()=>{\n                return 'doing stuff';\n            };\n            return __apm$wrapped.apply(null, __apm$original_args);\n        };\n        if (!tr_ch_apm$unitTestCjs.hasSubscribers) return __apm$traced();\n        return tr_ch_apm$unitTestCjs.tracePromise(__apm$traced, {\n            arguments,\n            self: this,\n            moduleVersion: \"1.0.0\"\n        });\n    }\n}\nmodule.exports = Foo;\n"
+"const { tracingChannel: tr_ch_apm_tracingChannel } = require(\"diagnostics_channel\");\nconst tr_ch_apm$unitTestCjs = tr_ch_apm_tracingChannel(\"orchestrion:pkg-1:unitTestCjs\");\nclass Foo {\n    constructor(){\n        this.name = 'foo';\n    }\n    doStuff() {\n        const __apm$original_args = arguments;\n        const __apm$traced = ()=>{\n            const __apm$wrapped = ()=>{\n                return 'doing stuff';\n            };\n            return __apm$wrapped.apply(null, __apm$original_args);\n        };\n        if (!tr_ch_apm$unitTestCjs.hasSubscribers) return __apm$traced();\n        return tr_ch_apm$unitTestCjs.tracePromise(__apm$traced, {\n            arguments,\n            self: this,\n            moduleVersion: \"1.0.0\"\n        });\n    }\n}\nmodule.exports = Foo;\n"

.snapshots/814238ad8cda0e2502409f2591339137/0.json

@@ -1 +1 @@
-"const { tracingChannel: tr_ch_apm_tracingChannel } = require(\"diagnostics_channel\");\nconst tr_ch_apm$unitTestCjs = tr_ch_apm_tracingChannel(\"orchestrion:pkg-1:unitTestCjs\");\nclass Foo {\n    constructor(){\n        this.name = 'foo';\n    }\n    doStuff() {\n        const __apm$original_args = arguments;\n        const __apm$traced = async ()=>{\n            const __apm$wrapped = async ()=>{\n                return 'doing stuff';\n            };\n            return __apm$wrapped.apply(null, __apm$original_args);\n        };\n        if (!tr_ch_apm$unitTestCjs.hasSubscribers) return __apm$traced();\n        return tr_ch_apm$unitTestCjs.tracePromise(__apm$traced, {\n            arguments,\n            self: this,\n            moduleVersion: \"1.0.0\"\n        });\n    }\n}\nmodule.exports = Foo;\n"
+"const { tracingChannel: tr_ch_apm_tracingChannel } = require(\"diagnostics_channel\");\nconst tr_ch_apm$unitTestCjs = tr_ch_apm_tracingChannel(\"orchestrion:pkg-1:unitTestCjs\");\nclass Foo {\n    constructor(){\n        this.name = 'foo';\n    }\n    doStuff() {\n        const __apm$original_args = arguments;\n        const __apm$traced = ()=>{\n            const __apm$wrapped = ()=>{\n                return 'doing stuff';\n            };\n            return __apm$wrapped.apply(null, __apm$original_args);\n        };\n        if (!tr_ch_apm$unitTestCjs.hasSubscribers) return __apm$traced();\n        return tr_ch_apm$unitTestCjs.tracePromise(__apm$traced, {\n            arguments,\n            self: this,\n            moduleVersion: \"1.0.0\"\n        });\n    }\n}\nmodule.exports = Foo;\n"

.snapshots/e9d3709e54638f0a8fb975da4ec13452/0.json

@@ -1 +1 @@
-"import { tracingChannel as tr_ch_apm_tracingChannel } from \"diagnostics_channel\";\nconst tr_ch_apm$unitTestEsm = tr_ch_apm_tracingChannel(\"orchestrion:esm-pkg:unitTestEsm\");\nclass Foo {\n    constructor(){\n        this.name = 'foo';\n    }\n    doStuff() {\n        const __apm$original_args = arguments;\n        const __apm$traced = async ()=>{\n            const __apm$wrapped = async ()=>{\n                return 'doing stuff';\n            };\n            return __apm$wrapped.apply(null, __apm$original_args);\n        };\n        if (!tr_ch_apm$unitTestEsm.hasSubscribers) return __apm$traced();\n        return tr_ch_apm$unitTestEsm.tracePromise(__apm$traced, {\n            arguments,\n            self: this,\n            moduleVersion: \"1.0.0\"\n        });\n    }\n}\nexport default Foo;\n"
+"import { tracingChannel as tr_ch_apm_tracingChannel } from \"diagnostics_channel\";\nconst tr_ch_apm$unitTestEsm = tr_ch_apm_tracingChannel(\"orchestrion:esm-pkg:unitTestEsm\");\nclass Foo {\n    constructor(){\n        this.name = 'foo';\n    }\n    doStuff() {\n        const __apm$original_args = arguments;\n        const __apm$traced = ()=>{\n            const __apm$wrapped = ()=>{\n                return 'doing stuff';\n            };\n            return __apm$wrapped.apply(null, __apm$original_args);\n        };\n        if (!tr_ch_apm$unitTestEsm.hasSubscribers) return __apm$traced();\n        return tr_ch_apm$unitTestEsm.tracePromise(__apm$traced, {\n            arguments,\n            self: this,\n            moduleVersion: \"1.0.0\"\n        });\n    }\n}\nexport default Foo;\n"

.snapshots/f9e55bce8f9e3f3009403bfd870b4e79/0.json

@@ -1 +1 @@
-"class Foo {\n    constructor(){\n        this.name = 'foo';\n    }\n    doStuff() {\n        const __apm$original_args = arguments;\n        const __apm$traced = async ()=>{\n            const __apm$wrapped = async ()=>{\n                return 'doing stuff';\n            };\n            return __apm$wrapped.apply(null, __apm$original_args);\n        };\n        if (!tr_ch_apm$unitTest.hasSubscribers) return __apm$traced();\n        return tr_ch_apm$unitTest.tracePromise(__apm$traced, {\n            arguments,\n            self: this,\n            moduleVersion: \"1.0.0\"\n        });\n    }\n}"
+"class Foo {\n    constructor(){\n        this.name = 'foo';\n    }\n    doStuff() {\n        const __apm$original_args = arguments;\n        const __apm$traced = ()=>{\n            const __apm$wrapped = ()=>{\n                return 'doing stuff';\n            };\n            return __apm$wrapped.apply(null, __apm$original_args);\n        };\n        if (!tr_ch_apm$unitTest.hasSubscribers) return __apm$traced();\n        return tr_ch_apm$unitTest.tracePromise(__apm$traced, {\n            arguments,\n            self: this,\n            moduleVersion: \"1.0.0\"\n        });\n    }\n}"

index.js

@@ -9,45 +9,36 @@ class ModulePatch {
   constructor({ instrumentations = [] } = {}) {
     this.packages = new Set(instrumentations.map(i => i.module.name))
     this.instrumentator = create(instrumentations)
-    this.transformers = new Map()
-    this.resolve = Module._resolveFilename
     this.compile = Module.prototype._compile
   }
 
   /**
-   * Patches the Node.js module class methods that are responsible for resolving filePaths and compiling code.
+   * Patches the Node.js module class method that is responsible for compiling code.
    * If a module is found that has an instrumentator, it will transform the code before compiling it
    * with tracing channel methods.
    */
   patch() {
     const self = this
-    Module._resolveFilename = function wrappedResolveFileName() {
-      const resolvedName = self.resolve.apply(this, arguments)
-      const resolvedModule = parse(resolvedName)
+    Module.prototype._compile = function wrappedCompile(...args) {
+      const [content, filename] = args
+      const resolvedModule = parse(filename)
       if (resolvedModule && self.packages.has(resolvedModule.name)) {
+        debug('found resolved module, checking if there is a transformer %s', filename)
         const version = getPackageVersion(resolvedModule.basedir, resolvedModule.name)
         const transformer = self.instrumentator.getTransformer(resolvedModule.name, version, resolvedModule.path)
         if (transformer) {
-          self.transformers.set(resolvedName, transformer)
-        }
-      }
-      return resolvedName
-    }
-
-    Module.prototype._compile = function wrappedCompile(...args) {
-      const [content, filename] = args
-      if (self.transformers.has(filename)) {
-        const transformer = self.transformers.get(filename)
-        try {
-          const transformedCode = transformer.transform(content, 'unknown')
-          args[0] = transformedCode?.code
-          if (process.env.TRACING_DUMP) {
-            dump(args[0], filename)
+          debug('transforming file %s', filename)
+          try {
+            const transformedCode = transformer.transform(content, 'unknown')
+            args[0] = transformedCode?.code
+            if (process.env.TRACING_DUMP) {
+              dump(args[0], filename)
+            }
+          } catch (error) {
+            debug('Error transforming module %s: %o', filename, error)
+          } finally {
+            transformer.free()
           }
-        } catch (error) {
-          debug('Error transforming module %s: %o', filename, error)
-        } finally {
-          transformer.free()
         }
       }
 
@@ -56,12 +47,10 @@ class ModulePatch {
   }
 
   /**
-   * Clears all the transformers and restores the original Module methods that were wrapped.
+   * Restores the original Module.prototype._compile method
    * **Note**: This is intended to be used in testing only.
    */
   unpatch() {
-    this.transformers.clear()
-    Module._resolveFilename = this.resolve
     Module.prototype._compile = this.compile
   }
 }

package.json

@@ -10,7 +10,8 @@
   },
   "main": "index.js",
   "scripts": {
-    "test": "c8 borp 'test/**/*.test.{js,mjs}'"
+    "test": "c8 borp 'test/**/*.test.{js,mjs}'",
+    "test:update-snapshots": "SNAP_UPDATE=1 npm test"
   },
   "files": [
     "index.js",
@@ -27,4 +28,4 @@
     "borp": "^0.20.1",
     "c8": "^10.1.3"
   }
-}
+}

test/index.test.js

@@ -41,26 +41,7 @@ test('should init ModulePatch', (t) => {
   const { modulePatch } = t.ctx
   assert.ok(modulePatch instanceof ModulePatch)
   assert.ok(modulePatch.instrumentator)
-  assert.equal(modulePatch.resolve, Module._resolveFilename)
   assert.ok(modulePatch.compile, Module.prototype._compile)
-  assert.ok(modulePatch.transformers instanceof Map)
-})
-
-test('should set a transformer for a matched patch', (t) => {
-  const { modulePath, modulePatch } = t.ctx
-  modulePatch.patch()
-  Module._resolveFilename(modulePath, null, false)
-  assert.ok(modulePatch.transformers.has(modulePath))
-  modulePatch.unpatch()
-  assert.equal(modulePatch.transformers.size, 0)
-})
-
-test('should not set a transformer for an unmatched patch', (t) => {
-  const { modulePatch } = t.ctx
-  modulePatch.patch()
-  const modulePath = path.join(__dirname, './example-deps/lib/node_modules/pkg-2/index.js')
-  Module._resolveFilename(modulePath, null, false)
-  assert.equal(modulePatch.transformers.size, 0)
 })
 
 test('should rewrite code for a match transformer', async (t) => {