Skip to content

Improve the update CLI command and fix missing tsconfig.json error #1315

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

Merged
merged 5 commits into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/clever-buses-watch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"trigger.dev": patch
"@trigger.dev/core": patch
---

Fix an issue where a missing tsconfig.json file would throw an error on dev/deploy
5 changes: 5 additions & 0 deletions .changeset/soft-ladybugs-promise.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"trigger.dev": patch
---

Fixes for CLI update command, and make the hide the "whoami" command output when running in dev.
5 changes: 5 additions & 0 deletions .changeset/twelve-onions-decide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@trigger.dev/build": patch
---

Strip out TRIGGER\_ keys when using syncEnvVars, to prevent deploy errors
7 changes: 7 additions & 0 deletions packages/build/src/extensions/core/syncEnvVars.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ const UNSYNCABLE_ENV_VARS = [
"_",
];

const UNSYNCABLE_ENV_VARS_PREFIXES = ["TRIGGER_"];

export type SyncEnvVarsFunction = (params: SyncEnvVarsParams) => SyncEnvVarsResult;

export type SyncEnvVarsOptions = {
Expand Down Expand Up @@ -98,6 +100,11 @@ export function syncEnvVars(fn: SyncEnvVarsFunction, options?: SyncEnvVarsOption
return acc;
}

// Strip out any TRIGGER_ prefix env vars
if (UNSYNCABLE_ENV_VARS_PREFIXES.some((prefix) => key.startsWith(prefix))) {
return acc;
}

acc[key] = value;
return acc;
},
Expand Down
1 change: 1 addition & 0 deletions packages/cli-v3/src/commands/dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export async function devCommand(options: DevCommandOptions) {

const authorization = await login({
embedded: true,
silent: true,
defaultApiUrl: options.apiUrl,
profile: options.profile,
});
Expand Down
11 changes: 9 additions & 2 deletions packages/cli-v3/src/commands/login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,18 @@ export type LoginOptions = {
defaultApiUrl?: string;
embedded?: boolean;
profile?: string;
silent?: boolean;
};

export async function login(options?: LoginOptions): Promise<LoginResult> {
return await tracer.startActiveSpan("login", async (span) => {
try {
const opts = { defaultApiUrl: "https://api.trigger.dev", embedded: false, ...options };
const opts = {
defaultApiUrl: "https://api.trigger.dev",
embedded: false,
silent: false,
...options,
};

span.setAttributes({
"cli.config.apiUrl": opts.defaultApiUrl,
Expand Down Expand Up @@ -111,7 +117,8 @@ export async function login(options?: LoginOptions): Promise<LoginResult> {
skipTelemetry: !span.isRecording(),
logLevel: logger.loggerLevel,
},
true
true,
opts.silent
);

if (!whoAmIResult.success) {
Expand Down
65 changes: 40 additions & 25 deletions packages/cli-v3/src/commands/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export function configureUpdateCommand(program: Command) {
const triggerPackageFilter = /^@trigger\.dev/;

export async function updateCommand(dir: string, options: UpdateCommandOptions) {
await updateTriggerPackages(dir, options);
await updateTriggerPackages(dir, options, false);
}

export async function updateTriggerPackages(
Expand Down Expand Up @@ -74,7 +74,7 @@ export async function updateTriggerPackages(

const newCliVersion = await updateCheck();

if (newCliVersion) {
if (newCliVersion && !cliVersion.startsWith("0.0.0")) {
prettyWarning(
"You're not running the latest CLI version, please consider updating ASAP",
`Current: ${cliVersion}\nLatest: ${newCliVersion}`,
Expand Down Expand Up @@ -127,24 +127,30 @@ export async function updateTriggerPackages(

if (mismatches.length === 0) {
if (!embedded) {
outro(`Nothing to do${newCliVersion ? " ..but you should really update your CLI!" : ""}`);
outro(`Nothing to update${newCliVersion ? " ..but you should really update your CLI!" : ""}`);
return hasOutput;
}
return hasOutput;
}

if (isDowngrade) {
prettyError("Some of the installed @trigger.dev packages are newer than your CLI version");
} else {
prettyWarning(
"Mismatch between your CLI version and installed packages",
"We recommend pinned versions for guaranteed compatibility"
);
if (embedded) {
if (isDowngrade) {
prettyError("Some of the installed @trigger.dev packages are newer than your CLI version");
} else {
if (embedded) {
prettyWarning(
"Mismatch between your CLI version and installed packages",
"We recommend pinned versions for guaranteed compatibility"
);
}
}
}

if (!hasTTY) {
// Running in CI with version mismatch detected
outro("Deploy failed");
if (embedded) {
outro("Deploy failed");
}

console.log(
`ERROR: Version mismatch detected while running in CI. This won't end well. Aborting.
Expand All @@ -162,8 +168,7 @@ export async function updateTriggerPackages(
}

// WARNING: We can only start accepting user input once we know this is a TTY, otherwise, the process will exit with an error in CI

if (isDowngrade) {
if (isDowngrade && embedded) {
printUpdateTable("Versions", mismatches, cliVersion, "installed", "CLI");

outro("CLI update required!");
Expand All @@ -187,14 +192,20 @@ export async function updateTriggerPackages(

if (!userWantsToUpdate) {
if (requireUpdate) {
outro("You shall not pass!");

logger.log(
`${chalkError(
"X Error:"
)} Update required: Version mismatches are a common source of bugs and errors. Please update or use \`--skip-update-check\` at your own risk.\n`
);
process.exit(1);
if (embedded) {
outro("You shall not pass!");

logger.log(
`${chalkError(
"X Error:"
)} Update required: Version mismatches are a common source of bugs and errors. Please update or use \`--skip-update-check\` at your own risk.\n`
);
process.exit(1);
} else {
outro("No updates applied");

process.exit(0);
}
}

if (!embedded) {
Expand All @@ -205,7 +216,7 @@ export async function updateTriggerPackages(
}

const installSpinner = spinner();
installSpinner.start("Writing new package.json file");
installSpinner.start("Updating dependencies in package.json");

// Backup package.json
const packageJsonBackupPath = `${packageJsonPath}.bak`;
Expand Down Expand Up @@ -235,12 +246,16 @@ export async function updateTriggerPackages(
const packageManager = await detectPackageManager(projectPath);

try {
installSpinner.message(`Installing new package versions with ${packageManager}`);
installSpinner.message(
`Installing new package versions${packageManager ? ` with ${packageManager.name}` : ""}`
);

await installDependencies({ cwd: projectPath });
await installDependencies({ cwd: projectPath, silent: true });
} catch (error) {
installSpinner.stop(
`Failed to install new package versions${packageManager ? ` with ${packageManager}` : ""}`
`Failed to install new package versions${
packageManager ? ` with ${packageManager.name}` : ""
}`
);

// Remove exit handler in case of failure
Expand Down
23 changes: 14 additions & 9 deletions packages/cli-v3/src/commands/whoami.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,27 +51,32 @@ export async function whoAmICommand(options: unknown) {

export async function whoAmI(
options?: WhoamiCommandOptions,
embedded: boolean = false
embedded: boolean = false,
silent: boolean = false
): Promise<WhoAmIResult> {
if (!embedded) {
intro(`Displaying your account details [${options?.profile ?? "default"}]`);
}

const loadingSpinner = spinner();
loadingSpinner.start("Checking your account details");

if (!silent) {
loadingSpinner.start("Checking your account details");
}

const authentication = await isLoggedIn(options?.profile);

if (!authentication.ok) {
if (authentication.error === "fetch failed") {
loadingSpinner.stop("Fetch failed. Platform down?");
!silent && loadingSpinner.stop("Fetch failed. Platform down?");
} else {
if (embedded) {
loadingSpinner.stop(
`Failed to check account details. You may want to run \`trigger.dev logout --profile ${
options?.profile ?? "default"
}\` and try again.`
);
!silent &&
loadingSpinner.stop(
`Failed to check account details. You may want to run \`trigger.dev logout --profile ${
options?.profile ?? "default"
}\` and try again.`
);
} else {
loadingSpinner.stop(
`You must login first. Use \`trigger.dev login --profile ${
Expand Down Expand Up @@ -110,7 +115,7 @@ URL: ${chalkLink(authentication.auth.apiUrl)}
`Account details [${authentication.profile}]`
);
} else {
loadingSpinner.stop(`Retrieved your account details for ${userData.data.email}`);
!silent && loadingSpinner.stop(`Retrieved your account details for ${userData.data.email}`);
}

return userData;
Expand Down
12 changes: 10 additions & 2 deletions packages/cli-v3/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ async function resolveConfig(
warn = true
): Promise<ResolvedConfig> {
const packageJsonPath = await resolvePackageJSON(cwd);
const tsconfigPath = await resolveTSConfig(cwd);
const tsconfigPath = await safeResolveTsConfig(cwd);
const lockfilePath = await resolveLockfile(cwd);
const workspaceDir = await findWorkspaceDir(cwd);

Expand Down Expand Up @@ -179,7 +179,7 @@ async function resolveConfig(
conditions: [],
},
}
);
) as ResolvedConfig; // TODO: For some reason, without this, there is a weird type error complaining about tsconfigPath being string | nullish, which can't be assigned to string | undefined
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Investigate the root cause of the type error instead of using a type assertion.

The type assertion is used to suppress a type error related to the tsconfigPath property. However, it's better to investigate and fix the root cause of the type mismatch instead of relying on a type assertion.

Please consider the following:

  • Analyze why tsconfigPath is being treated as string | nullish instead of string | undefined.
  • Ensure that the type definitions for tsconfigPath are consistent throughout the codebase.
  • Consider updating the type definitions or the logic that assigns the value to tsconfigPath to resolve the type mismatch.

Using a type assertion may hide potential issues and make the code less maintainable in the long run.


return {
...mergedConfig,
Expand All @@ -188,6 +188,14 @@ async function resolveConfig(
};
}

async function safeResolveTsConfig(cwd: string) {
try {
return await resolveTSConfig(cwd);
} catch {
return undefined;
}
}

const IGNORED_DIRS = ["node_modules", ".git", "dist", "out", "build"];

async function autoDetectDirs(workingDir: string): Promise<string[]> {
Expand Down
2 changes: 1 addition & 1 deletion packages/cli-v3/src/utilities/sourceFiles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export async function resolveFileSources(
}

await resolveConfigSource(sources, resolvedConfig.workingDir, resolvedConfig.configFile);
await resolveConfigSource(sources, resolvedConfig.workingDir, resolvedConfig.tsconfig);
await resolveConfigSource(sources, resolvedConfig.workingDir, resolvedConfig.tsconfigPath);
await resolveConfigSource(sources, resolvedConfig.workingDir, resolvedConfig.packageJsonPath);

return sources;
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/v3/build/resolvedConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export type ResolvedConfig = Prettify<
packageJsonPath: string;
lockfilePath: string;
configFile?: string;
tsconfigPath?: string;
resolveEnvVars?: ResolveEnvironmentVariablesFunction;
instrumentedPackageNames?: string[];
}
Expand Down
Loading