fix: delay ObjectURL revocation and silence TS5 baseUrl deprecations#1181
Conversation
- Delay URL.revokeObjectURL() from 0ms to 1000ms in useFrameCapture so the browser has time to initiate the download before the blob is freed. A 0ms timeout fires synchronously after the current microtask queue, before the browser's download machinery reads the URL. - Add ignoreDeprecations: '5.0' to cli and studio tsconfigs to silence TypeScript baseUrl/paths deprecation warnings without changing behavior. Co-Authored-By: Jefsky Wong <jefsky@qq.com>
jrusso1020
left a comment
There was a problem hiding this comment.
Reviewed at 2d5c0038. Clean cherry-pick of valid changes from #795. Tiny + focused (5+/3-, 3 files), exactly what the PR body claims. One real attribution issue worth fixing before merge.
Verified the diff matches the PR-body description
# packages/studio/src/hooks/useFrameCapture.ts
- setTimeout(() => URL.revokeObjectURL(blobUrl), 0);
+ setTimeout(() => URL.revokeObjectURL(blobUrl), 1000);
# packages/cli/tsconfig.json + packages/studio/tsconfig.json
+ "ignoreDeprecations": "5.0"- Race fix:
setTimeout(..., 0)fires before the browser initiates the download (the click handler returns, the task queue processes the timeout BEFORE the download dispatch). The revocation invalidates the blob URL while the download machinery is still reading it. Some browsers silently fail the download; others race.1000msgives the download dispatch enough time to read the URL. ✓ - TS5 deprecations:
baseUrl/pathsare deprecated in TypeScript 5.0+ but still functional.ignoreDeprecations: "5.0"is the canonical opt-out without changing behavior — supported since TS 5.0. ✓
The drops vs #795 (LayersPanel.tsx, generator.ts) are correctly cited as already-resolved by #792 + #625.
Attribution gap — should be fixed before merge
PR body has:
Co-authored-by: Jefsky Wong
But the commit at 2d5c0038 does NOT have this trailer in its message. Per GitHub's contributor-attribution model, Co-authored-by trailers must be in the COMMIT MESSAGE (not just the PR body) for the co-author to get the contribution on their GitHub profile. PR-body mentions don't credit them.
This matters for OSS-contributor trust on a public repo (~23K stars) — Jefsky did the original work, and the proper attribution is what makes the next external contributor feel safe submitting. Easy fix:
git commit --amend -m "$(cat <<'EOF'
fix: delay ObjectURL revocation and silence TS5 baseUrl deprecations
[existing body…]
Co-authored-by: Jefsky Wong <jefsky-email-or-handle@example.com>
EOF
)"
git push --force-with-lease(The email needs to be Jefsky's actual GitHub-registered email or <noreply-style-id+username@users.noreply.github.com>. If you don't have it handy, GitHub displays the trailer's author-line as a public profile link as long as the username is right.)
Smaller observations (non-blocking)
-
1000msis the contributor's chosen number. Worth knowing it's arbitrary — there's no spec'd safe-revocation time for ObjectURL. The bulletproof alternative is an event-driven revocation (e.g. listen forunload/pagehideon the iframe-or-equivalent, revoke then), but that's invasive.1000msworks in practice; documented as "conservative-safe" in the PR body. Fine. -
No tests added. The URL.revokeObjectURL race isn't easily unit-testable (no good fake for browser blob mechanics). The tsconfig changes are config-only with no test surface. Acceptable to skip tests for this class of fix; CI's lint + typecheck catch the tsconfig changes' validity.
-
TS5 deprecation strategy.
ignoreDeprecationsis a silence-the-warning fix, not a resolve-the-deprecation fix. The proper resolution is movingbaseUrl/pathsconfiguration to a different shape (e.g. relying on TS's automatic path resolution withtsconfig.jsonlocation). Out of scope for this PR — Vance / Magi can file a follow-up if anyone wants to tackle the actual deprecation rather than the warning. The PR's framing as "silence warnings until TS deprecates further" is reasonable. -
Closes #795. Verified the referenced PR exists + this is a clean subset of its valid changes. ✓
Verdict
LGTM on the fix mechanics. Co-authored-by attribution is the one real item before merge — easy --amend + force-push, but it does matter for OSS contributor goodwill on a public repo this size.
Stamp held — James gates.
— Rames Jusso (hyperframes)
Clean cherry-pick of the valid changes from #795 (originally by @Jefsky), rebased on current main. The spurious
LayersPanel.tsxand already-merged caption fix have been dropped.Changes
useFrameCapture.ts— ObjectURL revocation timingsetTimeout(..., 0)fires before the browser initiates the download, causing the blob URL to be revoked while the download machinery is still reading it. 1000ms is conservative-safe.cli/tsconfig.json+studio/tsconfig.json— TS5 deprecation warningsbaseUrl/pathsare deprecated in TypeScript 5.0+.ignoreDeprecations: "5.0"is the canonical way to silence the warnings without changing behavior.What was dropped vs #795
LayersPanel.tsx— stale addition that would have conflicted with existing file; layer seek fix already merged via fix(studio): keep layer selection seek within clip range #792generator.ts— caption escaping fix already merged via fix: use JSON.stringify for caption text escaping in generated JS #625Closes #795.
Co-authored-by: Jefsky Wong jefsky@qq.com