Skip to content

Commit 1c65950

Browse files
committed
fix(cli,build): dev warnings default on for undeclared extensions, scanner edge fixes
Inverts the extension-declaration default: an extension that declares no installed packages is assumed to install none, so dev warnings stay live for third-party and yet-to-declare extensions instead of silently disabling the feature (proven twice by first-party sweeps missing extensions). The incomplete guard remains only where false warnings are genuinely likely: a throwing declaration hook, or an additionalPackages extension too old to declare, and the engine-only prisma mode now declares @prisma/engines. Also: specifier-form exports (export { req }) are followed cross-file, npm-alias install tokens suppress the aliased name, literal Windows path specifiers never warn, the bare-specifier check reuses isBareModuleImport, collector usages swap atomically per build, exported-name mention checks use identifier boundaries, and signature-miss re-reads run under the concurrency cap with failures logged.
1 parent 8c7f184 commit 1c65950

5 files changed

Lines changed: 145 additions & 39 deletions

File tree

.changeset/warn-createrequire-deploy.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,4 @@
44
"@trigger.dev/core": patch
55
---
66

7-
`deploy` and `dev` now warn, with the file, line, and suggested `additionalPackages` fix, when code loads a package through `createRequire()` that won't be available in the deployed image and would previously only fail at runtime in production.
7+
`deploy` and `dev` now warn, with the file, line, and suggested `additionalPackages` fix, when code loads a package through `createRequire()` that won't be available in the deployed image and would previously only fail at runtime in production. Deploys also now show the bundler's own warnings for your code instead of discarding them.

packages/build/src/extensions/prisma.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -815,6 +815,14 @@ export class PrismaEngineOnlyModeExtension implements BuildExtension {
815815
this._binaryTarget = options.binaryTarget ?? "debian-openssl-3.0.x";
816816
}
817817

818+
installedPackagesForTarget(target: BuildTarget) {
819+
if (target !== "deploy") {
820+
return [];
821+
}
822+
823+
return ["@prisma/engines"];
824+
}
825+
818826
async onBuildComplete(context: BuildContext, manifest: BuildManifest) {
819827
if (context.target === "dev") {
820828
return;

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

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,24 @@ const pg = req("pg");
348348
expect(scanSourceForCreateRequire(source).map((r) => r.specifier)).toEqual(["pg"]);
349349
});
350350

351+
it("ignores literal Windows path specifiers", () => {
352+
const source =
353+
'import { createRequire } from "node:module";\n' +
354+
"const req = createRequire(import.meta.url);\n" +
355+
'const helper = req("C:\\\\tools\\\\helper.cjs");\n';
356+
357+
expect(scanSourceForCreateRequire(source)).toEqual([]);
358+
});
359+
360+
it("records require functions exported in specifier form", () => {
361+
const source = `import { createRequire } from "node:module";
362+
const cjsRequire = createRequire(import.meta.url);
363+
export { cjsRequire };
364+
`;
365+
366+
expect(scanSource(source).exportedRequireFns).toEqual(["cjsRequire"]);
367+
});
368+
351369
it("follows require functions imported from other scanned files", () => {
352370
const util = `import { createRequire } from "node:module";
353371
export const cjsRequire = createRequire(import.meta.url);
@@ -712,18 +730,21 @@ describe("extensionInstalledPackageMatchers", () => {
712730
expect(incomplete).toBe(true);
713731
});
714732

715-
it("marks the result incomplete for any hook-bearing extension that declares no packages", () => {
716-
const oldAdditionalPackages = extensionInstalledPackageMatchers(
717-
configWith([{ name: "additionalPackages", onBuildStart: () => {} }])
733+
it("assumes an undeclared extension installs nothing", () => {
734+
const { matchers, incomplete } = extensionInstalledPackageMatchers(
735+
configWith([{ name: "someThirdPartyExtension", onBuildComplete: () => {} }])
718736
);
719737

720-
expect(oldAdditionalPackages.incomplete).toBe(true);
738+
expect(incomplete).toBe(false);
739+
expect(matchers).toEqual([]);
740+
});
721741

722-
const layerInstaller = extensionInstalledPackageMatchers(
723-
configWith([{ name: "syncEnvVars", onBuildComplete: () => {} }])
742+
it("marks the result incomplete for an additionalPackages extension that predates the hook", () => {
743+
const { incomplete } = extensionInstalledPackageMatchers(
744+
configWith([{ name: "additionalPackages", onBuildStart: () => {} }])
724745
);
725746

726-
expect(layerInstaller.incomplete).toBe(true);
747+
expect(incomplete).toBe(true);
727748
});
728749
});
729750

@@ -828,12 +849,20 @@ describe("packagesInstalledByCommands", () => {
828849
"npm install @prisma/engines@5.0.0",
829850
"pnpm add wrangler prisma@3.0.0 --save-dev",
830851
"yarn add -D typescript",
852+
"npm install sqlite3@npm:@vscode/sqlite3",
853+
"npm install file:../local-lib",
831854
"bun run generate",
832855
"npm ci",
833856
"apt-get install -y ffmpeg",
834857
]);
835858

836-
expect(packages.sort()).toEqual(["@prisma/engines", "prisma", "typescript", "wrangler"]);
859+
expect(packages.sort()).toEqual([
860+
"@prisma/engines",
861+
"prisma",
862+
"sqlite3",
863+
"typescript",
864+
"wrangler",
865+
]);
837866
});
838867
});
839868

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

Lines changed: 97 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,13 @@ import { dirname, isAbsolute, resolve } from "node:path";
77
import pLimit from "p-limit";
88
import { tryCatch } from "@trigger.dev/core/v3";
99
import { logger } from "../utilities/logger.js";
10-
import { isBuiltinModule, makeExternalRegexp, packageNameForImportPath } from "./externals.js";
10+
import {
11+
escapeRegExp,
12+
isBareModuleImport,
13+
isBuiltinModule,
14+
makeExternalRegexp,
15+
packageNameForImportPath,
16+
} from "./externals.js";
1117

1218
export { packageNameForImportPath as packageNameForSpecifier } from "./externals.js";
1319

@@ -140,6 +146,12 @@ export function scanSource(
140146
}
141147
}
142148

149+
for (const name of collected.exportSpecifierNames) {
150+
if (requireFnNames.has(name)) {
151+
exportedRequireFns.add(name);
152+
}
153+
}
154+
143155
if (isKnownRequireFnImport) {
144156
for (const relativeImport of collected.relativeNamedImports) {
145157
if (isKnownRequireFnImport(relativeImport.importedName, relativeImport.fromSpecifier)) {
@@ -210,6 +222,8 @@ function parseWithFallbacks(source: string): AstNode | undefined {
210222
type AstFacts = {
211223
moduleImports: Array<{ kind: "createRequire" | "namespace"; localName: string }>;
212224
bindings: Array<{ name: string; value: AstNode; exported: boolean }>;
225+
/** Local names exported via `export { name }` (specifier form) */
226+
exportSpecifierNames: string[];
213227
relativeNamedImports: Array<{ importedName: string; localName: string; fromSpecifier: string }>;
214228
calls: Array<{
215229
callee: AstNode;
@@ -224,6 +238,7 @@ function collectAstFacts(ast: AstNode): AstFacts {
224238
const facts: AstFacts = {
225239
moduleImports: [],
226240
bindings: [],
241+
exportSpecifierNames: [],
227242
relativeNamedImports: [],
228243
calls: [],
229244
};
@@ -235,6 +250,14 @@ function collectAstFacts(ast: AstNode): AstFacts {
235250

236251
if (declaration) {
237252
visit(declaration, true);
253+
} else if (!node.source) {
254+
for (const specifier of (node.specifiers as AstNode[]) ?? []) {
255+
const local = specifier.local as AstNode | undefined;
256+
257+
if (specifier.type === "ExportSpecifier" && local?.type === "Identifier") {
258+
facts.exportSpecifierNames.push(local.name as string);
259+
}
260+
}
238261
}
239262

240263
return;
@@ -429,13 +452,17 @@ function stringArgumentValue(node: AstNode | undefined): string | undefined {
429452
}
430453

431454
function isWarnableSpecifier(specifier: string): boolean {
432-
const nonPackagePrefixes = [".", "/", "~", "#", "file:", "data:", "node:"];
433-
434-
if (nonPackagePrefixes.some((prefix) => specifier.startsWith(prefix)) || specifier.length === 0) {
455+
if (
456+
specifier.length === 0 ||
457+
specifier.startsWith("#") ||
458+
specifier.startsWith("node:") ||
459+
specifier.includes("\\") ||
460+
/^[A-Za-z]:/.test(specifier)
461+
) {
435462
return false;
436463
}
437464

438-
return !isBuiltinModule(packageNameForImportPath(specifier));
465+
return isBareModuleImport(specifier) && !isBuiltinModule(packageNameForImportPath(specifier));
439466
}
440467

441468
const SCANNABLE_FILE_REGEX = /\.(?:m|c)?(?:j|t)sx?$/;
@@ -478,14 +505,14 @@ export class CreateRequireCollector {
478505
name: "create-require-collector",
479506
setup: (build) => {
480507
build.onEnd(async (result) => {
481-
this._usages = [];
482-
483508
if (!result.metafile) {
509+
this._usages = [];
510+
484511
return;
485512
}
486513

487514
try {
488-
await this.collect(result.metafile);
515+
this._usages = await this.collect(result.metafile);
489516
} catch (error) {
490517
logger.debug("[createRequire] Scan failed; skipping warnings", { error });
491518
this._usages = [];
@@ -497,7 +524,8 @@ export class CreateRequireCollector {
497524
return this._plugin;
498525
}
499526

500-
private async collect(metafile: esbuild.Metafile): Promise<void> {
527+
private async collect(metafile: esbuild.Metafile): Promise<CreateRequireUsage[]> {
528+
const usages: CreateRequireUsage[] = [];
501529
const files: Array<{ inputPath: string; filePath: string }> = [];
502530
const seenPaths = new Set<string>();
503531
const importResolutions = new Map<string, Map<string, string>>();
@@ -601,22 +629,49 @@ export class CreateRequireCollector {
601629
.sort()
602630
.join(",");
603631

632+
const exportedNamePattern =
633+
exportedNames.size > 0
634+
? new RegExp(`\\b(?:${Array.from(exportedNames).map(escapeRegExp).join("|")})\\b`)
635+
: undefined;
636+
637+
const needsSource = scanned.filter(
638+
(entry) =>
639+
entry.source === undefined &&
640+
!(entry.cached && entry.cached.knownsSignature === knownsSignature)
641+
);
642+
643+
await Promise.all(
644+
needsSource.map((entry) =>
645+
limit(async () => {
646+
const [readError, contents] = await tryCatch(readFile(entry.filePath, "utf8"));
647+
648+
if (readError) {
649+
logger.debug("[createRequire] Unable to re-read bundle input file", {
650+
filePath: entry.filePath,
651+
error: readError,
652+
});
653+
654+
return;
655+
}
656+
657+
entry.source = contents;
658+
})
659+
)
660+
);
661+
604662
for (const entry of scanned) {
605663
let finalSpecifiers: CreateRequireSpecifier[];
606664

607665
if (entry.cached && entry.cached.knownsSignature === knownsSignature) {
608666
finalSpecifiers = entry.cached.finalSpecifiers;
609667
} else {
610-
const source =
611-
entry.source ?? (await tryCatch(readFile(entry.filePath, "utf8")))[1] ?? undefined;
668+
const source = entry.source;
612669

613670
if (source === undefined) {
614671
continue;
615672
}
616673

617-
const mentionsExportedName = Array.from(exportedNames).some((name) =>
618-
source.includes(name)
619-
);
674+
const mentionsExportedName = exportedNamePattern?.test(source) ?? false;
620675

621676
if (!mentionsExportedName) {
622677
finalSpecifiers = entry.base.specifiers;
@@ -647,26 +702,29 @@ export class CreateRequireCollector {
647702
}
648703

649704
for (const found of finalSpecifiers) {
650-
this._usages.push({
705+
usages.push({
651706
...found,
652707
file: entry.inputPath,
653708
packageName: packageNameForImportPath(found.specifier),
654709
});
655710
}
656711
}
712+
713+
return usages;
657714
}
658715
}
659716

660717
export type ExtensionInstalledPackages = {
661718
matchers: RegExp[];
662719
/**
663-
* True when what extensions install can't be fully determined: an extension
664-
* hook threw, or an extension participates in the build (has build hooks)
665-
* without declaring installedPackagesForTarget or externalsForTarget, so it
666-
* may install packages invisibly (e.g. via a build layer). Dev-mode
667-
* warnings must stay silent in that case rather than risk false "deploys
668-
* will fail" claims; deploy-mode warnings are unaffected because the
669-
* manifest externals capture layer dependencies there.
720+
* True when what extensions install can't be determined in a way that
721+
* makes false warnings likely: an extension hook threw, or an
722+
* additionalPackages extension predates the installedPackagesForTarget
723+
* hook (the exact extension the warning's own fix advice prescribes).
724+
* Dev-mode warnings stay silent in that case; deploy-mode warnings are
725+
* unaffected because the manifest externals capture layer dependencies
726+
* there. Extensions that declare nothing are assumed to install nothing:
727+
* package-installing extensions are the rare case and declare themselves.
670728
*/
671729
incomplete: boolean;
672730
};
@@ -694,12 +752,12 @@ export function extensionInstalledPackageMatchers(
694752
const declaresPackages =
695753
typeof buildExtension.installedPackagesForTarget === "function" ||
696754
typeof buildExtension.externalsForTarget === "function";
697-
const hasBuildHooks =
698-
typeof buildExtension.onBuildStart === "function" ||
699-
typeof buildExtension.onBuildComplete === "function";
700755

701-
if (!declaresPackages && hasBuildHooks) {
702-
incomplete = true;
756+
if (!declaresPackages) {
757+
if (buildExtension.name === "additionalPackages") {
758+
incomplete = true;
759+
}
760+
703761
continue;
704762
}
705763

@@ -757,7 +815,18 @@ export function packagesInstalledByCommands(commands: ReadonlyArray<string>): st
757815
for (const command of commands) {
758816
for (const match of command.matchAll(INSTALL_COMMAND_REGEX)) {
759817
for (const token of match[1]!.trim().split(/\s+/)) {
760-
if (token.length === 0 || token.startsWith("-") || token.includes(":")) {
818+
if (token.length === 0 || token.startsWith("-")) {
819+
continue;
820+
}
821+
822+
const aliasIndex = token.indexOf("@npm:");
823+
824+
if (aliasIndex > 0) {
825+
names.add(token.slice(0, aliasIndex));
826+
continue;
827+
}
828+
829+
if (token.includes(":")) {
761830
continue;
762831
}
763832

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -525,7 +525,7 @@ export function makeExternalRegexp(packageName: string): RegExp {
525525
return new RegExp(pattern);
526526
}
527527

528-
function escapeRegExp(value: string): string {
528+
export function escapeRegExp(value: string): string {
529529
return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
530530
}
531531

@@ -572,7 +572,7 @@ function resolveSync(id: string, resolveDir: string) {
572572
}
573573
}
574574

575-
function isBareModuleImport(path: string): boolean {
575+
export function isBareModuleImport(path: string): boolean {
576576
const excludes = [".", "/", "~", "file:", "data:"];
577577
return !excludes.some((exclude) => path.startsWith(exclude));
578578
}

0 commit comments

Comments
 (0)