Make start() types unknown when deploymentId is provided#1367
Open
Make start() types unknown when deploymentId is provided#1367
Conversation
Ensure types are 'unknown[]' and 'unknown' for 'deploymentId' and update exports and documentation. Slack-Thread: https://vercel.slack.com/archives/C09G3EQAL84/p1773368990070059?thread_ts=1773368990.070059&cid=C09G3EQAL84 Co-authored-by: Pranay Prakash <1797812+pranaygp@users.noreply.github.com>
|
Contributor
Contributor
📊 Benchmark Results
workflow with no steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 1 step💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express | Nitro workflow with 10 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro workflow with 25 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express | Nitro workflow with 50 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Nitro | Express Promise.all with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro Promise.all with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Promise.all with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Promise.race with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Promise.race with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Promise.race with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Stream Benchmarks (includes TTFB metrics)workflow with stream💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) SummaryFastest Framework by WorldWinner determined by most benchmark wins
Fastest World by FrameworkWinner determined by most benchmark wins
Column Definitions
Worlds:
|
Contributor
🧪 E2E Test Results❌ Some tests failed Summary
❌ Failed Tests🌍 Community Worlds (55 failed)mongodb (3 failed):
redis (2 failed):
turso (50 failed):
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
❌ 🌍 Community Worlds
✅ 📋 Other
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adjusts @workflow/core’s start() TypeScript API so that providing an explicit deploymentId no longer implies compile-time argument/return type compatibility with the local workflow code, avoiding misleading type inference when targeting other deployments.
Changes:
- Split
StartOptionsintoStartOptionsBase,StartOptionsWithDeploymentId, andStartOptionsWithoutDeploymentId. - Added
start()overloads sodeploymentIdcalls useunknown[]args and returnPromise<Run<unknown>>. - Exported the new option types from
packages/core/src/runtime.tsand documented theunknowntyping behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/core/src/runtime/start.ts | Refactors start option types and adds overloads to switch to unknown typing when deploymentId is provided. |
| packages/core/src/runtime.ts | Re-exports the newly split start option types. |
| docs/content/docs/api-reference/workflow-api/start.mdx | Documents that passing deploymentId makes args/return type unknown. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| */ | ||
| specVersion?: number; | ||
| export interface StartOptionsWithoutDeploymentId extends StartOptionsBase { | ||
| deploymentId?: undefined; |
Comment on lines
+83
to
+90
| export function start( | ||
| workflow: WorkflowFunction<unknown[], unknown> | WorkflowMetadata, | ||
| args: unknown[], | ||
| options: StartOptionsWithDeploymentId | ||
| ): Promise<Run<unknown>>; | ||
|
|
||
| export function start( | ||
| workflow: WorkflowFunction<unknown[], unknown> | WorkflowMetadata, |
| ): Promise<Run<unknown>>; | ||
|
|
||
| export function start( | ||
| workflow: WorkflowFunction<unknown[], unknown> | WorkflowMetadata, |
Collaborator
|
Claude flagged a blocker and some more comments: PR #1367 Review: Make start() types unknown when deploymentId is providedIntent: When BlockerOverload with The // (from the PR diff - new overloads)
export function start(
workflow: WorkflowFunction<unknown[], unknown> | WorkflowMetadata,
args: unknown[],
options: StartOptionsWithDeploymentId
): Promise<Run<unknown>>;
export function start(
workflow: WorkflowFunction<unknown[], unknown> | WorkflowMetadata,
options: StartOptionsWithDeploymentId
): Promise<Run<unknown>>;Due to TypeScript's function parameter contravariance under import { myWorkflow } from './workflows/my-workflow'; // WorkflowFunction<[string], number>
start(myWorkflow, ['hello'], { deploymentId: 'dpl_xxx' }); // TYPE ERRORThis defeats the purpose of the PR since the main use case is calling export function start<TArgs extends unknown[], TResult>(
workflow: WorkflowFunction<TArgs, TResult> | WorkflowMetadata,
args: unknown[],
options: StartOptionsWithDeploymentId
): Promise<Run<unknown>>;(Copilot's review comment #2 flagged the same issue.) Potential Regressions
Minor Issues
VerdictThe contravariance issue with the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When
deploymentIdis explicitly provided tostart(), the workflow function at that deployment may have different argument and return types than the current codebase. This change splitsStartOptionsinto separate types and adds function overloads so that:deploymentId: Full type inference is preserved (TArgsandTResult)deploymentId: Args becomeunknown[]and return becomesRun<unknown>to avoid false type expectationsWhat changed
StartOptionsintoStartOptionsBase,StartOptionsWithDeploymentId, andStartOptionsWithoutDeploymentIdstart()that returnRun<unknown>whendeploymentIdis providedruntime.tsdeploymentId