Get rid of all complexity?

getsentry · Dec 12, 2024 · 1eefeac · 1eefeac
1 parent fd618e9
commit 1eefeac
Show file tree

Hide file tree

Showing 6 changed files with 17 additions and 88 deletions.
diff --git a/...est-applications/node-profiling/build.mjs → ...applications/node-profiling/build-cjs.mjs b/...est-applications/node-profiling/build.mjs → ...applications/node-profiling/build-cjs.mjs
@@ -11,7 +11,7 @@ console.log('Running build using esbuild version', esbuild.version);
 esbuild.buildSync({
   platform: 'node',
   entryPoints: ['./index.ts'],
-  outdir: './dist',
+  outfile: './dist/cjs/index.js',
   target: 'esnext',
   format: 'cjs',
   bundle: true,

diff --git a/...ications/node-profiling/build.shimmed.mjs → ...applications/node-profiling/build-esm.mjs b/...ications/node-profiling/build.shimmed.mjs → ...applications/node-profiling/build-esm.mjs
@@ -11,19 +11,9 @@ console.log('Running build using esbuild version', esbuild.version);
 esbuild.buildSync({
   platform: 'node',
   entryPoints: ['./index.ts'],
-  outfile: './dist/index.shimmed.mjs',
+  outfile: './dist/esm/index.mjs',
   target: 'esnext',
   format: 'esm',
   bundle: true,
   loader: { '.node': 'copy' },
-  banner: {
-    js: `
-        import { dirname } from 'node:path';
-        import { fileURLToPath } from 'node:url';
-        import { createRequire } from 'node:module';
-        const require = createRequire(import.meta.url);
-        const __filename = fileURLToPath(import.meta.url);
-        const __dirname = dirname(__filename);
-        `,
-  },
 });
diff --git a/dev-packages/e2e-tests/test-applications/node-profiling/index.mjs b/dev-packages/e2e-tests/test-applications/node-profiling/index.mjs
@@ -8,7 +8,7 @@ console.log('🧵 Starting ESM test');
 
 const wait = ms => new Promise(resolve => setTimeout(resolve, ms));
 
-function assertUnpatechedRequire() {
+function assertUnpatchedRequire() {
   if (typeof require !== 'undefined') {
     // Test that globalThis.require is not defined by any side effects of the profiling
     // https://github.com/getsentry/sentry-javascript/issues/13662
@@ -30,5 +30,5 @@ Sentry.startSpan({ name: 'Precompile test' }, async () => {
   await wait(500);
 });
 
-assertUnpatechedRequire();
+assertUnpatchedRequire();
 console.log('✅ Require is not patched');
diff --git a/dev-packages/e2e-tests/test-applications/node-profiling/package.json b/dev-packages/e2e-tests/test-applications/node-profiling/package.json
@@ -4,8 +4,8 @@
   "private": true,
   "scripts": {
     "typecheck": "tsc --noEmit",
-    "build": "node build.mjs && node build.shimmed.mjs",
-    "test": "node dist/index.js && node --experimental-require-module dist/index.js && node dist/index.shimmed.mjs",
+    "build": "node build-cjs.mjs && node build-esm.mjs",
+    "test": "node dist/index.js && node --experimental-require-module dist/cjs/index.js && node dist/esm/index.mjs",
     "clean": "npx rimraf node_modules dist",
     "test:electron": "$(pnpm bin)/electron-rebuild && playwright test",
     "test:build": "pnpm run typecheck && pnpm run build",

diff --git a/packages/profiling-node/rollup.npm.config.mjs b/packages/profiling-node/rollup.npm.config.mjs
@@ -1,68 +1,11 @@
 import commonjs from '@rollup/plugin-commonjs';
 import { makeBaseNPMConfig, makeNPMConfigVariants } from '@sentry-internal/rollup-utils';
 
-export const ESMImportShim = `
-import {createRequire} from 'node:module';
-import {fileURLToPath} from 'node:url';
-import {dirname } from 'node:path';
-`;
-
-const ESMRequireShim = `
-const require = createRequire(import.meta.url);
-`;
-
-const ESMDirnameShim = `
-const filename = fileURLToPath(import.meta.url);
-const __dirname = dirname(__filename);
-`;
-
-function makeESMImportShimPlugin(shim) {
-  return {
-    transform(code) {
-      const SHIM_REGEXP = /\/\/ #START_SENTRY_ESM_IMPORT_SHIM[\s\S]*?\/\/ #END_SENTRY_ESM_IMPORT_SHIM/;
-      return code.replace(SHIM_REGEXP, shim);
-    },
-  };
-}
-
-function makeESMRequireShimPlugin(shim) {
-  return {
-    transform(code) {
-      const SHIM_REGEXP = /\/\/ #START_SENTRY_ESM_REQUIRE_SHIM[\s\S]*?\/\/ #END_SENTRY_ESM_REQUIRE_SHIM/;
-      return code.replace(SHIM_REGEXP, shim);
-    },
-  };
-}
-
-function makeESMDirnameShimPlugin(shim) {
-  return {
-    transform(code) {
-      const SHIM_REGEXP = /\/\/ #START_SENTRY_ESM_DIRNAME_SHIM[\s\S]*?\/\/ #END_SENTRY_ESM_DIRNAME_SHIM/;
-      return code.replace(SHIM_REGEXP, shim);
-    },
-  };
-}
-
-const variants = makeNPMConfigVariants(
+export default makeNPMConfigVariants(
   makeBaseNPMConfig({
     packageSpecificConfig: {
       output: { dir: 'lib', preserveModules: false },
       plugins: [commonjs()],
     },
   }),
 );
-
-for (const variant of variants) {
-  if (variant.output.format === 'esm') {
-    variant.plugins.push(makeESMImportShimPlugin(ESMImportShim));
-    variant.plugins.push(makeESMRequireShimPlugin(ESMRequireShim));
-    variant.plugins.push(makeESMDirnameShimPlugin(ESMDirnameShim));
-  } else {
-    // Remove the ESM shim comment
-    variant.plugins.push(makeESMImportShimPlugin(''));
-    variant.plugins.push(makeESMRequireShimPlugin(''));
-    variant.plugins.push(makeESMDirnameShimPlugin(''));
-  }
-}
-
-export default variants;
diff --git a/packages/profiling-node/src/cpu_profiler.ts b/packages/profiling-node/src/cpu_profiler.ts
@@ -15,11 +15,11 @@ import type {
 } from './types';
 import type { ProfileFormat } from './types';
 
-// #START_SENTRY_ESM_IMPORT_SHIM
-// When building for ESM, we shim require to use createRequire and __dirname.
-// We need to do this because .node extensions in esm are not supported.
-// The comment below this line exists as a placeholder for where to insert the shim.
-// #END_SENTRY_ESM_IMPORT_SHIM
+import { createRequire } from 'node:module';
+import { dirname } from 'node:path';
+import { fileURLToPath } from 'node:url';
+
+const require = createRequire(import.meta.url);
 
 const stdlib = familySync();
 const platform = process.env['BUILD_PLATFORM'] || _platform();
@@ -32,10 +32,6 @@ const identifier = [platform, arch, stdlib, abi].filter(c => c !== undefined &&
  */
 // eslint-disable-next-line complexity
 export function importCppBindingsModule(): PrivateV8CpuProfilerBindings {
-  // #START_SENTRY_ESM_REQUIRE_SHIM
-  // When building for ESM, we shim require to use createRequire because .node extensions in esm are not supported.
-  // #END_SENTRY_ESM_REQUIRE_SHIM
-
   // If a binary path is specified, use that.
   if (env['SENTRY_PROFILER_BINARY_PATH']) {
     const envPath = env['SENTRY_PROFILER_BINARY_PATH'];
@@ -163,11 +159,11 @@ export function importCppBindingsModule(): PrivateV8CpuProfilerBindings {
     }
   }
 
-  // #START_SENTRY_ESM_DIRNAME_SHIM
-  // const filename = fileURLToPath(import.meta.url);
-  // const __dirname = dirname(filename);
-  // #END_SENTRY_ESM_DIRNAME_SHIM
-  const built_from_source_path = resolve(__dirname, '..', `./sentry_cpu_profiler-${identifier}`);
+  const built_from_source_path = resolve(
+    dirname(fileURLToPath(import.meta.url)),
+    '..',
+    `sentry_cpu_profiler-${identifier}`,
+  );
   return require(`${built_from_source_path}.node`);
 }