Skip to content

Commit

Permalink
feat: Make ESM output valid Node ESM (#10928)
Browse files Browse the repository at this point in the history
Some bundlers, especially older ones, don't support resolving `.mjs`
files out of the box. To retain compatibility with these bundlers
without requiring users to adjust their configurations, we need to keep
all our ESM output as `.js` files.

So node.js loads these as modules, we output a `package.json` into each
`./build/esm` output directory containing simply `{ "type": "module" }`.
This works because whenever node.js is asked to load a `.js` file, it
tries to work out whether this is CJS or ESM by searching up through
parent directories until it finds a `package.json` with the `type` set.

This PR:
- Adds a Rollup plugin that injects the `package.json` into the ESM
output
- Adds the package `exports` that @AbhiPrasad worked out for #10833
- Fixes an import issue with `next/router` which is CJS (at least in
v10)
- Fixes an import issue with `@prisma/instrumentation` which is CJS
- Ensures that CJS only integrations are not included in the
`@sentry/node` default integrations when running as ESM

This PR also makes some unrelated changes:
- Changes to the old Node SDKs to allow the tests to pass
- Removes the bundle size analysis CI job as it doesn't appears to be
compatible with the node ESM output
  • Loading branch information
timfish committed Mar 8, 2024
1 parent 8d0b779 commit 7f2e804
Show file tree
Hide file tree
Showing 44 changed files with 373 additions and 64 deletions.
33 changes: 0 additions & 33 deletions .github/workflows/build.yml
Expand Up @@ -279,39 +279,6 @@ jobs:
# `job_build` can't see `job_install_deps` and what it returned)
dependency_cache_key: ${{ needs.job_install_deps.outputs.dependency_cache_key }}

job_size_check:
name: Size Check
needs: [job_get_metadata, job_build]
timeout-minutes: 15
runs-on: ubuntu-20.04
if:
github.event_name == 'pull_request' || needs.job_get_metadata.outputs.is_develop == 'true' ||
needs.job_get_metadata.outputs.is_release == 'true'
steps:
- name: Check out current commit (${{ needs.job_get_metadata.outputs.commit_label }})
uses: actions/checkout@v4
with:
ref: ${{ env.HEAD_COMMIT }}
- name: Set up Node
uses: actions/setup-node@v4
with:
# The size limit action runs `yarn` and `yarn build` when this job is executed on
# use Node 14 for now.
node-version: '14'
- name: Restore caches
uses: ./.github/actions/restore-cache
env:
DEPENDENCY_CACHE_KEY: ${{ needs.job_build.outputs.dependency_cache_key }}
- name: Check bundle sizes
uses: getsentry/size-limit-action@runForBranch
with:
github_token: ${{ secrets.GITHUB_TOKEN }}
skip_step: build
main_branch: develop
# When on release branch, we want to always run
# Else, we fall back to the default handling of the action
run_for_branch: ${{ (needs.job_get_metadata.outputs.is_release == 'true' && 'true') || '' }}

job_lint:
name: Lint
# Even though the linter only checks source code, not built code, it needs the built code in order check that all
Expand Down
4 changes: 3 additions & 1 deletion dev-packages/rollup-utils/npmHelpers.mjs
Expand Up @@ -17,6 +17,7 @@ import {
makeSetSDKSourcePlugin,
makeSucrasePlugin,
} from './plugins/index.mjs';
import { makePackageNodeEsm } from './plugins/make-esm-plugin.mjs';
import { mergePlugins } from './utils.mjs';

const packageDotJSON = JSON.parse(fs.readFileSync(path.resolve(process.cwd(), './package.json'), { encoding: 'utf8' }));
Expand Down Expand Up @@ -104,6 +105,7 @@ export function makeBaseNPMConfig(options = {}) {
...builtinModules,
...Object.keys(packageDotJSON.dependencies || {}),
...Object.keys(packageDotJSON.peerDependencies || {}),
...Object.keys(packageDotJSON.optionalDependencies || {}),
],
};

Expand All @@ -120,7 +122,7 @@ export function makeBaseNPMConfig(options = {}) {
export function makeNPMConfigVariants(baseConfig) {
const variantSpecificConfigs = [
{ output: { format: 'cjs', dir: path.join(baseConfig.output.dir, 'cjs') } },
{ output: { format: 'esm', dir: path.join(baseConfig.output.dir, 'esm') } },
{ output: { format: 'esm', dir: path.join(baseConfig.output.dir, 'esm'), plugins: [makePackageNodeEsm()] } },
];

return variantSpecificConfigs.map(variant => deepMerge(baseConfig, variant));
Expand Down
16 changes: 16 additions & 0 deletions dev-packages/rollup-utils/plugins/make-esm-plugin.mjs
@@ -0,0 +1,16 @@
/**
* Outputs a package.json file with {type: module} in the root of the output directory so that Node
* treats .js files as ESM.
*/
export function makePackageNodeEsm() {
return {
name: 'make-package-node-esm',
generateBundle() {
this.emitFile({
type: 'asset',
fileName: 'package.json',
source: '{ "type": "module" }',
});
},
};
}
13 changes: 13 additions & 0 deletions packages/browser/package.json
Expand Up @@ -18,6 +18,19 @@
"main": "build/npm/cjs/index.js",
"module": "build/npm/esm/index.js",
"types": "build/npm/types/index.d.ts",
"exports": {
"./package.json": "./package.json",
".": {
"import": {
"types": "./build/npm/types/index.d.ts",
"default": "./build/npm/esm/index.js"
},
"require": {
"types": "./build/npm.types/index.d.ts",
"default": "./build/npm/cjs/index.js"
}
}
},
"typesVersions": {
"<4.9": {
"build/npm/types/index.d.ts": [
Expand Down
13 changes: 13 additions & 0 deletions packages/bun/package.json
Expand Up @@ -18,6 +18,19 @@
"main": "build/esm/index.js",
"module": "build/esm/index.js",
"types": "build/types/index.d.ts",
"exports": {
"./package.json": "./package.json",
".": {
"import": {
"types": "./build/types/index.d.ts",
"default": "./build/esm/index.js"
},
"require": {
"types": "./build/types/index.d.ts",
"default": "./build/cjs/index.js"
}
}
},
"typesVersions": {
"<4.9": {
"build/npm/types/index.d.ts": [
Expand Down
13 changes: 13 additions & 0 deletions packages/core/package.json
Expand Up @@ -18,6 +18,19 @@
"main": "build/cjs/index.js",
"module": "build/esm/index.js",
"types": "build/types/index.d.ts",
"exports": {
"./package.json": "./package.json",
".": {
"import": {
"types": "./build/types/index.d.ts",
"default": "./build/esm/index.js"
},
"require": {
"types": "./build/types/index.d.ts",
"default": "./build/cjs/index.js"
}
}
},
"typesVersions": {
"<4.9": {
"build/types/index.d.ts": [
Expand Down
9 changes: 9 additions & 0 deletions packages/deno/package.json
Expand Up @@ -8,6 +8,15 @@
"license": "MIT",
"module": "build/index.mjs",
"types": "build/index.d.ts",
"exports": {
"./package.json": "./package.json",
".": {
"import": {
"types": "./build/index.d.ts",
"default": "./build/index.mjs"
}
}
},
"publishConfig": {
"access": "public"
},
Expand Down
13 changes: 13 additions & 0 deletions packages/feedback/package.json
Expand Up @@ -18,6 +18,19 @@
"main": "build/npm/cjs/index.js",
"module": "build/npm/esm/index.js",
"types": "build/npm/types/index.d.ts",
"exports": {
"./package.json": "./package.json",
".": {
"import": {
"types": "./build/npm/types/index.d.ts",
"default": "./build/npm/esm/index.js"
},
"require": {
"types": "./build/npm/types/index.d.ts",
"default": "./build/npm/cjs/index.js"
}
}
},
"typesVersions": {
"<4.9": {
"build/npm/types/index.d.ts": [
Expand Down
13 changes: 13 additions & 0 deletions packages/gatsby/package.json
Expand Up @@ -26,6 +26,19 @@
"main": "build/cjs/index.js",
"module": "build/esm/index.js",
"types": "build/types/index.d.ts",
"exports": {
"./package.json": "./package.json",
".": {
"import": {
"types": "./build/types/index.d.ts",
"default": "./build/esm/index.js"
},
"require": {
"types": "./build/types/index.d.ts",
"default": "./build/cjs/index.js"
}
}
},
"typesVersions": {
"<4.9": {
"build/types/index.d.ts": [
Expand Down
13 changes: 13 additions & 0 deletions packages/integration-shims/package.json
Expand Up @@ -5,6 +5,19 @@
"main": "build/cjs/index.js",
"module": "build/esm/index.js",
"types": "build/types/index.d.ts",
"exports": {
"./package.json": "./package.json",
".": {
"import": {
"types": "./build/types/index.d.ts",
"default": "./build/esm/index.js"
},
"require": {
"types": "./build/types/index.d.ts",
"default": "./build/cjs/index.js"
}
}
},
"typesVersions": {
"<4.9": {
"build/types/index.d.ts": [
Expand Down
11 changes: 11 additions & 0 deletions packages/nextjs/package.json
Expand Up @@ -13,6 +13,17 @@
"module": "build/esm/index.server.js",
"browser": "build/esm/index.client.js",
"types": "build/types/index.types.d.ts",
"exports": {
"./package.json": "./package.json",
".": {
"browser": {
"import": "./build/esm/index.client.js",
"require": "./build/cjs/index.client.js"
},
"node": "./build/cjs/index.server.js",
"types": "./build/types/index.types.d.ts"
}
},
"typesVersions": {
"<4.9": {
"build/npm/types/index.d.ts": [
Expand Down
Expand Up @@ -14,7 +14,13 @@ import {
stripUrlQueryAndFragment,
} from '@sentry/utils';
import type { NEXT_DATA as NextData } from 'next/dist/next-server/lib/utils';
import { default as Router } from 'next/router';
import RouterImport from 'next/router';

// next/router v10 is CJS
//
// For ESM/CJS interoperability 'reasons', depending on how this file is loaded, Router might be on the default export
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any
const Router: typeof RouterImport = RouterImport.events ? RouterImport : (RouterImport as any).default;

import { DEBUG_BUILD } from '../../common/debug-build';

Expand Down
@@ -1,7 +1,7 @@
import { WINDOW } from '@sentry/react';
import { JSDOM } from 'jsdom';
import type { NEXT_DATA as NextData } from 'next/dist/next-server/lib/utils';
import { default as Router } from 'next/router';
import Router from 'next/router';

import { pagesRouterInstrumentation } from '../../src/client/routing/pagesRouterRoutingInstrumentation';

Expand Down
13 changes: 13 additions & 0 deletions packages/node-experimental/package.json
Expand Up @@ -18,6 +18,19 @@
"main": "build/cjs/index.js",
"module": "build/esm/index.js",
"types": "build/types/index.d.ts",
"exports": {
"./package.json": "./package.json",
".": {
"import": {
"types": "./build/types/index.d.ts",
"default": "./build/esm/index.js"
},
"require": {
"types": "./build/types/index.d.ts",
"default": "./build/cjs/index.js"
}
}
},
"typesVersions": {
"<4.9": {
"build/types/index.d.ts": [
Expand Down
26 changes: 13 additions & 13 deletions packages/node-experimental/src/integrations/node-fetch.ts
Expand Up @@ -27,30 +27,29 @@ const _nativeNodeFetchIntegration = ((options: NodeFetchOptions = {}) => {
const _breadcrumbs = typeof options.breadcrumbs === 'undefined' ? true : options.breadcrumbs;
const _ignoreOutgoingRequests = options.ignoreOutgoingRequests;

function getInstrumentation(): [Instrumentation] | void {
async function getInstrumentation(): Promise<[Instrumentation] | void> {
// Only add NodeFetch if Node >= 16, as previous versions do not support it
if (NODE_MAJOR < 16) {
return;
}

try {
// eslint-disable-next-line @typescript-eslint/no-var-requires
const { FetchInstrumentation } = require('opentelemetry-instrumentation-fetch-node');
const pkg = await import('opentelemetry-instrumentation-fetch-node');
return [
new FetchInstrumentation({
new pkg.FetchInstrumentation({
ignoreRequestHook: (request: { origin?: string }) => {
const url = request.origin;
return _ignoreOutgoingRequests && url && _ignoreOutgoingRequests(url);
},

onRequest: ({ span }: { span: Span }) => {
_updateSpan(span);

if (_breadcrumbs) {
_addRequestBreadcrumb(span);
}
},
}),
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any),
];
} catch (error) {
// Could not load instrumentation
Expand All @@ -60,13 +59,14 @@ const _nativeNodeFetchIntegration = ((options: NodeFetchOptions = {}) => {
return {
name: 'NodeFetch',
setupOnce() {
const instrumentations = getInstrumentation();

if (instrumentations) {
registerInstrumentations({
instrumentations,
});
}
// eslint-disable-next-line @typescript-eslint/no-floating-promises
getInstrumentation().then(instrumentations => {
if (instrumentations) {
registerInstrumentations({
instrumentations,
});
}
});
},
};
}) satisfies IntegrationFn;
Expand Down
5 changes: 3 additions & 2 deletions packages/node-experimental/src/integrations/tracing/prisma.ts
@@ -1,5 +1,6 @@
import { registerInstrumentations } from '@opentelemetry/instrumentation';
import { PrismaInstrumentation } from '@prisma/instrumentation';
// When importing CJS modules into an ESM module, we cannot import the named exports directly.
import * as prismaInstrumentation from '@prisma/instrumentation';
import { defineIntegration } from '@sentry/core';
import type { IntegrationFn } from '@sentry/types';

Expand All @@ -10,7 +11,7 @@ const _prismaIntegration = (() => {
registerInstrumentations({
instrumentations: [
// does not have a hook to adjust spans & add origin
new PrismaInstrumentation({}),
new prismaInstrumentation.PrismaInstrumentation({}),
],
});
},
Expand Down
7 changes: 5 additions & 2 deletions packages/node-experimental/src/sdk/init.ts
Expand Up @@ -39,6 +39,10 @@ import { defaultStackParser, getSentryRelease } from './api';
import { NodeClient } from './client';
import { initOtel } from './initOtel';

function getCjsOnlyIntegrations(isCjs = typeof require !== 'undefined'): Integration[] {
return isCjs ? [modulesIntegration()] : [];
}

/** Get the default integrations for the Node Experimental SDK. */
export function getDefaultIntegrations(options: Options): Integration[] {
// TODO
Expand All @@ -59,9 +63,8 @@ export function getDefaultIntegrations(options: Options): Integration[] {
contextLinesIntegration(),
localVariablesIntegration(),
nodeContextIntegration(),
modulesIntegration(),
httpIntegration(),
nativeNodeFetchIntegration(),
...getCjsOnlyIntegrations(),
...(hasTracingEnabled(options) ? getAutoPerformanceIntegrations() : []),
];
}
Expand Down
3 changes: 2 additions & 1 deletion packages/node-experimental/tsconfig.json
Expand Up @@ -4,6 +4,7 @@
"include": ["src/**/*"],

"compilerOptions": {
"lib": ["es2017"]
"lib": ["es2017"],
"module": "Node16"
}
}

0 comments on commit 7f2e804

Please sign in to comment.