Skip to content

Ashwin/fix strict ts errors #937

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

Draft
wants to merge 20 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
d4e7f2d
Updated pnpm-lock.yaml
MrHaikuSwan Jul 30, 2025
56ca7a7
Enabled strict: true in tsconfig.json
MrHaikuSwan Jul 30, 2025
44c472e
Fixed TS errors in lib/index.ts, added notes
MrHaikuSwan Jul 30, 2025
fd6c2d7
Used guarantee that StagehandPage will always receive an LLMClient (t…
MrHaikuSwan Jul 31, 2025
d250aa3
Fixes all but one TS error in StagehandPage.ts
MrHaikuSwan Jul 31, 2025
0dcaf0c
Fixed all TS errors in observeHandler.ts
MrHaikuSwan Jul 31, 2025
b8a81a9
Fixed most TS issues in api.ts
MrHaikuSwan Jul 31, 2025
3ec67ea
Fixed TS errors in LLMClient.ts by marking type, hasVision, and clien…
MrHaikuSwan Jul 31, 2025
e8722e8
Updated logger type to LogLine in LLMCache.ts
MrHaikuSwan Jul 31, 2025
54fa3f7
Fixed TS typing issues in LLMProvider.ts
MrHaikuSwan Aug 1, 2025
2c2ccb5
Fixed some TS errors in extractHandler.ts
MrHaikuSwan Aug 1, 2025
c2d02af
Implemented overloaded function signatures for extract with more spec…
MrHaikuSwan Aug 1, 2025
0a4d127
Added note to ExtractOptions interface definition and added tempfix +…
MrHaikuSwan Aug 1, 2025
650addd
Removed Zod type inference for ObserveResponse to define a more speci…
MrHaikuSwan Aug 1, 2025
2a9d633
Left note about remaining skipped TS errors in inference.ts
MrHaikuSwan Aug 1, 2025
9ce37fb
Fixed TS errors in actHandler.ts
MrHaikuSwan Aug 1, 2025
b6c9df3
Refactored unnecessary Promise.all on async callbacks in observeHandl…
MrHaikuSwan Aug 1, 2025
23ed7c1
Fixed a few TS errors in operatorHandler.ts
MrHaikuSwan Aug 1, 2025
aa706d0
Fixed some TS errors in utils.ts
MrHaikuSwan Aug 1, 2025
e36891b
Added TS tempfixes to api.ts, these definitely should be removed
MrHaikuSwan Aug 1, 2025
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
96 changes: 58 additions & 38 deletions lib/StagehandPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export class StagehandPage {
private observeHandler: StagehandObserveHandler;
private llmClient: LLMClient;
private cdpClient: CDPSession | null = null;
private api: StagehandAPI;
private api?: StagehandAPI;
private userProvidedInstructions?: string;
private waitForCaptchaSolves: boolean;
private initialized: boolean = false;
Expand Down Expand Up @@ -101,7 +101,8 @@ export class StagehandPage {
const value = target[prop];
// If the property is a function, wrap it to update active page before execution
if (typeof value === "function" && prop !== "on") {
return (...args: unknown[]) => value.apply(target, args);
return (...args: unknown[]) =>
(value as (...a: unknown[]) => unknown).apply(target, args);
}
return value;
},
Expand All @@ -114,25 +115,23 @@ export class StagehandPage {
this.userProvidedInstructions = userProvidedInstructions;
this.waitForCaptchaSolves = waitForCaptchaSolves ?? false;

if (this.llmClient) {
this.actHandler = new StagehandActHandler({
logger: this.stagehand.logger,
stagehandPage: this,
selfHeal: this.stagehand.selfHeal,
});
this.extractHandler = new StagehandExtractHandler({
stagehand: this.stagehand,
logger: this.stagehand.logger,
stagehandPage: this,
userProvidedInstructions,
});
this.observeHandler = new StagehandObserveHandler({
stagehand: this.stagehand,
logger: this.stagehand.logger,
stagehandPage: this,
userProvidedInstructions,
});
}
this.actHandler = new StagehandActHandler({
logger: this.stagehand.logger,
stagehandPage: this,
selfHeal: this.stagehand.selfHeal,
});
this.extractHandler = new StagehandExtractHandler({
stagehand: this.stagehand,
logger: this.stagehand.logger,
stagehandPage: this,
userProvidedInstructions,
});
this.observeHandler = new StagehandObserveHandler({
stagehand: this.stagehand,
logger: this.stagehand.logger,
stagehandPage: this,
userProvidedInstructions,
});
}

public ordinalForFrameId(fid: string | undefined): number {
Expand Down Expand Up @@ -180,7 +179,10 @@ ${scriptContent} \
level: 1,
auxiliary: {
error: { value: (err as Error).message, type: "string" },
trace: { value: (err as Error).stack, type: "string" },
trace: {
value: (err as Error).stack ?? "No stack trace available",
type: "string",
},
},
});
throw err;
Expand Down Expand Up @@ -280,17 +282,18 @@ ${scriptContent} \
}

// Use type assertion to safely call the method with proper typing
type EnhancedOptions =
| ActOptions
| ExtractOptions<z.AnyZodObject>
| ObserveOptions;
type EnhancedMethod = (
options:
| ActOptions
| ExtractOptions<z.AnyZodObject>
| ObserveOptions,
options: EnhancedOptions,
) => Promise<
ActResult | ExtractResult<z.AnyZodObject> | ObserveResult[]
>;

const method = this[prop as keyof StagehandPage] as EnhancedMethod;
return (options: unknown) => method.call(this, options);
return (options: EnhancedOptions) => method.call(this, options);
}

// Handle screenshots with CDP
Expand Down Expand Up @@ -395,7 +398,8 @@ ${scriptContent} \

// For all other method calls, update active page
if (typeof value === "function") {
return (...args: unknown[]) => value.apply(target, args);
return (...args: unknown[]) =>
(value as (...a: unknown[]) => unknown).apply(target, args);
}

return value;
Expand Down Expand Up @@ -702,34 +706,42 @@ ${scriptContent} \

await clearOverlays(this.page);

// check if user called extract() with no arguments
if (!instructionOrOptions) {
// check if user called extract() with no arguments or empty string
// NOTE: This explicitly matches the old behavior of early exiting on empty string,
// but it might make more sense to only early exit on undefined.
if (instructionOrOptions === undefined || instructionOrOptions === "") {
let result: ExtractResult<T>;
if (this.api) {
result = await this.api.extract<T>({ frameId: this.rootFrameId });
} else {
result = await this.extractHandler.extract();
}
this.stagehand.addToHistory("extract", instructionOrOptions, result);
// NOTE: This always pushes empty string into the history because of `addToHistory`'s
// type signature, feel free to change this to something else.
this.stagehand.addToHistory("extract", "", result);
return result;
}

const options: ExtractOptions<T> =
typeof instructionOrOptions === "string"
? {
instruction: instructionOrOptions,
schema: defaultExtractSchema as T,
schema: defaultExtractSchema as unknown as T,
}
: instructionOrOptions.schema
? instructionOrOptions
: {
...instructionOrOptions,
schema: defaultExtractSchema as T,
schema: defaultExtractSchema as unknown as T,
};

// NOTE: The current code early exits on empty string, but proceeds with { instruction: "" }
// This is a bit confusing, but it's consistent with the old behavior.
// NOTE: instruction and schema should be defined at this point as per the docs, see ExtractOptions interface definition
// The below default values should be removed in the future
const {
instruction,
schema,
instruction = "",
schema = defaultExtractSchema,
modelName,
modelClientOptions,
domSettleTimeoutMs,
Expand Down Expand Up @@ -832,7 +844,7 @@ ${scriptContent} \
: instructionOrOptions || {};

const {
instruction,
instruction = "",
modelName,
modelClientOptions,
domSettleTimeoutMs,
Expand All @@ -845,7 +857,11 @@ ${scriptContent} \
if (this.api) {
const opts = { ...options, frameId: this.rootFrameId };
const result = await this.api.observe(opts);
this.stagehand.addToHistory("observe", instructionOrOptions, result);
this.stagehand.addToHistory(
"observe",
instructionOrOptions ?? "",
result,
);
return result;
}

Expand Down Expand Up @@ -923,7 +939,11 @@ ${scriptContent} \
throw e;
});

this.stagehand.addToHistory("observe", instructionOrOptions, result);
this.stagehand.addToHistory(
"observe",
instructionOrOptions ?? "",
result,
);

return result;
} catch (err: unknown) {
Expand Down
9 changes: 7 additions & 2 deletions lib/a11y/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,11 @@ export async function buildBackendIdMaps(
xpathMap[enc] = path;

// recurse into sub-document if <iframe>
if (lc(node.nodeName) === "iframe" && node.contentDocument) {
if (
node.nodeName &&
lc(node.nodeName) === "iframe" &&
node.contentDocument
) {
const childFid = node.contentDocument.frameId ?? fid;
stack.push({ node: node.contentDocument, path: "", fid: childFid });
}
Expand Down Expand Up @@ -263,7 +267,8 @@ async function cleanStructuralNodes(
logger?: (l: LogLine) => void,
): Promise<AccessibilityNode | null> {
// 0. ignore negative pseudo-nodes
if (+node.nodeId < 0) return null;
// NOTE: Nodes with non-numeric node.nodeId won't be ignored: Number(node.nodeId) returns NaN, and NaN < 0 is false
if (!node.nodeId || Number(node.nodeId) < 0) return null;

// 1. leaf check
if (!node.children?.length) {
Expand Down
23 changes: 20 additions & 3 deletions lib/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export class StagehandAPI {
private apiKey: string;
private projectId: string;
private sessionId?: string;
private modelApiKey: string;
private modelApiKey?: string;
private logger: (message: LogLine) => void;
private fetchWithCookies;

Expand Down Expand Up @@ -65,8 +65,11 @@ export class StagehandAPI {
this.modelApiKey = modelApiKey;

const region = browserbaseSessionCreateParams?.region;
// NOTE: Looks like there was some discussion here -- I personally agree that this should throw
// Thread: https://github.com/browserbase/stagehand/pull/801#discussion_r2138856174
// I'm changing the fallback value to "" to make TS happy for now, but this probably shouldn't be the case
if (region && region !== "us-west-2") {
return { sessionId: browserbaseSessionID ?? null, available: false };
return { sessionId: browserbaseSessionID ?? "", available: false };
}
const sessionResponse = await this.request("/sessions/start", {
method: "POST",
Expand Down Expand Up @@ -165,6 +168,8 @@ export class StagehandAPI {
return response;
}

// NOTE: This is a strange way to process SSE, and also doesn't guarantee that it returns T (actually explicitly returns null in one case)
// I feel pretty confused by this, so I'll leave this here to refactor later
private async execute<T>({
method,
args,
Expand Down Expand Up @@ -197,8 +202,9 @@ export class StagehandAPI {
while (true) {
const { value, done } = await reader.read();

// NOTE: This is a tempfix to make TS happy, but needing this is a code smell
if (done && !buffer) {
return null;
return null as T;
}

buffer += decoder.decode(value, { stream: true });
Expand Down Expand Up @@ -231,12 +237,23 @@ export class StagehandAPI {

if (done) break;
}

// NOTE: This is a tempfix to make TS happy, but needing this is a code smell
return null as T;
}

private async request(
path: string,
options: RequestInit = {},
): Promise<Response> {
// These must be set for a request to be made successfully
if (!this.modelApiKey) {
throw new StagehandAPIError("modelApiKey is required");
}
if (!this.sessionId) {
throw new StagehandAPIError("sessionId is required");
}

const defaultHeaders: Record<string, string> = {
"x-bb-api-key": this.apiKey,
"x-bb-project-id": this.projectId,
Expand Down
7 changes: 2 additions & 5 deletions lib/cache/LLMCache.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
import { LogLine } from "../../types/log";
import { BaseCache, CacheEntry } from "./BaseCache";

export class LLMCache extends BaseCache<CacheEntry> {
constructor(
logger: (message: {
category?: string;
message: string;
level?: number;
}) => void,
logger: (message: LogLine) => void,
cacheDir?: string,
cacheFile?: string,
) {
Expand Down
Loading