@contrast/rewriter - npm Package Compare versions

Comparing version 1.8.1 to 1.8.2

lib/cache.js

@@ -222,8 +222,61 @@ /*
   /**
-   * Writes a rewritten file to the cache directory. Runs asynchronously so that
-   * disk I/O doesn't impact startup times, regardless of whether we're in a CJS
-   * or ESM environment.
+   * Synchronously writes a rewritten file to the cache directory. This is
+   * intended for use by require instrumentation because require is a sync
+   * operation.
+   *
+   * Incorrectly using the .write() method for require can result in the
+   * "unexpected end-of-file" error or rewriting the same file multiple
+   * times because it's required again before the write operation has
+   * completed.
+   *
    * @param {string} filename
    * @param {import('@swc/core').Output} result
+   * @returns {void}
    */
+  writeSync(filename, result) {
+    const filenameCached = this.getCachedFilename(filename);
+
+    try {
+      fs.mkdirSync(path.dirname(filenameCached), { recursive: true });
+
+      fs.writeFileSync(filenameCached, result.code, 'utf8');
+
+      if (result.map) {
+        fs.writeFileSync(`${filenameCached}.map`, result.map, 'utf8');
+      }
+
+      this.logger.trace(
+        {
+          filename,
+          filenameCached,
+        },
+        'Cache entry created.'
+      );
+    } catch (err) {
+      this.logger.warn(
+        {
+          err,
+          filename,
+          filenameCached,
+        },
+        'Unable to cache rewrite results.'
+      );
+    }
+  }
+
+  /**
+   * Asynchronously writes a rewritten file to the cache directory. This is
+   * intended for use by import instrumentation because import is an async
+   * operation.
+   *
+   * The caller should await this method to ensure that the cache is written
+   * before proceeding. If the caller doesn't wait, it's possible that the
+   * code will attempt to read a half-written file and get an "unexpected
+   * end-of-file" error or that the same file will be rewritten because it's
+   * required again before the file appears in the file system.
+   *
+   * @param {string} filename
+   * @param {import('@swc/core').Output} result
+   * @returns {Promise<void>}
+   */
   async write(filename, result) {
@@ -230,0 +283,0 @@ const filenameCached = this.getCachedFilename(filename);

lib/index.js

@@ -18,4 +18,7 @@ /*
 
+const Module = require('node:module');
+const fs = require('node:fs');
+const fsp = fs.promises;
+const { transfer } = require('multi-stage-sourcemap');
 const { transform, transformSync } = require('@swc/core');
-const Module = require('node:module');
 const { Cache } = require('./cache');
@@ -38,3 +41,2 @@
  * @prop {boolean=} wrap if true, wraps the content with a modified module wrapper IIFE
- * @prop {boolean=} trim if true, removes added characters from the end of the generated code
  */
@@ -73,29 +75,2 @@
 
-/**
- * Trims extraneous characters that may have been added by the rewriter.
- * Handles newline or semicolon insertion, removing the added characters if they
- * were not present in the original source content.
- * @param {string} content
- * @param {import('@swc/core').Output} result
- * @returns {import('@swc/core').Output}
- */
-const trim = (content, result) => {
-  let carriageReturn = 0;
-  // swc always adds a newline, so we only need to check the input
-  if (!content.endsWith('\n')) {
-    result.code = result.code.substring(0, result.code.length - 1);
-  } else if (content.endsWith('\r\n')) {
-    // if EOL is \r\n, then we need to account for that when we check the
-    // negative index of the last semicolon below
-    carriageReturn = 1;
-  }
-  const resultSemicolonIdx = result.code.lastIndexOf(';');
-  const contentSemicolonIdx = content.lastIndexOf(';');
-  if (contentSemicolonIdx === -1 || resultSemicolonIdx - result.code.length !== contentSemicolonIdx - content.length + carriageReturn) {
-    result.code = result.code.substring(0, resultSemicolonIdx) + result.code.substring(resultSemicolonIdx + 1, result.code.length);
-  }
-
-  return result;
-};
-
 class Rewriter {
@@ -145,5 +120,3 @@ /**
       },
-      // if we're trimming the output we're not rewriting an entire file, which
-      // means source maps are not relevant.
-      sourceMaps: !opts.trim && this.core.config.agent.node.source_maps.enable,
+      sourceMaps: this.core.config.agent.node.source_maps.enable,
     };
@@ -153,7 +126,8 @@ }
   /**
-   * Rewrites the provided source code string asynchronously to be consumed by
-   * ESM hooks.
+   * Rewrites the provided source code string asynchronously. this is used in an ESM
+   * context. CJS cannot use this because `require` is synchronous.
+   *
    * @param {string} content
    * @param {RewriteOpts=} opts
-   * @returns {Promise<import('@swc/core').Output>}
+   * @returns {Promise<import('@swc/core').Output>} with possibly modified source map.
    */
@@ -167,6 +141,6 @@ async rewrite(content, opts = {}) {
 
-    let result = await transform(content, this.rewriteConfig(opts));
+    const result = await transform(content, this.rewriteConfig(opts));
 
-    if (opts.trim) {
-      result = trim(content, result);
+    if (result.map) {
+      result.map = await this.ifSourceMapExistsChainIt(`${opts.filename}.map`, result.map);
     }
@@ -178,7 +152,9 @@
   /**
-   * Rewrites the provided source code string synchronously to be consumed by
-   * CJS hooks.
+   * Rewrites the provided source code string synchronously. this is used in a CJS
+   * context. while ESM could use this, performance is better when using the async
+   * version.
+   *
    * @param {string} content
    * @param {RewriteOpts=} opts
-   * @returns {import('@swc/core').Output}
+   * @returns {import('@swc/core').Output} with possibly modified source map.
    */
@@ -192,6 +168,6 @@ rewriteSync(content, opts = {}) {
 
-    let result = transformSync(content, this.rewriteConfig(opts));
+    const result = transformSync(content, this.rewriteConfig(opts));
 
-    if (opts.trim) {
-      result = trim(content, result);
+    if (result.map) {
+      result.map = this.ifSourceMapExistsChainItSync(`${opts.filename}.map`, result.map);
     }
@@ -222,2 +198,62 @@
   }
+
+
+  /**
+   * If there is a .map file in the same directory as the code being rewritten
+   * then chain the two maps together. This is an async function because there
+   * is no reason to wait for the source map to be finalized at startup. node-mono
+   * writes the map file asynchronously but performs two synchronous IO reads
+   * before calling transfer. This code performs a single async read before
+   * calling transfer.
+   *
+   * Question: should this log or just defer to the caller?
+   *
+   * @param {string} possibleMapPath the absolute path to a possibly pre-existing source map.
+   * @param {string} contrastMap the source map generated by the agent
+   * @returns {Promise<string>} promise to the final sourceMap object or, if an error,
+   * the input contrast source-map.
+   */
+  // @ts-ignore
+  async ifSourceMapExistsChainIt(possibleMapPath, contrastMap) {
+    try {
+      const data = await fsp.readFile(possibleMapPath, 'utf8');
+      const existingMap = JSON.parse(data);
+      contrastMap = transfer({ fromSourceMap: contrastMap, toSourceMap: existingMap });
+      this.logger.trace({ existingMap: possibleMapPath }, 'merged source-map');
+    } catch (err) {
+      // if the map file isn't found, it's not an error, otherwise log it.
+      // @ts-ignore
+      if (err.code !== 'ENOENT') {
+        this.logger.debug({ existingMap: possibleMapPath, err }, 'failed to read');
+      }
+    }
+
+    // return the merged map or the original contrast map
+    return contrastMap;
+  }
+
+  /**
+   * @param {string} possibleMapPath the absolute path to a possibly pre-existing source map.
+   * @param {string} contrastMap the source map generated by the agent
+   * @returns {string} the final sourceMap object or, if an error,
+   * the input contrast source-map.
+   */
+  // @ts-ignore
+  ifSourceMapExistsChainItSync(possibleMapPath, contrastMap) {
+    try {
+      const data = fs.readFileSync(possibleMapPath, 'utf8');
+      const existingMap = JSON.parse(data);
+      contrastMap = transfer({ fromSourceMap: contrastMap, toSourceMap: existingMap });
+      this.logger.trace({ existingMap: possibleMapPath }, 'merged source-map');
+    } catch (err) {
+      // if the map file isn't found, it's not an error, otherwise log it.
+      // @ts-ignore
+      if (err.code !== 'ENOENT') {
+        this.logger.debug({ existingMap: possibleMapPath, err }, 'failed to read');
+      }
+    }
+
+    // return the merged map or the original contrast map
+    return contrastMap;
+  }
 }
@@ -224,0 +260,0 @@

package.json

 {
   "name": "@contrast/rewriter",
-  "version": "1.8.1",
+  "version": "1.8.2",
   "description": "A transpilation tool mainly used for instrumentation",
@@ -19,3 +19,3 @@ "license": "SEE LICENSE IN LICENSE",
     "@contrast/agent-swc-plugin-unwrite": "1.5.0",
-    "@contrast/common": "1.21.1",
+    "@contrast/common": "1.21.2",
     "@contrast/synchronous-source-maps": "^1.1.3",
@@ -22,0 +22,0 @@ "@swc/core": "1.3.39",

New alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Fixed alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Filesystem access

Supply chain risk

Accesses the file system, and could potentially read sensitive data.

Found 1 instance in 1 package

Improved metrics

Total package byte prevSize

18762

increased by8.1%

Lines of code

514

increased by8.67%

Number of low supply chain risk alerts

1

decreased by-50%

Worsened metrics

Number of package files

5

decreased by-16.67%

Dependency changes

• + Added@contrast/common@1.21.2(transitive)

• - Removed@contrast/common@1.21.1(transitive)

• Updated@contrast/common@1.21.2