Skip to content

[chore]: pull stringified JS into TS file#1622

Merged
seanmcguire12 merged 3 commits intomainfrom
refactor-screenshot-utils
Jan 27, 2026
Merged

[chore]: pull stringified JS into TS file#1622
seanmcguire12 merged 3 commits intomainfrom
refactor-screenshot-utils

Conversation

@seanmcguire12
Copy link
Member

@seanmcguire12 seanmcguire12 commented Jan 27, 2026

why

  • stringified JS is hard to debug & not good practice

what changed

  • pulled stringified JS from screenshotUtils.ts into its own resolveMaskRect.ts file
  • added a generator to compile screenshot scripts into function strings: packages/core/lib/v3/dom/genScreenshotScripts.ts

test plan

  • existing tests should cover this

Summary by cubic

Replace stringified screenshot DOM code with typed TS functions and generate function strings at build time. This makes screenshot scripts easier to maintain and debug.

  • Refactors
    • Extracted resolveMaskRect into lib/v3/dom/screenshotScripts/resolveMaskRect.ts with types.
    • Added genScreenshotScripts.ts to bundle TS scripts and emit build/screenshotScripts.generated.ts.
    • Updated screenshotUtils to use screenshotScriptSources.resolveMaskRect; included generator in build-dom-scripts.

Written for commit 8ce9a91. Summary will update on new commits. Review in cubic

@changeset-bot
Copy link

changeset-bot bot commented Jan 27, 2026

⚠️ No Changeset found

Latest commit: 8ce9a91

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 5 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.
Architecture diagram
sequenceDiagram
    participant Build as Build Script<br/>(genScreenshotScripts.ts)
    participant Source as Source Code<br/>(resolveMaskRect.ts)
    participant Artifact as Generated Output<br/>(screenshotScripts.generated.ts)
    participant Runtime as ScreenshotUtils<br/>(Node.js)
    participant Browser as Browser Page<br/>(Chrome DevTools Protocol)

    Note over Build,Artifact: NEW: Build-Time Compilation Phase
    Build->>Source: Read TypeScript source
    Build->>Build: Bundle, Minify (esbuild), & Serialize
    Build->>Artifact: Write "screenshotScriptSources"<br/>(Contains stringified functions)

    Note over Runtime,Browser: Run-Time Execution Phase
    Runtime->>Artifact: Import screenshotScriptSources
    Note right of Runtime: CHANGED: Function string is now<br/>imported vs hardcoded
    
    Runtime->>Browser: CDP: Runtime.callFunctionOn
    Note right of Runtime: Payload includes:<br/>functionDeclaration: screenshotScriptSources.resolveMaskRect
    
    Browser->>Browser: Execute injected JS inside DOM context
    Browser-->>Runtime: Return {x, y, width, height}
Loading

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 27, 2026

Greptile Overview

Greptile Summary

Extracted stringified JavaScript from screenshotUtils.ts into a properly typed TypeScript file with build-time compilation. The refactoring follows established patterns from genLocatorScripts.ts and genA11yScripts.ts, using esbuild to bundle and minify the script, then converting to function strings via .toString().

Key changes:

  • Created resolveMaskRect.ts with proper TypeScript types (this: Element | null, MaskRect)
  • Added genScreenshotScripts.ts generator following existing conventions
  • Updated screenshotUtils.ts to use screenshotScriptSources.resolveMaskRect instead of inline string
  • Integrated generator into build-dom-scripts pipeline
  • Generated output (build/screenshotScripts.generated.ts) is properly gitignored

The approach preserves this binding required for CDP's Runtime.callFunctionOn, matches patterns already proven to work in production, and improves debuggability and maintainability.

Confidence Score: 5/5

  • Safe to merge - well-executed refactoring following established patterns with no functional changes
  • Code follows proven patterns from existing generators (genLocatorScripts, genA11yScripts), preserves exact functionality, includes proper TypeScript typing, and integrates correctly into build pipeline
  • No files require special attention

Important Files Changed

Filename Overview
packages/core/lib/v3/dom/genScreenshotScripts.ts Added generator script following existing patterns from genLocatorScripts and genA11yScripts; bundles screenshot scripts and exports function strings
packages/core/lib/v3/dom/screenshotScripts/resolveMaskRect.ts Extracted inline stringified function to typed TS file with proper `this: Element
packages/core/lib/v3/understudy/screenshotUtils.ts Replaced inline stringified function with generated script source from screenshotScriptSources.resolveMaskRect; functionality preserved
packages/core/package.json Added genScreenshotScripts.ts to build-dom-scripts pipeline before genA11yScripts.ts

Sequence Diagram

sequenceDiagram
    participant Dev as Developer
    participant Build as Build Process
    participant Gen as genScreenshotScripts.ts
    participant ESB as esbuild
    participant Src as screenshotScripts/
    participant Out as build/screenshotScripts.generated.ts
    participant Utils as screenshotUtils.ts
    participant CDP as Chrome DevTools Protocol

    Dev->>Build: npm run build-dom-scripts
    Build->>Gen: tsx genScreenshotScripts.ts
    Gen->>ESB: Bundle screenshotScripts/index.ts
    ESB->>Src: Read resolveMaskRect.ts
    ESB-->>Gen: Return minified module
    Gen->>Gen: Import compiled module
    Gen->>Gen: Extract functions & call .toString()
    Gen->>Out: Write screenshotScriptSources object
    
    Note over Utils,CDP: Runtime Usage
    Utils->>Out: Import screenshotScriptSources
    Utils->>CDP: callFunctionOn with functionDeclaration
    CDP->>CDP: Execute function string in browser context
    CDP-->>Utils: Return result
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

No files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@seanmcguire12 seanmcguire12 merged commit 1abcf73 into main Jan 27, 2026
28 checks passed
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.

2 participants