-
Notifications
You must be signed in to change notification settings - Fork 269
feat(builders,nitro): configurable sourcemap mode #1843
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,6 @@ | ||
| --- | ||
| "@workflow/builders": minor | ||
| "@workflow/nitro": minor | ||
| --- | ||
|
|
||
| Add a `sourcemap` builder option and matching `WORKFLOW_SOURCEMAP` environment variable that accept esbuild's sourcemap values. Setting this to `false` drops inline sourcemaps from generated bundles to reduce function size. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,151 @@ | ||
| import { afterEach, beforeEach, describe, expect, it } from 'vitest'; | ||
| import { BaseBuilder } from './base-builder.js'; | ||
| import type { SourcemapMode, StandaloneConfig } from './types.js'; | ||
|
|
||
| /** | ||
| * Minimal subclass that exposes the protected `resolveSourcemap()` and | ||
| * `sourcemapsEnabled` members for testing. | ||
| */ | ||
| class TestBuilder extends BaseBuilder { | ||
| async build(): Promise<void> { | ||
| // no-op | ||
| } | ||
|
|
||
| public callResolveSourcemap(defaultMode: SourcemapMode): SourcemapMode { | ||
| return this.resolveSourcemap(defaultMode); | ||
| } | ||
|
|
||
| public get publicSourcemapsEnabled(): boolean { | ||
| return this.sourcemapsEnabled; | ||
| } | ||
| } | ||
|
|
||
| function createBuilder(sourcemap?: SourcemapMode): TestBuilder { | ||
| const config: StandaloneConfig = { | ||
| buildTarget: 'standalone', | ||
| workingDir: '/tmp/workflow-test', | ||
| dirs: ['.'], | ||
| stepsBundlePath: '', | ||
| workflowsBundlePath: '', | ||
| webhookBundlePath: '', | ||
| sourcemap, | ||
| }; | ||
| return new TestBuilder(config); | ||
| } | ||
|
|
||
| describe('resolveSourcemap', () => { | ||
| const originalEnv = process.env.WORKFLOW_SOURCEMAP; | ||
|
|
||
| beforeEach(() => { | ||
| delete process.env.WORKFLOW_SOURCEMAP; | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| if (originalEnv === undefined) { | ||
| delete process.env.WORKFLOW_SOURCEMAP; | ||
| } else { | ||
| process.env.WORKFLOW_SOURCEMAP = originalEnv; | ||
| } | ||
| }); | ||
|
|
||
| it('returns the default when no config or env var is set', () => { | ||
| const builder = createBuilder(); | ||
| expect(builder.callResolveSourcemap('inline')).toBe('inline'); | ||
| expect(builder.callResolveSourcemap(false)).toBe(false); | ||
| expect(builder.callResolveSourcemap(true)).toBe(true); | ||
| }); | ||
|
|
||
| it('prefers explicit config over the default', () => { | ||
| expect(createBuilder(false).callResolveSourcemap('inline')).toBe(false); | ||
| expect(createBuilder('external').callResolveSourcemap('inline')).toBe( | ||
| 'external' | ||
| ); | ||
| expect(createBuilder('linked').callResolveSourcemap(false)).toBe('linked'); | ||
| expect(createBuilder(true).callResolveSourcemap('inline')).toBe(true); | ||
| }); | ||
|
|
||
| it('prefers explicit config over environment variable', () => { | ||
| process.env.WORKFLOW_SOURCEMAP = 'inline'; | ||
| expect(createBuilder(false).callResolveSourcemap('inline')).toBe(false); | ||
| expect(createBuilder('external').callResolveSourcemap('inline')).toBe( | ||
| 'external' | ||
| ); | ||
| }); | ||
|
|
||
| it('uses environment variable when config is not set', () => { | ||
| process.env.WORKFLOW_SOURCEMAP = 'false'; | ||
| expect(createBuilder().callResolveSourcemap('inline')).toBe(false); | ||
|
|
||
| process.env.WORKFLOW_SOURCEMAP = 'true'; | ||
| expect(createBuilder().callResolveSourcemap(false)).toBe(true); | ||
|
|
||
| for (const mode of ['inline', 'linked', 'external', 'both'] as const) { | ||
| process.env.WORKFLOW_SOURCEMAP = mode; | ||
| expect(createBuilder().callResolveSourcemap('inline')).toBe(mode); | ||
| } | ||
| }); | ||
|
|
||
| it('accepts "0" / "1" as environment variable aliases for false / true', () => { | ||
| process.env.WORKFLOW_SOURCEMAP = '0'; | ||
| expect(createBuilder().callResolveSourcemap('inline')).toBe(false); | ||
|
|
||
| process.env.WORKFLOW_SOURCEMAP = '1'; | ||
| expect(createBuilder().callResolveSourcemap(false)).toBe(true); | ||
| }); | ||
|
|
||
| it('falls back to default when env var is empty or unrecognized', () => { | ||
| process.env.WORKFLOW_SOURCEMAP = ''; | ||
| expect(createBuilder().callResolveSourcemap('inline')).toBe('inline'); | ||
|
|
||
| // Suppress the expected warning | ||
| const originalWarn = console.warn; | ||
| console.warn = () => {}; | ||
| try { | ||
| process.env.WORKFLOW_SOURCEMAP = 'nonsense'; | ||
| expect(createBuilder().callResolveSourcemap('inline')).toBe('inline'); | ||
| } finally { | ||
| console.warn = originalWarn; | ||
| } | ||
| }); | ||
| }); | ||
|
|
||
| describe('sourcemapsEnabled', () => { | ||
| const originalEnv = process.env.WORKFLOW_SOURCEMAP; | ||
|
|
||
| beforeEach(() => { | ||
| delete process.env.WORKFLOW_SOURCEMAP; | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| if (originalEnv === undefined) { | ||
| delete process.env.WORKFLOW_SOURCEMAP; | ||
| } else { | ||
| process.env.WORKFLOW_SOURCEMAP = originalEnv; | ||
| } | ||
| }); | ||
|
|
||
| it('is true by default', () => { | ||
| expect(createBuilder().publicSourcemapsEnabled).toBe(true); | ||
| }); | ||
|
|
||
| it('is false when config sourcemap is false', () => { | ||
| expect(createBuilder(false).publicSourcemapsEnabled).toBe(false); | ||
| }); | ||
|
|
||
| it('is true for any non-false config value', () => { | ||
| for (const mode of [ | ||
| true, | ||
| 'inline', | ||
| 'linked', | ||
| 'external', | ||
| 'both', | ||
| ] as const) { | ||
| expect(createBuilder(mode).publicSourcemapsEnabled).toBe(true); | ||
| } | ||
| }); | ||
|
|
||
| it('is false when WORKFLOW_SOURCEMAP env is false', () => { | ||
| process.env.WORKFLOW_SOURCEMAP = 'false'; | ||
| expect(createBuilder().publicSourcemapsEnabled).toBe(false); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -8,6 +8,19 @@ export const validBuildTargets = [ | |||||||||||||||||
| ] as const; | ||||||||||||||||||
| export type BuildTarget = (typeof validBuildTargets)[number]; | ||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
| * Sourcemap mode for generated workflow bundles. Matches the values | ||||||||||||||||||
| * accepted by esbuild's `sourcemap` option. | ||||||||||||||||||
| * | ||||||||||||||||||
| * - `true` / `'linked'` — emit a separate `.map` file and append a | ||||||||||||||||||
| * `//# sourceMappingURL=` comment pointing to it. | ||||||||||||||||||
| * - `'inline'` — inline the sourcemap as a base64 data URL in the bundle. | ||||||||||||||||||
| * - `'external'` — emit a separate `.map` file without a reference comment. | ||||||||||||||||||
| * - `'both'` — inline *and* emit a separate `.map` file. | ||||||||||||||||||
| * - `false` — do not emit a sourcemap at all. | ||||||||||||||||||
| */ | ||||||||||||||||||
| export type SourcemapMode = boolean | 'inline' | 'linked' | 'external' | 'both'; | ||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
| * Common configuration options shared across all builder types. | ||||||||||||||||||
| */ | ||||||||||||||||||
|
|
@@ -48,6 +61,24 @@ interface BaseWorkflowConfig { | |||||||||||||||||
|
|
||||||||||||||||||
| // Node.js runtime version for Vercel Functions (e.g., "nodejs22.x", "nodejs24.x") | ||||||||||||||||||
| runtime?: string; | ||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
| * Sourcemap mode for generated workflow bundles (steps, workflows, webhook). | ||||||||||||||||||
| * | ||||||||||||||||||
| * Accepts the same values as esbuild's `sourcemap` option: | ||||||||||||||||||
| * `true` / `'linked'`, `'inline'`, `'external'`, `'both'`, or `false`. | ||||||||||||||||||
| * | ||||||||||||||||||
| * If unset, the value of the `WORKFLOW_SOURCEMAP` environment variable is | ||||||||||||||||||
| * consulted (valid values: `true`, `false`, `inline`, `linked`, `external`, | ||||||||||||||||||
| * `both`). If neither is set, sourcemaps default to `'inline'` so stack | ||||||||||||||||||
| * traces from step and workflow VM execution include original file names | ||||||||||||||||||
| * and line numbers. | ||||||||||||||||||
|
Comment on lines
+73
to
+75
|
||||||||||||||||||
| * `both`). If neither is set, sourcemaps default to `'inline'` so stack | |
| * traces from step and workflow VM execution include original file names | |
| * and line numbers. | |
| * `both`). If neither is set, the effective default depends on the bundle | |
| * being generated. Step and workflow VM bundles typically default to | |
| * `'inline'` so stack traces include original file names and line numbers, | |
| * while webhook/final-wrapper bundles may default to `false` unless | |
| * debugging or an explicit sourcemap override enables them. |
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.
resolveSourcemap() re-parses WORKFLOW_SOURCEMAP on every call. When the env value is unrecognized, parseSourcemapEnv() emits a warning, which can result in repeated identical warnings during a single build (steps + workflows + webhook + vc-config). Consider memoizing the parsed env value (and/or warning once) to keep logs from getting noisy.