Skip to content

Commit 48e10d4

Browse files
committed
feat(cli,build,core): complete the dev createRequire warning, diagnostics-only suppression
Adds the scan/suppression halves of the dev warning (the devSession wiring landed in the previous commit): nested-arg matching feeds both targets, and a usage only warns when the package is missing from the resolved externals and every configured external. additionalPackages declares its packages via a new diagnostics-only BuildExtension field, installedPackagesForTarget, so a configured fix silences the warning without changing bundling output.
1 parent bf7849c commit 48e10d4

7 files changed

Lines changed: 148 additions & 20 deletions

File tree

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
---
22
"trigger.dev": patch
3+
"@trigger.dev/build": patch
4+
"@trigger.dev/core": patch
35
---
46

5-
Deploys now warn when a package is loaded with `createRequire()` but won't be available in the deployed image. The bundler can't follow `createRequire()` calls, so such a package is neither bundled nor installed, and previously this failed only at runtime with a confusing module-not-found error. The warning points at the exact file and line and suggests the `additionalPackages` build extension. Deploys also now surface the bundler's own warnings for your files (for example `require()` with a non-literal argument) instead of discarding them.
7+
`deploy` and `dev` now warn when a package is loaded with `createRequire()` but won't be available in the deployed image. The bundler can't follow `createRequire()` calls, so such a package is neither bundled nor installed, and previously this failed only at runtime in production with a confusing module-not-found error (`dev` works because your local `node_modules` exists, which made the failure deploy-only). The warning points at the exact file and line and shows the `additionalPackages` config that fixes it; packages declared via `additionalPackages` don't warn. Deploys also now surface the bundler's own warnings for your files (for example `require()` with a non-literal argument) instead of discarding them. Bundling output is unchanged.

‎packages/build/src/extensions/core/additionalPackages.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,13 @@ export type AdditionalPackagesOptions = {
1919
export function additionalPackages(options: AdditionalPackagesOptions): BuildExtension {
2020
return {
2121
name: "additionalPackages",
22+
installedPackagesForTarget(target) {
23+
if (target === "dev") {
24+
return [];
25+
}
26+
27+
return options.packages.map((pkg) => parsePackageName(pkg).name);
28+
},
2229
async onBuildStart(context) {
2330
if (context.target !== "deploy") {
2431
return;

‎packages/cli-v3/src/build/buildWorker.ts‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,19 @@ import {
77
createBuildManifestFromBundle,
88
logBuildWarnings,
99
} from "./bundle.js";
10-
import { CreateRequireCollector, createRequireUsageToWarning } from "./createRequireWarnings.js";
10+
import {
11+
CreateRequireCollector,
12+
createRequireUsageToWarning,
13+
unavailableCreateRequireUsages,
14+
} from "./createRequireWarnings.js";
1115
import { bundleSkills } from "./bundleSkills.js";
1216
import {
1317
createBuildContext,
1418
notifyExtensionOnBuildComplete,
1519
notifyExtensionOnBuildStart,
1620
resolvePluginsForContext,
1721
} from "./extensions.js";
18-
import { createExternalsBuildExtension } from "./externals.js";
22+
import { createExternalsBuildExtension, deployExternalMatchers } from "./externals.js";
1923
import { tmpdir } from "node:os";
2024
import { mkdtemp, rm } from "node:fs/promises";
2125
import { join, relative, sep } from "node:path";
@@ -142,7 +146,9 @@ export async function buildWorker(options: BuildWorkerOptions) {
142146
const buildWarnings = collectDeployBuildWarnings(
143147
bundleResult,
144148
createRequireCollector,
145-
buildManifest
149+
buildManifest,
150+
resolvedConfig,
151+
options.forcedExternals
146152
);
147153

148154
if (buildWarnings.length > 0) {
@@ -167,12 +173,15 @@ export async function buildWorker(options: BuildWorkerOptions) {
167173
/**
168174
* Deploy-only diagnostics: esbuild's own warnings scoped to the user's files,
169175
* plus packages loaded via createRequire() that end up neither bundled nor
170-
* installed in the image (i.e. not in the manifest's externals).
176+
* installed in the image (not in the manifest's externals and not configured
177+
* as an external anywhere).
171178
*/
172179
function collectDeployBuildWarnings(
173180
bundleResult: BundleResult,
174181
createRequireCollector: CreateRequireCollector,
175-
buildManifest: BuildManifest
182+
buildManifest: BuildManifest,
183+
resolvedConfig: ResolvedConfig,
184+
forcedExternals: string[] = []
176185
): esbuild.PartialMessage[] {
177186
const esbuildWarnings = bundleResult.warnings.filter(
178187
(warning) => warning.location?.file && !warning.location.file.includes("node_modules")
@@ -182,9 +191,11 @@ function collectDeployBuildWarnings(
182191
(buildManifest.externals ?? []).map((external) => external.name)
183192
);
184193

185-
const createRequireWarnings = createRequireCollector.usages
186-
.filter((usage) => !installedPackages.has(usage.packageName))
187-
.map(createRequireUsageToWarning);
194+
const createRequireWarnings = unavailableCreateRequireUsages(
195+
createRequireCollector.usages,
196+
installedPackages,
197+
deployExternalMatchers(resolvedConfig, forcedExternals)
198+
).map((usage) => createRequireUsageToWarning(usage, "deploy"));
188199

189200
return [...esbuildWarnings, ...createRequireWarnings];
190201
}

‎packages/cli-v3/src/build/createRequireWarnings.test.ts‎

Lines changed: 53 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
createRequireUsageToWarning,
99
packageNameForSpecifier,
1010
scanSourceForCreateRequire,
11+
unavailableCreateRequireUsages,
1112
} from "./createRequireWarnings.js";
1213

1314
describe("scanSourceForCreateRequire", () => {
@@ -225,15 +226,17 @@ export const mssql = createRequire(import.meta.url)("mssql");
225226
});
226227

227228
describe("createRequireUsageToWarning", () => {
229+
const usage = {
230+
specifier: "mssql/lib/tedious",
231+
packageName: "mssql",
232+
file: "src/db.ts",
233+
line: 12,
234+
column: 20,
235+
lineText: `const mssql = createRequire(import.meta.url)("mssql/lib/tedious");`,
236+
};
237+
228238
it("carries the concrete fix in a note", () => {
229-
const warning = createRequireUsageToWarning({
230-
specifier: "mssql/lib/tedious",
231-
packageName: "mssql",
232-
file: "src/db.ts",
233-
line: 12,
234-
column: 20,
235-
lineText: `const mssql = createRequire(import.meta.url)("mssql/lib/tedious");`,
236-
});
239+
const warning = createRequireUsageToWarning(usage, "deploy");
237240

238241
expect(warning.location).toMatchObject({ file: "src/db.ts", line: 12, column: 20 });
239242

@@ -242,6 +245,48 @@ describe("createRequireUsageToWarning", () => {
242245
expect(note).toContain(`additionalPackages({ packages: ["mssql"] })`);
243246
expect(note).toContain("https://trigger.dev/docs/config/extensions/additionalPackages");
244247
});
248+
249+
it("explains that the failure is deploy-only when building for dev", () => {
250+
const warning = createRequireUsageToWarning(usage, "dev");
251+
252+
expect(warning.text).toContain("works locally");
253+
expect(warning.text).toContain("deploys of this code will fail at runtime");
254+
});
255+
});
256+
257+
describe("unavailableCreateRequireUsages", () => {
258+
const usageFor = (specifier: string, packageName: string) => ({
259+
specifier,
260+
packageName,
261+
file: "src/db.ts",
262+
line: 1,
263+
column: 0,
264+
lineText: "",
265+
});
266+
267+
it("keeps usages that are neither installed nor configured as external", () => {
268+
const usages = [usageFor("mssql", "mssql")];
269+
270+
expect(unavailableCreateRequireUsages(usages, new Set(), [])).toHaveLength(1);
271+
});
272+
273+
it("drops usages whose package is in the resolved externals", () => {
274+
const usages = [usageFor("sharp", "sharp"), usageFor("mssql", "mssql")];
275+
276+
const result = unavailableCreateRequireUsages(usages, new Set(["sharp"]), []);
277+
278+
expect(result.map((u) => u.packageName)).toEqual(["mssql"]);
279+
});
280+
281+
it("drops usages matching a configured external pattern", () => {
282+
const usages = [usageFor("mssql/lib/tedious", "mssql"), usageFor("pg", "pg")];
283+
284+
const result = unavailableCreateRequireUsages(usages, new Set(), [
285+
new RegExp(`^mssql(?:/[^'"]*)?$`),
286+
]);
287+
288+
expect(result.map((u) => u.packageName)).toEqual(["pg"]);
289+
});
245290
});
246291

247292
describe("packageNameForSpecifier", () => {

‎packages/cli-v3/src/build/createRequireWarnings.ts‎

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { BuildTarget } from "@trigger.dev/core/v3/schemas";
12
import * as esbuild from "esbuild";
23
import { readFile } from "node:fs/promises";
34
import { builtinModules } from "node:module";
@@ -243,10 +244,37 @@ export class CreateRequireCollector {
243244
}
244245
}
245246

246-
export function createRequireUsageToWarning(usage: CreateRequireUsage): esbuild.PartialMessage {
247+
/**
248+
* Filters collected usages down to the ones that will actually be missing at
249+
* runtime in the deployed image: not in the resolved externals (installed
250+
* dependencies) and not matching any configured external.
251+
*/
252+
export function unavailableCreateRequireUsages(
253+
usages: ReadonlyArray<CreateRequireUsage>,
254+
installedPackages: Set<string>,
255+
externalMatchers: RegExp[]
256+
): CreateRequireUsage[] {
257+
return usages.filter(
258+
(usage) =>
259+
!installedPackages.has(usage.packageName) &&
260+
!externalMatchers.some(
261+
(matcher) => matcher.test(usage.packageName) || matcher.test(usage.specifier)
262+
)
263+
);
264+
}
265+
266+
export function createRequireUsageToWarning(
267+
usage: CreateRequireUsage,
268+
target: BuildTarget
269+
): esbuild.PartialMessage {
270+
const text =
271+
target === "dev"
272+
? `"${usage.specifier}" is loaded with createRequire(). This works locally because your project's node_modules exists, but the package won't be available in the deployed image, so deploys of this code will fail at runtime. The bundler can't follow createRequire() calls, so "${usage.packageName}" is neither bundled into your code nor installed in the image.`
273+
: `"${usage.specifier}" is loaded with createRequire() but won't be available in the deployed image, so loading it will fail at runtime. The bundler can't follow createRequire() calls, so "${usage.packageName}" is neither bundled into your code nor installed in the image.`;
274+
247275
return {
248276
pluginName: "create-require-collector",
249-
text: `"${usage.specifier}" is loaded with createRequire() but won't be available in the deployed image, so loading it will fail at runtime. The bundler can't follow createRequire() calls, so "${usage.packageName}" is neither bundled into your code nor installed in the image.`,
277+
text,
250278
location: {
251279
file: usage.file,
252280
line: usage.line,
@@ -266,7 +294,7 @@ export function createRequireUsageToWarning(usage: CreateRequireUsage): esbuild.
266294
},
267295
});
268296
269-
Alternatively, import the package statically so it gets bundled. Docs: https://trigger.dev/docs/config/extensions/additionalPackages`,
297+
Alternatively, replace the createRequire() call with a static import so the package is bundled. Docs: https://trigger.dev/docs/config/extensions/additionalPackages`,
270298
},
271299
],
272300
};

‎packages/cli-v3/src/build/externals.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -414,6 +414,34 @@ function createExternalsCollector(
414414
};
415415
}
416416

417+
/**
418+
* Matchers for every package that will be available at runtime in the deployed
419+
* image: configured externals (build.external, instrumented packages,
420+
* extension-declared externals, server-forced externals) plus packages that
421+
* extensions install into the image (e.g. additionalPackages). Diagnostics
422+
* only, regardless of the target currently being built.
423+
*/
424+
export function deployExternalMatchers(
425+
config: ResolvedConfig,
426+
forcedExternal: string[] = []
427+
): RegExp[] {
428+
const matchers = discoverMaybeExternals("deploy", config, forcedExternal).map(
429+
(external) => external.filter
430+
);
431+
432+
for (const buildExtension of config.build?.extensions ?? []) {
433+
for (const packageName of buildExtension.installedPackagesForTarget?.("deploy") ?? []) {
434+
const filter = makeExternalRegexp(packageName);
435+
436+
if (filter) {
437+
matchers.push(filter);
438+
}
439+
}
440+
}
441+
442+
return matchers;
443+
}
444+
417445
type MaybeExternal = { raw: string; filter: RegExp };
418446

419447
function discoverMaybeExternals(

‎packages/core/src/v3/build/extensions.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,13 @@ export function esbuildPlugin(plugin: Plugin, options: RegisterPluginOptions = {
1414
export interface BuildExtension {
1515
name: string;
1616
externalsForTarget?: (target: BuildTarget) => string[] | undefined;
17+
/**
18+
* Package names this extension installs into the deployed image for the
19+
* given target. Diagnostics only: the bundler ignores this, it just tells
20+
* build warnings (e.g. the createRequire scan) the package will be
21+
* available at runtime.
22+
*/
23+
installedPackagesForTarget?: (target: BuildTarget) => string[] | undefined;
1724
onBuildStart?: (context: BuildContext) => Promise<void> | void;
1825
onBuildComplete?: (
1926
context: BuildContext,

0 commit comments

Comments
 (0)