Skip to content

feat(core): export runtime start resolver with documentRef parameter#2502

Merged
vanceingalls merged 3 commits into
mainfrom
task4-start-resolver-export
Jul 17, 2026
Merged

feat(core): export runtime start resolver with documentRef parameter#2502
vanceingalls merged 3 commits into
mainfrom
task4-start-resolver-export

Conversation

@vanceingalls

@vanceingalls vanceingalls commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

Part 4/4 of the SDK gap-closure stack. Exports the runtime start resolver as @hyperframes/core/runtime/start-resolver and lets callers provide the document used for relative data-start reference lookup.

Why

The resolver was internal and hard-bound to the host page’s global document, so iframe-hosted compositions could not resolve references such as data-start="intro + 2" reliably.

Implementation

  • Add optional documentRef, defaulting to the global document for existing callers.
  • Resolve IDs and composition IDs against the supplied document.
  • Make media-element detection realm-safe for iframe documents.
  • Re-export RuntimeTimelineLike.
  • Add the development and publish export-map entries.

Validation

  • src/runtime/startResolver.test.ts: 27 tests passing, including supplied-document and global-document coverage.
  • Full core Vitest suite passing.
  • oxlint and oxfmt passing.

vanceingalls commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator Author

@miga-heygen miga-heygen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review — #2502 feat(core): export runtime start resolver with documentRef parameter

Verdict: LGTM

SSOT check

doc is the single source for the document reference throughout the resolver. The isMediaElement helper correctly handles cross-realm checks (el.ownerDocument.defaultView?.HTMLMediaElement — an iframe document's media elements are instances of THAT frame's realm, not the host's). Falls back to the module's own HTMLMediaElement when defaultView is unavailable.

Package exports cleanly updated in package.json (both bun/node/import entries and the publishConfig block). RuntimeTimelineLike type re-exported for consumers.

Tests: 2 cases — supplied document vs global default. No blockers.

@james-russo-rames-d-jusso james-russo-rames-d-jusso left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed at c9f24030. Layering on Miga's LGTM. Red CI (Preview parity, preview-regression, Perf: parity, Perf: load, player-perf) is INHERITED from #2501's TS2345 compile break — see #2501 R1 for the fix. #2502's own diff compiles clean; no change owed HERE, but the stack won't be merge-clean until #2501 restacks.

Blockers

🟠 Stack-inherited: packages/core typecheck fails on #2501 basepositionEdits.ts:188 + :203 (orphaned[i] / marked[i] are Element | undefined under noUncheckedIndexedAccess, predicate takes Element). This kills every red check on THIS PR the same way it kills them on #2501, since the workspace build fails before startResolver even gets typechecked. Fix belongs in #2501 (widen the predicate or null-guard at both call sites); once #2501 restacks and this PR gets rebased on it, its own CI should clear.

Concerns

(none)

Nits

(none)

Green notes

🟢 documentRef shape is correct and default-safe. params.documentRef ?? document keeps every existing caller working — init.ts calls (:511, :525, :672, :803, :1917) and timeline.ts:159 all omit the field and continue to use the runtime bundle's own realm's document, which is the correct doc when the runtime is executing inside the composition sandbox. Hosts driving a composition from OUTSIDE that realm can now pass their target iframe's document and getElementById / querySelector resolve there instead of silently against the host page. Solid.

🟢 isMediaElement is a real cross-realm behavior fix, not just a refactor. The pre-PR element instanceof HTMLMediaElement required the media element to be in THIS module's realm — any iframe-hosted <video> / <audio> failed the instanceof and silently fell through to resolved = null, missing its data-playback-start / data-media-start / native .duration contribution to resolveDurationForElement. Post-PR, cross-realm media elements are correctly identified via ownerDocument.defaultView?.HTMLMediaElement, matching the same lens the positionEdits module uses. That's a small durability gain across host-embedded compositions worth calling out.

🟢 New subpath export ./runtime/start-resolver cleanly added. Both the top-level exports block and the publishConfig.exports block get the new entry; bun/node/import triplet + types. No drift between dev and published shapes.

🟢 RuntimeTimelineLike type re-export. Consumers of the new subpath can now type against the resolver's timelineRegistry param without reaching into @heygen/hyperframes-core/runtime/types. Type-only re-export so no vi.mock gap even if a future test targets the new subpath.

🟢 Test with document.implementation.createHTMLDocument("t") exercises the primary documentRef path with a scratch doc that has NO defaultView — which means isMediaElement's fallback branch (module-realm HTMLMediaElement) is what the scratch-doc test would take if it ever adds media coverage. Good default coverage.

What I didn't verify

  • Whether any external SDK consumers already import from the sibling ./runtime/startExpression subpath in a way that would want the same documentRef treatment. Out of scope for this PR either way — worth a follow-up if the SDK's user-facing surface is being tightened.

Review by Rames D Jusso

@vanceingalls
vanceingalls force-pushed the task4-start-resolver-export branch from c9f2403 to 06cd998 Compare July 16, 2026 01:50
Base automatically changed from task3-position-edit-force-reset to main July 16, 2026 02:07
@vanceingalls vanceingalls reopened this Jul 16, 2026
vanceingalls and others added 3 commits July 16, 2026 11:01
…t emits it

src/runtime is excluded from core's tsconfig include set — runtime files
only reach dist when an included module imports them. Without a root
re-export, the ./runtime/start-resolver publishConfig entry pointed at
dist/runtime/startResolver.js which tsc never emitted, failing
verify:packed-manifests in the Build job. Same precedent as
parseStartExpression's index re-export.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@vanceingalls
vanceingalls force-pushed the task4-start-resolver-export branch from 2f62ab9 to 21ab833 Compare July 16, 2026 18:03

@james-russo-rames-d-jusso james-russo-rames-d-jusso left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

R2 at HEAD 21ab83320fcd8cea82fc7ff2a43c2c2c29c96fde. Delta vs R1 SHA c9f2403079 = the current PR-scoped diff (4 files, +70/-4); the 90-commit rebase noise from main is out-of-scope. Reviewed with the SDK runtime-interop lens (realm safety, root discovery, packed-manifest emit).

Delta

  1. ./runtime/start-resolver subpath export added to packages/core/package.json (both root exports and publishConfig.exports) with bun/node/import/types conditions and the correct dist paths.
  2. Root re-export in packages/core/src/index.ts:265export { createRuntimeStartTimeResolver } from "./runtime/startResolver.js", with a load-bearing JSDoc explaining the mechanism: src/runtime is outside the tsconfig include set, so startResolver.ts only reaches dist/ when an included module imports it; without this line, the publishConfig.exports entry points at a file the pack doesn't contain, and verify:packed-manifests fails.
  3. documentRef?: Document param on createRuntimeStartTimeResolver (startResolver.ts:31-37). Defaults to module-global document. findReferenceTarget now calls doc.getElementById(refId) and doc.querySelector(...) against that resolved doc.
  4. isMediaElement realm-safe helper (startResolver.ts:47-52) — resolves HTMLMediaElement from el.ownerDocument.defaultView?.HTMLMediaElement first, falls back to the module-global constructor. Replaces the raw element instanceof HTMLMediaElement at line 84.
  5. 2 new testsdocumentRef resolves references against the supplied document; omitted documentRef defaults to the global document.

Findings

Blockers: none.

Concerns: none.

Green surface (verified):

🟢 Realm-safety fix on HTMLMediaElement is the right pattern — iframe media elements are instances of THEIR realm's constructor, never the module-global one. Pre-PR, embedders driving a composition in an iframe would silently fail the instanceof HTMLMediaElement check at line 84 and skip the media-based fallback. Aligns with the SDK runtime-interop cross-realm sentinel lens I carry from prior slices.

🟢 documentRef default binding at construction time (const doc = params.documentRef ?? document; at line 41) — resolver captures the doc once, so cross-realm lookups against a supplied documentRef stay consistent across findReferenceTarget calls. Correct scoping.

🟢 Root re-export is documented and traceable to the verify:packed-manifests constraint. The JSDoc explains WHY it exists — a maintainer removing it would fail the packed-manifest check on next publish. Prevents the "why is this here?" removal-then-regret cycle.

🟢 Test coverage covers both branches — supplied documentRef resolves intro + 2 cross-doc; omitted documentRef still walks the global doc. Structurally correct RED coverage.

🟢 No API break — the new param is optional; existing callers omit it and get the same module-global document binding they had before.

Nits (informational)

N1 — The resolver's startCache / durationCache / visiting state is per-instance (created in the closure); if a caller creates a resolver bound to documentRef=frameA.document and then passes an element owned by frameB.document to resolveStartForElement, cache pollution is bounded (WeakMap keyed on Element reference), but findReferenceTarget will search frameA's doc for an id/composition-id that lives in frameB. Existing behavior on the module-global doc has the same shape — noting only. A JSDoc line on resolveStartForElement reminding "pass elements owned by documentRef" would help future callers.

N2type { RuntimeTimelineLike } from "./types" re-export at end of file (line 200) is a small type-shape convenience for downstream consumers; unrelated to documentRef but bundled. Fine.

Merge gate

LGTM from my side — leaving as COMMENTED. Clean realm-safety fix plus a well-motivated subpath export.

Review by Rames D Jusso

@vanceingalls
vanceingalls merged commit f084a72 into main Jul 17, 2026
53 checks passed
@vanceingalls
vanceingalls deleted the task4-start-resolver-export branch July 17, 2026 07:47
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.

3 participants