Skip to content

Commit

Permalink
ref(profiling): Conditionally shim cjs globals (#13267)
Browse files Browse the repository at this point in the history
The shims should only be applied if the globals are not present, else it
results in double decl and a runtime error. The profiling SDK should
gracefully handle env where the shims are already provided.

I couldn't find a way to modify the shim as it is hardcoded in the
plugin we are using so I went with the replace plugin approach and a
placeholder value #poormansmacros.
  • Loading branch information
JonasBa authored Sep 10, 2024
1 parent 703b5d4 commit 7fa366f
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 7 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1168,7 +1168,7 @@ jobs:
- name: Set up Node
uses: actions/setup-node@v4
with:
node-version-file: 'dev-packages/e2e-tests/package.json'
node-version: 22
- name: Restore caches
uses: ./.github/actions/restore-cache
with:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Because bundlers can now predetermine a static set of binaries we need to ensure those binaries
// actually exists, else we risk a compile time error when bundling the package. This could happen
// if we added a new binary in cpu_profiler.ts, but forgot to prebuild binaries for it. Because CI
// only runs integration and unit tests, this change would be missed and could end up in a release.
// Therefor, once all binaries are precompiled in CI and tests pass, run esbuild with bundle:true
// which will copy all binaries to the outfile folder and throw if any of them are missing.
import esbuild from 'esbuild';

console.log('Running build using esbuild version', esbuild.version);

esbuild.buildSync({
platform: 'node',
entryPoints: ['./index.ts'],
outfile: './dist/index.shimmed.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);
`,
},
});
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
"private": true,
"scripts": {
"typecheck": "tsc --noEmit",
"build": "node build.mjs",
"test": "npm run build && node dist/index.js",
"clean": "npx rimraf node_modules",
"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",
"clean": "npx rimraf node_modules dist",
"test:build": "npm run typecheck && npm run build",
"test:assert": "npm run test"
},
Expand Down
43 changes: 40 additions & 3 deletions packages/profiling-node/rollup.npm.config.mjs
Original file line number Diff line number Diff line change
@@ -1,12 +1,49 @@
import commonjs from '@rollup/plugin-commonjs';
import esmshim from '@rollup/plugin-esm-shim';
import { makeBaseNPMConfig, makeNPMConfigVariants } from '@sentry-internal/rollup-utils';

export default makeNPMConfigVariants(
export const ESMShim = `
import cjsUrl from 'node:url';
import cjsPath from 'node:path';
import cjsModule from 'node:module';
if(typeof __filename === 'undefined'){
globalThis.__filename = cjsUrl.fileURLToPath(import.meta.url);
}
if(typeof __dirname === 'undefined'){
globalThis.__dirname = cjsPath.dirname(__filename);
}
if(typeof require === 'undefined'){
globalThis.require = cjsModule.createRequire(import.meta.url);
}
`;

function makeESMShimPlugin(shim) {
return {
transform(code) {
const SHIM_REGEXP = /\/\/ #START_SENTRY_ESM_SHIM[\s\S]*?\/\/ #END_SENTRY_ESM_SHIM/;
return code.replace(SHIM_REGEXP, shim);
},
};
}

const variants = makeNPMConfigVariants(
makeBaseNPMConfig({
packageSpecificConfig: {
output: { dir: 'lib', preserveModules: false },
plugins: [commonjs(), esmshim()],
plugins: [commonjs()],
},
}),
);

for (const variant of variants) {
if (variant.output.format === 'esm') {
variant.plugins.push(makeESMShimPlugin(ESMShim));
} else {
// Remove the ESM shim comment
variant.plugins.push(makeESMShimPlugin(''));
}
}

export default variants;
6 changes: 6 additions & 0 deletions packages/profiling-node/src/cpu_profiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ import type {
} from './types';
import type { ProfileFormat } from './types';

// #START_SENTRY_ESM_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_SHIM

const stdlib = familySync();
const platform = process.env['BUILD_PLATFORM'] || _platform();
const arch = process.env['BUILD_ARCH'] || _arch();
Expand Down

0 comments on commit 7fa366f

Please sign in to comment.