Skip to content

feat(node): Make ESM output valid Node ESM #10928

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Mar 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 0 additions & 33 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"include": ["src/**/*"],

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