Repository navigation
test(bundler-tests): Guard server runtime entries against build-time tooling #25724
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,197 @@ | ||
| import { describe, expect, test } from 'vitest'; | ||
| import * as path from 'node:path'; | ||
| import * as fs from 'node:fs'; | ||
| import { fileURLToPath } from 'node:url'; | ||
|
|
||
| import { nodeFileTrace, resolve } from '@vercel/nft'; | ||
|
|
||
| /** | ||
| * Server runtime entries must not reach build-time tooling. When they do, | ||
| * the tooling ends up in server bundles and traced outputs (Next standalone, | ||
| * Nitro, workerd, ...), where it breaks in a different way for each | ||
| * consumer. `@vercel/nft` is the tracer that Next standalone and the Vercel | ||
| * adapters use, so it reports what those consumers would ship. | ||
| */ | ||
|
|
||
| const __dirname = path.dirname(fileURLToPath(import.meta.url)); | ||
| const ROOT_DIR = path.join(__dirname, '../../..'); | ||
| const FIXTURE_DIR = path.join(__dirname, '..', 'dist', 'runtime-entries'); | ||
|
|
||
| const BUILD_TIME_PATTERNS: RegExp[] = [ | ||
| /(^|\/)packages\/bundler-plugins\//, | ||
| /(^|\/)node_modules\/@sentry\/bundler-plugins(\/|$)/, | ||
| /(^|\/)packages\/server-utils\/build\/[^/]+\/orchestrion\/bundler\//, | ||
| /(^|\/)node_modules\/(vite|webpack|rollup|esbuild|oxc-parser)\//, | ||
| /(^|\/)node_modules\/@(babel|rollup|oxc-parser)\//, | ||
| /\.wasm$/, | ||
| ]; | ||
|
|
||
| /** | ||
| * These root entries also re-export config-time code (`sentryAstro`, | ||
| * `withSentry`). They must keep it until v12 for backwards compatibility. | ||
| */ | ||
| const KNOWN_VIOLATIONS = new Set(['@sentry/astro', '@sentry/solidstart']); | ||
|
|
||
| type Format = 'esm' | 'cjs'; | ||
|
|
||
| interface Case { | ||
| specifier: string; | ||
| conditions: string[]; | ||
| formats: Format[]; | ||
| } | ||
|
|
||
| const NODE = ['node']; | ||
| const WORKERD = ['workerd', 'worker']; | ||
| const BOTH: Format[] = ['esm', 'cjs']; | ||
| const ESM: Format[] = ['esm']; | ||
|
|
||
| const CASES: Case[] = [ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. m: I fear we'll soon forget about this list and it will go out of date, should we provide some guardrail to prevent this? I was thinking maybe we could switch this to auto-detect subpaths and then see if we find any subpaths that are neither in here or in some exemption list (i.e. force to classify). Alternatively a simple bugbot comment could also do the trick and would probably catch most cases. Wdyt? |
||
| { specifier: '@sentry/node', conditions: NODE, formats: BOTH }, | ||
| { specifier: '@sentry/node/import', conditions: NODE, formats: ESM }, | ||
| { specifier: '@sentry/bun', conditions: ['bun', 'node'], formats: BOTH }, | ||
| { specifier: '@sentry/deno', conditions: ['deno', 'node'], formats: ESM }, | ||
| { specifier: '@sentry/vercel-edge', conditions: ['edge-light'], formats: BOTH }, | ||
| { specifier: '@sentry/cloudflare', conditions: WORKERD, formats: BOTH }, | ||
| { specifier: '@sentry/cloudflare', conditions: [...WORKERD, 'development'], formats: BOTH }, | ||
|
|
||
| { specifier: '@sentry/nextjs', conditions: NODE, formats: BOTH }, | ||
| { specifier: '@sentry/nextjs', conditions: ['edge-light'], formats: BOTH }, | ||
| { specifier: '@sentry/nextjs', conditions: WORKERD, formats: BOTH }, | ||
| { specifier: '@sentry/nextjs/cloudflare', conditions: WORKERD, formats: BOTH }, | ||
| { specifier: '@sentry/nextjs/import', conditions: NODE, formats: ESM }, | ||
| { specifier: '@sentry/nextjs/async-storage-shim', conditions: NODE, formats: BOTH }, | ||
| { specifier: '@sentry/nextjs/orchestrion-runtime/index', conditions: NODE, formats: BOTH }, | ||
| { specifier: '@sentry/nextjs/orchestrion-runtime/register', conditions: NODE, formats: BOTH }, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. l: should |
||
|
|
||
| { specifier: '@sentry/nuxt', conditions: NODE, formats: BOTH }, | ||
| { specifier: '@sentry/nuxt/module/plugins', conditions: NODE, formats: ESM }, | ||
|
|
||
| { specifier: '@sentry/sveltekit', conditions: NODE, formats: BOTH }, | ||
| { specifier: '@sentry/sveltekit', conditions: WORKERD, formats: BOTH }, | ||
| { specifier: '@sentry/sveltekit', conditions: ['worker'], formats: BOTH }, | ||
|
|
||
| { specifier: '@sentry/astro', conditions: NODE, formats: BOTH }, | ||
| { specifier: '@sentry/astro/middleware', conditions: NODE, formats: BOTH }, | ||
| { specifier: '@sentry/astro/import', conditions: NODE, formats: ESM }, | ||
|
|
||
| { specifier: '@sentry/react-router', conditions: NODE, formats: BOTH }, | ||
| { specifier: '@sentry/react-router', conditions: WORKERD, formats: BOTH }, | ||
| { specifier: '@sentry/react-router/cloudflare', conditions: WORKERD, formats: BOTH }, | ||
|
|
||
| { specifier: '@sentry/remix', conditions: NODE, formats: BOTH }, | ||
| { specifier: '@sentry/remix', conditions: ['worker'], formats: BOTH }, | ||
| { specifier: '@sentry/remix/cloudflare', conditions: WORKERD, formats: BOTH }, | ||
| { specifier: '@sentry/remix/import', conditions: NODE, formats: ESM }, | ||
|
|
||
| { specifier: '@sentry/solidstart', conditions: NODE, formats: BOTH }, | ||
|
|
||
| { specifier: '@sentry/tanstackstart-react', conditions: NODE, formats: BOTH }, | ||
| { specifier: '@sentry/tanstackstart-react/import', conditions: NODE, formats: ESM }, | ||
| ]; | ||
|
|
||
| function writeFixture(specifier: string, conditions: string[], format: Format): string { | ||
| fs.mkdirSync(FIXTURE_DIR, { recursive: true }); | ||
| const name = `${specifier.replace(/[@/]/g, '_')}-${conditions.join('_')}.${format === 'esm' ? 'mjs' : 'cjs'}`; | ||
| const file = path.join(FIXTURE_DIR, name); | ||
| fs.writeFileSync(file, format === 'esm' ? `import '${specifier}';\n` : `require('${specifier}');\n`); | ||
| return file; | ||
| } | ||
|
|
||
| function isBuildTime(file: string): boolean { | ||
| return BUILD_TIME_PATTERNS.some(pattern => pattern.test(file)); | ||
| } | ||
|
|
||
| type Reasons = Awaited<ReturnType<typeof nodeFileTrace>>['reasons']; | ||
|
|
||
| // Most entries share `@sentry/node` and its dependencies, so sharing the | ||
| // file and analysis cache saves most of the work. | ||
| const traceCache = {}; | ||
|
|
||
| /** The shortest import chain from `entry` to `file`, both relative to `ROOT_DIR`. */ | ||
| function importChain(reasons: Reasons, entry: string, file: string): string[] { | ||
| const importedBy = new Map<string, string>([[file, file]]); | ||
| const queue = [file]; | ||
| while (queue.length) { | ||
| const current = queue.shift() as string; | ||
| if (current === entry) { | ||
| const chain = [current]; | ||
| for (let link = current; link !== file;) { | ||
| link = importedBy.get(link) as string; | ||
| chain.push(link); | ||
| } | ||
| return chain; | ||
| } | ||
| for (const parent of reasons.get(current)?.parents ?? []) { | ||
| if (!importedBy.has(parent)) { | ||
| importedBy.set(parent, current); | ||
| queue.push(parent); | ||
| } | ||
| } | ||
| } | ||
| return [file]; | ||
| } | ||
|
|
||
| /** | ||
| * Each place where a runtime module imports a build-time module, as the | ||
| * import chain from the entry. | ||
| */ | ||
| async function findBuildTimeImports(fixture: string, conditions: string[]): Promise<string[]> { | ||
| const imports: [importer: string, file: string][] = []; | ||
|
|
||
| const { fileList, reasons } = await nodeFileTrace([fixture], { | ||
| base: ROOT_DIR, | ||
| conditions, | ||
| cache: traceCache, | ||
| // A dynamic `fs` path in config code makes nft glob the whole repo. | ||
| // That is slow and finds user files, not package code. | ||
| analysis: { emitGlobs: false }, | ||
| // Stop at the first build-time module, instead of tracing all of Vite | ||
| // or Babel. `.wasm` files are assets, so they stay in the trace with | ||
| // their importer. | ||
| ignore: file => isBuildTime(file) && !file.endsWith('.wasm'), | ||
| async resolve(id, parent, job, cjsResolve) { | ||
| const resolved = await resolve(id, parent, job, cjsResolve); | ||
| for (const file of [resolved].flat()) { | ||
| if (isBuildTime(file)) { | ||
| imports.push([path.relative(ROOT_DIR, parent), path.relative(ROOT_DIR, file)]); | ||
| } | ||
| } | ||
| return resolved; | ||
| }, | ||
| }); | ||
|
|
||
| for (const file of fileList) { | ||
| if (file.endsWith('.wasm')) { | ||
| for (const parent of reasons.get(file)?.parents ?? []) { | ||
| imports.push([parent, file]); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const entry = path.relative(ROOT_DIR, fixture); | ||
| const violations = imports.map(([importer, file]) => | ||
| importChain(reasons, entry, importer).concat(file).join('\n -> '), | ||
| ); | ||
| return [...new Set(violations)].sort(); | ||
| } | ||
|
|
||
| describe('server runtime entries do not reach build-time tooling', () => { | ||
| for (const { specifier, conditions, formats } of CASES) { | ||
| for (const format of formats) { | ||
| const name = `${specifier} (${format}, conditions: ${conditions.join(', ')})`; | ||
|
|
||
| if (KNOWN_VIOLATIONS.has(specifier)) { | ||
| test(`${name} is a known violation`, async () => { | ||
| const violations = await findBuildTimeImports(writeFixture(specifier, conditions, format), conditions); | ||
| // Remove the entry from KNOWN_VIOLATIONS when this fails. | ||
| expect(violations).not.toEqual([]); | ||
| }); | ||
| } else { | ||
| test(name, async () => { | ||
| const violations = await findBuildTimeImports(writeFixture(specifier, conditions, format), conditions); | ||
| expect(violations).toEqual([]); | ||
| }); | ||
| } | ||
| } | ||
| } | ||
| }); | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same is probably true for this list, i.e. if we change our build tooling we would want a way to remind us that this needs to be updated I think