getsentry / getsentry/sentry-javascript

Consolidate duplicated low-level helpers

オープン
#22,003 コメント 1 件 リアクション 2 件 担当者 0 名 GitHub で見る
javascript Task
主要言語
TypeScript
スター
8.7k
フォーク
1.8k
平均マージ
1日 17時間
マージ済み PR(30日)
515

説明

We re-implement the same small helpers (type guards, coercions, safe serializers) across many packages, often looser or subtly different from what already lives in other packages. This is a maintainability and correctness cleanup, not a bundle-size one as I think it wouldn't amount to much.

This came up after noticing we keep getting those `isRecord` utils, probably from agents liking it too much. Still it creates a bit of a situation where we have competing or duplicate utils that only differ in name or slight behavior mismatch.

I did a scan over what could be consolidated to our now three main util sources: `server-utils`, `browser-utils` and `core` depending on where the utils are duplicated and which runtimes they are relevant to.

### Stuff to consolidate in `core`

- `isRecord` (server-utils/src/vercel-ai/util.ts:19), `isObject` (browser/src/integrations/graphqlClient.ts:218), `isObject` (node-core/src/integrations/winston.ts:133). All identical `typeof === 'object' && x !== null`, and all intentionally accept arrays unlike core's existing `isPlainObject`.
- `arrayify` / `toArray` (bundler-plugins/src/core/utils.ts:17, plus aws vendored copies). Identical `Array.isArray(x) ? x : [x]`. Leave the vendored aws copies.
- `uniq` (inline): tanstackstart-react (routePatterns.ts:71, autoInstrumentMiddleware.ts:182), aws-serverless (MessageAttributes.ts:76), server-utils (orchestrion/config/index.ts:29) use `[...new Set()]`; vue (tracing.ts:65) uses `filter`+`indexOf` (O(n^2)). Same output, vue variant is the odd one.
- Nullish guards `isEmptyValue`: nitro (captureStorageEvents.ts:102) + nuxt (instrumentStorage.ts:215). Byte-identical `v === null || v === undefined`.
- `safeStringify` (server-utils/src/vercel-ai/util.ts:24) vs core-internal `getJsonString` (tracing/ai/utils.ts:198, NOT exported from the barrel). Identical except `safeStringify` adds a try/catch and returns `'[unserializable]'`, while `getJsonString` throws on circular refs.
- `addOriginToSpan`: cloudflare (utils/addOriginToSpan.ts:6), node-core (utils/addOriginToSpan.ts:6), opentelemetry (utils/addOriginToSpan.ts:6). Byte-identical including imports. cloudflare only depends on core (not opentelemetry), so core is the shared home. Sets `SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN`, already core-exported.
- `lowQualityTransactionsFilter`: nuxt (server/sdk.ts:58) + solidstart (server/utils.ts:25) + react-router (integration variant) (+ hono patterns). Shared scaffolding (transaction-type guard, `Object.assign(fn, { id })`, log-then-return-null), but the match predicates are genuinely per-framework. Share a `makeTransactionFilter(id, predicate)`; keep predicates local.
- `parseEventHintOrCaptureContext`: node-core (utils/prepareEvent.ts:17) duplicates core's exported copy.
- `isStaticAssetRequest`: node-core (integrations/http/httpServerSpansIntegration.ts:292) duplicates core's exported copy.
- `classifyResponseStreaming`: deno `packages/deno/src/utils/streaming.ts` and cloudflare `packages/cloudflare/src/utils/streaming.ts`. Bun needs it too.

### Consolidate into @sentry/browser-utils

- `isElement`: already re-homed here (browser-utils/src/is.ts:6) and adds a try/catch core lacks. Core's copy is deprecated.
- `htmlTreeAsString`: browser-utils (htmlTreeAsString.ts:55) vs core (utils/browser.ts:20, exported). Divergent, should drop core's.

### Consolidate into @sentry/server-utils

- `isAsyncIterable`: server-utils tracing-channel/anthropic.ts:138 + openai.ts:116. Byte-identical intra-package duplicate. Hoist into one server-utils util (no API change).
- `getVercelEnv`: nextjs (common/getVercelEnv.ts:6) + vercel-edge (utils/vercel.ts:10). Byte-identical bodies (vercel-edge only adds a local `declare const process`).
- `flushSafelyWithTimeout`: nextjs (common/utils/responseEnd.ts:48) + nuxt (server/sdk.ts:123). Byte-identical (comment wording differs only). Core already has an internal `flushWithTimeout` (utils/flushIfServerless.ts:11) with the same shape - export it (or a `flushSafely`) and have both import it.
- `customRewriteFramesIntegration`: nextjs (server + edge - and these two already drifted) + sveltekit. Only the `defineIntegration` wrapper is shared; the iteratees are framework-specific. De-dup the nextjs server/edge pair first; sharing the thin wrapper is optional.
- `packages/vercel-edge/src/logs/exports.ts` is dead code. Imported nowhere, exported nowhere. Delete.

### Maybe in node-core?

- `markEventUnhandled`: aws-serverless (utils.ts:16) + google-cloud-serverless (utils.ts:46). Byte-identical. Both depend on node-core (not server-utils), so node-core is the home.

コントリビューションガイド

コントリビューションガイドを開く

調査の方向性

Start by comparing the duplicate implementations in the listed package files and choose one consolidation target, such as the core helpers or a server-utils duplicate. Trace the relevant exports and imports before changing anything; done means the selected duplicate is removed, consumers use the shared helper, and the affected packages still pass their checks.

索引モデルが issue の本文から書いたものです。

評価

技術スタック
typescript
領域
tooling
issue の種類
リファクタリング
難易度
5/5
見積もり時間
1週間以上
活発さ
静か
明瞭さ
おおむね明確
初心者へのやさしさ
35/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。