Skip to content

refactor: replace overloads with generics for streamable methods#268

Open
majiayu000 wants to merge 1 commit intoollama:mainfrom
majiayu000:fix/replace-overloads-with-generics
Open

refactor: replace overloads with generics for streamable methods#268
majiayu000 wants to merge 1 commit intoollama:mainfrom
majiayu000:fix/replace-overloads-with-generics

Conversation

@majiayu000
Copy link
Copy Markdown
Contributor

Summary

  • Replace function overloads with generic type parameters for streamable methods
  • Allows stream parameter to accept boolean type while preserving correct return type inference
  • Methods updated: generate, chat, create, pull, push

Problem

When using overloads, passing a boolean variable as the stream parameter caused TypeScript errors due to ambiguity in overload resolution:

// This caused type errors before
const generate = (request: GenerateRequest, stream: boolean) => {
  return ollama.generate({ ...request, stream })
}

Solution

Replace overloads with a generic type parameter that defaults to false:

async generate<S extends boolean = false>(
  request: GenerateRequest & { stream?: S },
): Promise<S extends true ? AbortableAsyncIterator<GenerateResponse> : GenerateResponse>

Test plan

  • TypeScript compilation succeeds
  • Existing tests pass
  • New tests verify generic typing works correctly

Fixes #78

Replace function overloads with generic type parameters for methods
that accept a stream parameter. This allows the stream parameter to
be typed as boolean while still providing correct return type inference.

Methods updated:
- generate
- chat
- create
- pull
- push

Fixes ollama#78

Signed-off-by: majiayu000 <1835304752@qq.com>
Copy link
Copy Markdown
Member

@BruceMacD BruceMacD left a comment

Choose a reason for hiding this comment

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

This is looking good, thanks for the contribution. I'll test a bit more once the conflict is resolved.

We should note that this will effect any clients that extend ollama-js for some reason, although I am not aware of anyone using the library like that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Overloads should be replaced with generics.

2 participants