diff --git a/.changeset/purple-bikes-own.md b/.changeset/purple-bikes-own.md new file mode 100644 index 0000000000..565c2797f2 --- /dev/null +++ b/.changeset/purple-bikes-own.md @@ -0,0 +1,5 @@ +--- +"trigger.dev": patch +--- + +Use only one call to dependencies listing commands in order to build the bundle's package.json file. diff --git a/packages/cli-v3/e2e/compile.ts b/packages/cli-v3/e2e/compile.ts index 11b930e684..778e208159 100644 --- a/packages/cli-v3/e2e/compile.ts +++ b/packages/cli-v3/e2e/compile.ts @@ -2,7 +2,7 @@ import { esbuildDecorators } from "@anatine/esbuild-decorators"; import { build } from "esbuild"; import { readFileSync } from "node:fs"; import { writeFile } from "node:fs/promises"; -import { basename, join, posix, relative, resolve, sep } from "node:path"; +import { join, posix, relative, resolve, sep } from "node:path"; import invariant from "tiny-invariant"; import { @@ -68,6 +68,8 @@ export async function compile(options: CompileOptions) { const e2eJsProject = new E2EJavascriptProject(config.projectDir, packageManager); const directDependenciesMeta = await e2eJsProject.extractDirectDependenciesMeta(); + logger.debug("Direct dependencies metadata", directDependenciesMeta); + const result = await build({ stdin: { contents: workerContents, @@ -214,9 +216,10 @@ export async function compile(options: CompileOptions) { await writeFile(join(tempDir, "index.js"), entryPointOutputFile.text); return { + entryPointMetaOutput, workerMetaOutput: metaOutput, + directDependenciesMeta, workerOutputFile, - entryPointMetaOutput, entryPointOutputFile, }; } diff --git a/packages/cli-v3/e2e/handleDependencies.ts b/packages/cli-v3/e2e/handleDependencies.ts index 6a021ddf32..388a984419 100644 --- a/packages/cli-v3/e2e/handleDependencies.ts +++ b/packages/cli-v3/e2e/handleDependencies.ts @@ -1,7 +1,6 @@ #!/usr/bin/env node import { log } from "@clack/prompts"; -import { Metafile } from "esbuild"; import { join } from "node:path"; import { SkipLoggingError } from "../src/cli/common.js"; @@ -16,10 +15,13 @@ import { PackageManager } from "../src/utilities/getUserPackageManager.js"; import { logger } from "../src/utilities/logger.js"; import { cliLink } from "../src/utilities/cliOutput.js"; import { E2EJavascriptProject } from "./javascriptProject.js"; +import { DependencyMeta } from "../src/utilities/javascriptProject.js"; +import { Metafile } from "esbuild"; type HandleDependenciesOptions = { entryPointMetaOutput: Metafile["outputs"]["out/stdin.js"]; metaOutput: Metafile["outputs"]["out/stdin.js"]; + directDependenciesMeta: Record; packageManager: PackageManager; resolvedConfig: ReadConfigResult; tempDir: string; @@ -32,6 +34,7 @@ export async function handleDependencies(options: HandleDependenciesOptions) { const { entryPointMetaOutput, metaOutput, + directDependenciesMeta, packageManager, resolvedConfig: { config }, tempDir, @@ -47,7 +50,11 @@ export async function handleDependencies(options: HandleDependenciesOptions) { const javascriptProject = new E2EJavascriptProject(config.projectDir, packageManager); - const dependencies = await resolveRequiredDependencies(allImports, config, javascriptProject); + const dependencies = await resolveRequiredDependencies( + directDependenciesMeta, + allImports, + config + ); logger.debug("gatherRequiredDependencies()", { dependencies }); diff --git a/packages/cli-v3/e2e/index.test.ts b/packages/cli-v3/e2e/index.test.ts index 18b06d66d2..a8af618728 100644 --- a/packages/cli-v3/e2e/index.test.ts +++ b/packages/cli-v3/e2e/index.test.ts @@ -20,6 +20,7 @@ import { E2EOptions, E2EOptionsSchema } from "./schemas"; import { fixturesConfig, TestCase } from "./fixtures.config"; import { Metafile, OutputFile } from "esbuild"; import { findUpMultiple } from "find-up"; +import { DependencyMeta } from "../src/utilities/javascriptProject"; interface E2EFixtureTest extends TestCase { fixtureDir: string; @@ -164,6 +165,7 @@ if (testCases.length > 0) { } let entryPointMetaOutput: Metafile["outputs"]["out/stdin.js"]; + let directDependenciesMeta: Record; let entryPointOutputFile: OutputFile; let workerMetaOutput: Metafile["outputs"]["out/stdin.js"]; let workerOutputFile: OutputFile; @@ -176,6 +178,7 @@ if (testCases.length > 0) { tempDir, }); entryPointMetaOutput = compilationResult.entryPointMetaOutput; + directDependenciesMeta = compilationResult.directDependenciesMeta; entryPointOutputFile = compilationResult.entryPointOutputFile; workerMetaOutput = compilationResult.workerMetaOutput; workerOutputFile = compilationResult.workerOutputFile; @@ -203,6 +206,7 @@ if (testCases.length > 0) { dependencies = await handleDependencies({ entryPointMetaOutput: entryPointMetaOutput!, metaOutput: workerMetaOutput!, + directDependenciesMeta: directDependenciesMeta!, resolvedConfig: resolvedConfig!, tempDir, packageManager, diff --git a/packages/cli-v3/src/commands/deploy.ts b/packages/cli-v3/src/commands/deploy.ts index a1a21bbaa6..6642800dc8 100644 --- a/packages/cli-v3/src/commands/deploy.ts +++ b/packages/cli-v3/src/commands/deploy.ts @@ -9,12 +9,12 @@ import { } from "@trigger.dev/core/v3"; import { recordSpanException } from "@trigger.dev/core/v3/workers"; import { Command, Option as CommandOption } from "commander"; -import { Metafile, build } from "esbuild"; +import { build, Metafile } from "esbuild"; import { execa } from "execa"; import { createHash } from "node:crypto"; import { readFileSync } from "node:fs"; import { copyFile, mkdir, readFile, writeFile } from "node:fs/promises"; -import { dirname, join, posix, relative, resolve } from "node:path"; +import { dirname, join, posix, relative } from "node:path"; import { setTimeout } from "node:timers/promises"; import invariant from "tiny-invariant"; import { z } from "zod"; @@ -56,7 +56,7 @@ import { parseBuildErrorStack, parseNpmInstallError, } from "../utilities/deployErrors"; -import { JavascriptProject } from "../utilities/javascriptProject"; +import { DependencyMeta, JavascriptProject } from "../utilities/javascriptProject"; import { docs, getInTouch } from "../utilities/links"; import { cliRootPath } from "../utilities/resolveInternalFilePath"; import { safeJsonParse } from "../utilities/safeJsonParse"; @@ -1170,6 +1170,11 @@ async function compileProject( ); } + const javascriptProject = new JavascriptProject(config.projectDir); + const directDependenciesMeta = await javascriptProject.extractDirectDependenciesMeta(); + + logger.debug("Direct dependencies metadata", directDependenciesMeta); + const result = await build({ stdin: { contents: workerContents, @@ -1335,9 +1340,11 @@ async function compileProject( // Get all the required dependencies from the metaOutputs and save them to /tmp/dir/package.json const allImports = [...metaOutput.imports, ...entryPointMetaOutput.imports]; - const javascriptProject = new JavascriptProject(config.projectDir); - - const dependencies = await resolveRequiredDependencies(allImports, config, javascriptProject); + const dependencies = await resolveRequiredDependencies( + directDependenciesMeta, + allImports, + config + ); logger.debug("gatherRequiredDependencies()", { dependencies }); @@ -1707,12 +1714,13 @@ export async function typecheckProject(config: ResolvedConfig) { // Returns the dependencies that are required by the output that are found in output and the CLI package dependencies // Returns the dependency names and the version to use (taken from the CLI deps package.json) export async function resolveRequiredDependencies( + directDependenciesMeta: Record, imports: Metafile["outputs"][string]["imports"], - config: ResolvedConfig, - project: JavascriptProject + config: ResolvedConfig ) { return await tracer.startActiveSpan("resolveRequiredDependencies", async (span) => { - const resolvablePackageNames = new Set(); + const dependencies: Record = {}; + const missingPackages: string[] = []; for (const file of imports) { if ((file.kind !== "require-call" && file.kind !== "dynamic-import") || !file.external) { @@ -1725,37 +1733,36 @@ export async function resolveRequiredDependencies( continue; } - resolvablePackageNames.add(packageName); - } - - span.setAttribute("resolvablePackageNames", Array.from(resolvablePackageNames)); - - const resolvedPackageVersions = await project.resolveAll(Array.from(resolvablePackageNames)); - const missingPackages = Array.from(resolvablePackageNames).filter( - (packageName) => !resolvedPackageVersions[packageName] - ); + if (!directDependenciesMeta[packageName]) { + continue; + } - span.setAttributes({ - ...flattenAttributes(resolvedPackageVersions, "resolvedPackageVersions"), - }); - span.setAttribute("missingPackages", missingPackages); + if (!directDependenciesMeta[packageName].external) { + continue; + } - const dependencies: Record = {}; + if (!directDependenciesMeta[packageName].version) { + missingPackages.push(packageName); + const internalDependencyVersion = + (packageJson.dependencies as Record)[packageName] ?? + detectDependencyVersion(packageName); - for (const missingPackage of missingPackages) { - const internalDependencyVersion = - (packageJson.dependencies as Record)[missingPackage] ?? - detectDependencyVersion(missingPackage); + if (internalDependencyVersion) { + dependencies[packageName] = stripWorkspaceFromVersion(internalDependencyVersion); + } - if (internalDependencyVersion) { - dependencies[missingPackage] = stripWorkspaceFromVersion(internalDependencyVersion); + continue; } - } - for (const [packageName, version] of Object.entries(resolvedPackageVersions)) { - dependencies[packageName] = version; + dependencies[packageName] = directDependenciesMeta[packageName].version; } + span.setAttribute("resolvablePackageNames", Object.keys(dependencies)); + span.setAttributes({ + ...flattenAttributes(dependencies, "resolvedPackageVersions"), + }); + span.setAttribute("missingPackages", missingPackages); + if (config.additionalPackages) { span.setAttribute("additionalPackages", config.additionalPackages); @@ -1770,12 +1777,10 @@ export async function resolveRequiredDependencies( dependencies[packageParts.name] = packageParts.version; continue; } else { - const externalDependencyVersion = await project.resolve(packageParts.name, { - allowDev: true, - }); + const dependencyVersion = dependencies[packageParts.name]; - if (externalDependencyVersion) { - dependencies[packageParts.name] = externalDependencyVersion; + if (dependencyVersion) { + dependencies[packageParts.name] = dependencyVersion; continue; } else { logger.log( diff --git a/packages/cli-v3/src/utilities/javascriptProject.ts b/packages/cli-v3/src/utilities/javascriptProject.ts index 1bbc2e60be..3a063a3d6d 100644 --- a/packages/cli-v3/src/utilities/javascriptProject.ts +++ b/packages/cli-v3/src/utilities/javascriptProject.ts @@ -1,16 +1,14 @@ -import { $, ExecaError } from "execa"; +import { $ } from "execa"; import { join } from "node:path"; import { readJSONFileSync } from "./fileSystem"; import { logger } from "./logger"; import { PackageManager, getUserPackageManager } from "./getUserPackageManager"; import { PackageJson } from "type-fest"; import { assertExhaustive } from "./assertExhaustive"; -import { builtinModules } from "node:module"; import { tracer } from "../cli/common"; import { recordSpanException } from "@trigger.dev/core/v3/otel"; import { flattenAttributes } from "@trigger.dev/core/v3"; -export type ResolveOptions = { allowDev: boolean }; export type DependencyMeta = { version: string; external: boolean }; export class JavascriptProject { @@ -106,10 +104,51 @@ export class JavascriptProject { }); try { - span.end(); - return await command.extractDirectDependenciesMeta({ + const packagesMeta = await command.extractDirectDependenciesMeta({ cwd: this.projectPath, }); + + // Merge the resolved versions with the package.json dependencies + const missingPackagesMeta = Object.entries(packagesMeta).filter( + ([, { version }]) => !version + ); + const missingPackageVersions: Record = {}; + + for (const [packageName, { external }] of missingPackagesMeta) { + const packageJsonVersion = this.packageJson.dependencies?.[packageName]; + + if (typeof packageJsonVersion === "string") { + logger.debug(`Resolved ${packageName} version using package.json`, { + packageJsonVersion, + }); + + packagesMeta[packageName] = { version: packageJsonVersion, external }; + missingPackageVersions[packageName] = packageJsonVersion; + } else { + // Last resort: check devDependencies + const devPackageJsonVersion = this.packageJson.devDependencies?.[packageName]; + + if (typeof devPackageJsonVersion === "string") { + logger.debug(`Resolved ${packageName} version using devDependencies`, { + devPackageJsonVersion, + }); + + packagesMeta[packageName] = { version: devPackageJsonVersion, external }; + missingPackageVersions[packageName] = devPackageJsonVersion; + } + } + } + + span.setAttributes({ + ...flattenAttributes(missingPackageVersions, "missingPackageVersions"), + missingPackages: missingPackagesMeta.map( + ([packageName]: [string, DependencyMeta]) => packageName + ), + }); + + span.end(); + + return packagesMeta; } catch (error) { recordSpanException(span, error); span.end(); @@ -124,117 +163,6 @@ export class JavascriptProject { ); } - async resolveAll(packageNames: string[]): Promise> { - return tracer.startActiveSpan("JavascriptProject.resolveAll", async (span) => { - const externalPackages = packageNames.filter((packageName) => !isBuiltInModule(packageName)); - - const command = await this.#getCommand(); - - span.setAttributes({ - externalPackages, - packageManager: command.name, - }); - - try { - const versions = await command.resolveDependencyVersions(externalPackages, { - cwd: this.projectPath, - }); - - if (versions) { - logger.debug(`Resolved [${externalPackages.join(", ")}] version using ${command.name}`, { - versions, - }); - - span.setAttributes({ - ...flattenAttributes(versions, "versions"), - }); - } - - // Merge the resolved versions with the package.json dependencies - const missingPackages = externalPackages.filter((packageName) => !versions[packageName]); - const missingPackageVersions: Record = {}; - - for (const packageName of missingPackages) { - const packageJsonVersion = this.packageJson.dependencies?.[packageName]; - - if (typeof packageJsonVersion === "string") { - logger.debug(`Resolved ${packageName} version using package.json`, { - packageJsonVersion, - }); - - missingPackageVersions[packageName] = packageJsonVersion; - } - } - - span.setAttributes({ - ...flattenAttributes(missingPackageVersions, "missingPackageVersions"), - missingPackages, - }); - - span.end(); - - return { ...versions, ...missingPackageVersions }; - } catch (error) { - recordSpanException(span, error); - span.end(); - - logger.debug(`Failed to resolve dependency versions using ${command.name}`, { - packageNames, - error, - }); - - return {}; - } - }); - } - - async resolve(packageName: string, options?: ResolveOptions): Promise { - if (isBuiltInModule(packageName)) { - return undefined; - } - - const opts = { allowDev: false, ...options }; - - const command = await this.#getCommand(); - - try { - const version = await command.resolveDependencyVersion(packageName, { - cwd: this.projectPath, - }); - - if (version) { - logger.debug(`Resolved ${packageName} version using ${command.name}`, { version }); - - return version; - } - - const packageJsonVersion = this.packageJson.dependencies?.[packageName]; - - if (typeof packageJsonVersion === "string") { - logger.debug(`Resolved ${packageName} version using package.json`, { packageJsonVersion }); - - return packageJsonVersion; - } - - if (opts.allowDev) { - const devPackageJsonVersion = this.packageJson.devDependencies?.[packageName]; - - if (typeof devPackageJsonVersion === "string") { - logger.debug(`Resolved ${packageName} version using devDependencies`, { - devPackageJsonVersion, - }); - - return devPackageJsonVersion; - } - } - } catch (error) { - logger.debug(`Failed to resolve dependency version using ${command.name}`, { - packageName, - error, - }); - } - } - async #getCommand(): Promise { const packageManager = await this.getPackageManager(); @@ -287,16 +215,6 @@ interface PackageManagerCommands { extractDirectDependenciesMeta( options: PackageManagerOptions ): Promise>; - - resolveDependencyVersion( - packageName: string, - options: PackageManagerOptions - ): Promise; - - resolveDependencyVersions( - packageNames: string[], - options: PackageManagerOptions - ): Promise>; } class PNPMCommands implements PackageManagerCommands { @@ -314,46 +232,6 @@ class PNPMCommands implements PackageManagerCommands { logger.debug(`Installing dependencies using ${this.name}`, { stdout, stderr }); } - async resolveDependencyVersion(packageName: string, options: PackageManagerOptions) { - const { stdout } = await $({ cwd: options.cwd })`${this.cmd} list ${packageName} -r --json`; - const result = JSON.parse(stdout) as PnpmList; - - logger.debug(`Resolving ${packageName} version using ${this.name}`); - - // Return the first dependency version that matches the package name - for (const dep of result) { - const dependency = dep.dependencies?.[packageName]; - - if (dependency) { - return dependency.version; - } - } - } - - async resolveDependencyVersions( - packageNames: string[], - options: PackageManagerOptions - ): Promise> { - const result = await this.#listDependencies(packageNames, options); - - logger.debug(`Resolving ${packageNames.join(" ")} version using ${this.name}`); - - const results: Record = {}; - - // Return the first dependency version that matches the package name - for (const dep of result) { - for (const packageName of packageNames) { - const dependency = dep.dependencies?.[packageName]; - - if (dependency) { - results[packageName] = dependency.version; - } - } - } - - return results; - } - async extractDirectDependenciesMeta(options: PackageManagerOptions) { const result = await this.#listDirectDependencies(options); @@ -393,21 +271,6 @@ class PNPMCommands implements PackageManagerCommands { return JSON.parse(childProcess.stdout) as PnpmList; } - - async #listDependencies(packageNames: string[], options: PackageManagerOptions) { - const childProcess = await $({ - cwd: options.cwd, - reject: false, - })`${this.cmd} list ${packageNames} -r --json`; - - if (childProcess.failed) { - logger.debug("Failed to list dependencies, using stdout anyway...", { - error: childProcess, - }); - } - - return JSON.parse(childProcess.stdout) as PnpmList; - } } type NpmDependency = { @@ -437,36 +300,6 @@ class NPMCommands implements PackageManagerCommands { logger.debug(`Installing dependencies using ${this.name}`, { stdout, stderr }); } - async resolveDependencyVersion(packageName: string, options: PackageManagerOptions) { - const { stdout } = await $({ cwd: options.cwd })`${this.cmd} list ${packageName} --json`; - const output = JSON.parse(stdout) as NpmListOutput; - - logger.debug(`Resolving ${packageName} version using ${this.name}`, { output }); - - return this.#recursivelySearchDependencies(output.dependencies, packageName); - } - - async resolveDependencyVersions( - packageNames: string[], - options: PackageManagerOptions - ): Promise> { - const output = await this.#listDependencies(packageNames, options); - - logger.debug(`Resolving ${packageNames.join(" ")} version using ${this.name}`, { output }); - - const results: Record = {}; - - for (const packageName of packageNames) { - const version = this.#recursivelySearchDependencies(output.dependencies, packageName); - - if (version) { - results[packageName] = version; - } - } - - return results; - } - async extractDirectDependenciesMeta( options: PackageManagerOptions ): Promise> { @@ -492,40 +325,6 @@ class NPMCommands implements PackageManagerCommands { return JSON.parse(childProcess.stdout) as NpmListOutput; } - async #listDependencies(packageNames: string[], options: PackageManagerOptions) { - const childProcess = await $({ - cwd: options.cwd, - reject: false, - })`${this.cmd} list ${packageNames} --json`; - - if (childProcess.failed) { - logger.debug("Failed to list dependencies, using stdout anyway...", { - error: childProcess, - }); - } - - return JSON.parse(childProcess.stdout) as NpmListOutput; - } - - #recursivelySearchDependencies( - dependencies: Record, - packageName: string - ): string | undefined { - for (const [name, dependency] of Object.entries(dependencies)) { - if (name === packageName) { - return dependency.version; - } - - if (dependency.dependencies) { - const result = this.#recursivelySearchDependencies(dependency.dependencies, packageName); - - if (result) { - return result; - } - } - } - } - #flattenDependenciesMeta( dependencies: Record ): Record { @@ -559,47 +358,6 @@ class YarnCommands implements PackageManagerCommands { logger.debug(`Installing dependencies using ${this.name}`, { stdout, stderr }); } - async resolveDependencyVersion(packageName: string, options: PackageManagerOptions) { - const { stdout } = await $({ cwd: options.cwd })`${this.cmd} info ${packageName} --json`; - - const lines = stdout.split("\n"); - - logger.debug(`Resolving ${packageName} version using ${this.name}`); - - for (const line of lines) { - const json = JSON.parse(line); - - if (json.value === packageName) { - return json.children.Version; - } - } - } - - async resolveDependencyVersions( - packageNames: string[], - options: PackageManagerOptions - ): Promise> { - const stdout = await this.#listDependencies(packageNames, options); - - const lines = stdout.split("\n"); - - logger.debug(`Resolving ${packageNames.join(" ")} version using ${this.name}`); - - const results: Record = {}; - - for (const line of lines) { - const json = JSON.parse(line); - - const packageName = this.#parseYarnValueIntoPackageName(json.value); - - if (packageNames.includes(packageName)) { - results[packageName] = json.children.Version; - } - } - - return results; - } - async extractDirectDependenciesMeta(options: PackageManagerOptions) { const result = await this.#listDirectDependencies(options); @@ -633,37 +391,10 @@ class YarnCommands implements PackageManagerCommands { return childProcess.stdout; } - async #listDependencies(packageNames: string[], options: PackageManagerOptions) { - const childProcess = await $({ - cwd: options.cwd, - reject: false, - })`${this.cmd} info ${packageNames} --json`; - - if (childProcess.failed) { - logger.debug("Failed to list dependencies, using stdout anyway...", { - error: childProcess, - }); - } - - return childProcess.stdout; - } - // The "value" when doing yarn info is formatted like this: // "package-name@npm:version" or "package-name@workspace:version" // This function will parse the value into just the package name. // This correctly handles scoped packages as well e.g. @scope/package-name@npm:version - #parseYarnValueIntoPackageName(value: string): string { - const parts = value.split("@"); - - // If the value does not contain an "@" symbol, then it's just the package name - if (parts.length === 3) { - return parts[1] as string; - } - - // If the value contains an "@" symbol, then the package name is the first part - return parts[0] as string; - } - #parseYarnValueIntoDependencyMeta(value: string): [string, DependencyMeta] { const parts = value.split("@"); let name: string, protocol: string, version: string; @@ -689,12 +420,3 @@ class YarnCommands implements PackageManagerCommands { ]; } } - -function isBuiltInModule(module: string): boolean { - // if the module has node: prefix, it's a built-in module - if (module.startsWith("node:")) { - return true; - } - - return builtinModules.includes(module); -}