slides: a save drops the assets nothing refers to, so a deck can shrink - #447
Merged
Conversation
doc.assets was append-only. Add images, delete every slide, save — and the "empty" deck was still 20 MB (#442), because deleting an element removes the reference and nothing ever removed the bytes. A deck could only ever grow. The fix lives in slides' own save facade, which now shadows serializeAuto and serializeFile with wrappers that hand the kernel a COPY of the document with unreferenced assets removed. The kernel never learns the slides document shape; the live document is never touched, so undo after a save still brings an image back and the next save keeps it. Every path that writes a .bento.html from this app goes through the facade: ⌘S write-back, the download fallback, the four export copies, and window.bento.serialize(). referencedAssetKeys enumerates every form the format has rather than walking generically, because the format uses two conventions — `asset:<key>` on image/media src and media poster, and BARE keys on svg.asset, code's grammar/theme ids and fonts[].asset — and a walk that knew only one would delete the other. That enumeration is the cost of this change and the rig pins each form individually; a count would pass with the wrong six surviving. blobs (the live-collab offload map) follows assets to the same key set. Measured, matched pair on the same scenario (one image plus two orphaned 1 MB assets, then delete everything and save): main: 3.74 MB / 3 assets -> 3.74 MB / 3 assets patched: 1.75 MB / 1 asset -> 0.75 MB / 0 assets test-export-secrets' serializeFile call-site guard is made precise rather than looser: it now matches a CALL and not a `function serializeFile(` DEFINITION, which is what the facade wrapper is. Verified by injecting a fresh plain call into autosave.ts — the guard still fails on it. Under live collaboration the CRDT syncs assets per key, so pruning the saved copy cannot lose a peer their asset. The one gap is documented at the function: a file that pruned K, reopened, then received a peer's op re-referencing K shows that image broken on that replica only, until re-added. Prior art: 1eevy's #372 addresses the same bug as one part of a larger save-time compactor (dedupe, key renaming, data-URI interning) bundled with three UI features and a kernel hook, and is deferred to a later release. This is the narrow fix and does not preclude that landing later.
Build size
Updated: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #442.
What was wrong.
doc.assetswas append-only. Add images, delete every slide, save — the "empty" deck was still 20 MB, because deleting an element removes the reference and nothing ever removed the bytes. A deck could only ever grow. Confirmed onmain: no code path prunesdoc.assets.The fix. Slides' own save facade (
slides/src/save.ts) now shadowsserializeAutoandserializeFilewith wrappers that hand the kernel a copy of the document with unreferenced assets removed. The kernel never learns the slides document shape. Every path that writes a.bento.htmlfrom this app goes through the facade — ⌘S write-back, the download fallback, the four export copies, andwindow.bento.serialize().The live document is never touched. Undo history in memory still holds the deleted element and its asset, so ⌘Z after a save brings both back and the next save keeps the asset because it is referenced again.
Measured, same scenario on both builds — one image plus two orphaned 1 MB assets, then delete everything and save:
mainReal headless Chrome through the built shell and
window.bento.serialize(), not the pure function in isolation.The one design choice worth reading.
referencedAssetKeysenumerates every reference form the format has rather than walking generically, because the format uses two conventions —asset:<key>onimage.src,media.src,media.poster; bare keys onsvg.asset,code.grammarAssetId,code.themeAssetId,fonts[].asset— and a walk that knew only one would silently delete the other. Dropping too much is the worse failure (an image gone from a file with nothing on screen to say so), so the new rigscripts/test-slides-assets.tspins each form individually and is weighted toward "kept". A count would pass with the wrong six surviving. A future field that references an asset must be added to that function, and the rig is where a miss shows up.blobs(the live-collab offload map) is pruned to the same key set.test-export-secrets.ts— itsserializeFilecall-site guard is made precise, not looser: it now matches a call and not afunction serializeFile(definition, which is what the facade wrapper is. Verified by injecting a fresh plainserializeFile(call intoautosave.ts: the guard still fails on it. Flagging this explicitly for security, since it's a security-flavoured rig: the property it protects — no user-facing path writes plaintext — is unchanged, and the wrapper's only caller is stillwindow.bento.serialize().Collab. The CRDT syncs
assetsper key, so pruning the saved copy cannot lose a peer their asset. One narrow gap, documented at the function: a file that pruned key K, was reopened, and then receives a peer's op re-referencing K (an undo on the peer, say) shows that image broken on that replica only, until re-added. Rare and non-corrupting; the trade against every deck growing without bound is not close.Prior art, credited. @1eevy's #372 addresses this same bug as one part of a larger save-time compactor (dedupe, key renaming, data-URI interning) bundled with three UI features and a kernel hook, deferred to a later release. This is the narrow fix and does not preclude that compaction landing later.
Not in this PR: the CHANGELOG entry — that's scribe's file.
Verified:
tsc -b,build:single, shell gate,test-ci-registered290/290 (the new rig is registered), preview 28/28, savepurpose 16/16, export-secrets 57/57, new rig 25/25.