-
Notifications
You must be signed in to change notification settings - Fork 96
Make @standard-schema/spec be a regular dependency
#231
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
Conversation
The Standard Schema docs explicitly say that it should be a regular dep, not a dev dep: https://standardschema.dev/#can-i-add-it-as-a-dev-dependency
🦋 Changeset detectedLatest commit: eca41b9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
VaguelySerious
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving since it seems harmless, but we're not actually using @standard-schema/spec anywhere. What's the point of having it as a dependency?
|
We are using it. See 5c0268b |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional Suggestion:
The hard-coded TypeScript interface in the documentation still shows a single type parameter DefineHook<T>, but the actual function implementation now supports two type parameters defineHook<TInput, TOutput = TInput>. This creates a mismatch between the documented and actual API.
View Details
📝 Patch Details
diff --git a/docs/content/docs/api-reference/workflow/define-hook.mdx b/docs/content/docs/api-reference/workflow/define-hook.mdx
index 56e8a07..c8a273c 100644
--- a/docs/content/docs/api-reference/workflow/define-hook.mdx
+++ b/docs/content/docs/api-reference/workflow/define-hook.mdx
@@ -48,18 +48,18 @@ showSections={['parameters']}
<TSDoc
definition={generateDefinition({
code: `
-interface DefineHook<T> {
+interface DefineHook<TInput, TOutput = TInput> {
/**
-* Creates a new hook with the defined payload type.
+* Creates a new hook with the defined output type.
*/
- create: (options?: HookOptions) => Hook<T>;
+ create: (options?: HookOptions) => Hook<TOutput>;
/**
-* Resumes a hook by sending a payload with the defined type.
+* Resumes a hook by sending a payload with the defined input type.
*/
- resume: (token: string, payload: T) => Promise<HookEntity | null>;
+ resume: (token: string, payload: TInput) => Promise<HookEntity | null>;
}
export default DefineHook;`
})}
Analysis
Documentation shows outdated TypeScript interface for defineHook
What fails: The documentation in docs/content/docs/api-reference/workflow/define-hook.mdx (lines 51-62) displays an incorrect DefineHook interface signature that doesn't match the actual implementation.
How to reproduce:
- View the documentation at
docs/content/docs/api-reference/workflow/define-hook.mdxlines 51-62 - Compare the documented interface with the actual implementation in
packages/core/src/define-hook.ts - Observe the type parameter mismatch
Expected vs Actual:
Documentation shows:
interface DefineHook<T> {
create: (options?: HookOptions) => Hook<T>;
resume: (token: string, payload: T) => Promise<HookEntity | null>;
}Actual implementation signature:
export function defineHook<TInput, TOutput = TInput>({schema}: {schema?: StandardSchemaV1<TInput, TOutput>} = {}) {
return {
create(_options?: HookOptions): Hook<TOutput> { ... }
resume(token: string, payload: TInput): Promise<HookEntity | null> { ... }
}
}The actual API supports schema-based transformations where input and output types can differ (via StandardSchemaV1<TInput, TOutput>). This is demonstrated in the code's own example at lines 73-100 where a Zod schema transforms the input payload.
Root cause: The documentation interface was not updated when the defineHook function signature evolved to support separate input and output type parameters.
|
Ah, was looking at an outdated branch, LGTM |
|
re: Vade's comment - I think we actually do want to document it as only one generic because if the user is typing the hook manually (without a schema) then there would be no transformation and the input = the output. |
The Standard Schema docs explicitly say that it should be a regular dep, not a dev dep: https://standardschema.dev/#can-i-add-it-as-a-dev-dependency