From 1eefeac3ab2b96bfbc33720aa1860f35457c104e Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 12 Dec 2024 10:48:41 +0000 Subject: [PATCH] Get rid of all complexity? --- .../{build.mjs => build-cjs.mjs} | 2 +- .../{build.shimmed.mjs => build-esm.mjs} | 12 +--- .../node-profiling/index.mjs | 4 +- .../node-profiling/package.json | 4 +- packages/profiling-node/rollup.npm.config.mjs | 59 +------------------ packages/profiling-node/src/cpu_profiler.ts | 24 ++++---- 6 files changed, 17 insertions(+), 88 deletions(-) rename dev-packages/e2e-tests/test-applications/node-profiling/{build.mjs => build-cjs.mjs} (96%) rename dev-packages/e2e-tests/test-applications/node-profiling/{build.shimmed.mjs => build-esm.mjs} (68%) diff --git a/dev-packages/e2e-tests/test-applications/node-profiling/build.mjs b/dev-packages/e2e-tests/test-applications/node-profiling/build-cjs.mjs similarity index 96% rename from dev-packages/e2e-tests/test-applications/node-profiling/build.mjs rename to dev-packages/e2e-tests/test-applications/node-profiling/build-cjs.mjs index 55ec0b5fae52..3fadfaad42fe 100644 --- a/dev-packages/e2e-tests/test-applications/node-profiling/build.mjs +++ b/dev-packages/e2e-tests/test-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/dev-packages/e2e-tests/test-applications/node-profiling/build.shimmed.mjs b/dev-packages/e2e-tests/test-applications/node-profiling/build-esm.mjs similarity index 68% rename from dev-packages/e2e-tests/test-applications/node-profiling/build.shimmed.mjs rename to dev-packages/e2e-tests/test-applications/node-profiling/build-esm.mjs index c45e30539bc0..397c1b468091 100644 --- a/dev-packages/e2e-tests/test-applications/node-profiling/build.shimmed.mjs +++ b/dev-packages/e2e-tests/test-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 index 71608ef53260..bd178eb0f716 100644 --- 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 index 2029ac1370ed..0711c9b4baaa 100644 --- 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 index c0da1ddb3e24..968f4c3e8198 100644 --- a/packages/profiling-node/rollup.npm.config.mjs +++ b/packages/profiling-node/rollup.npm.config.mjs @@ -1,49 +1,7 @@ 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 }, @@ -51,18 +9,3 @@ const variants = makeNPMConfigVariants( }, }), ); - -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 index bbdf2c621844..abb8f76f45ed 100644 --- 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`); }