feat(compile): persist Web Storage/KV in a per-app data directory#34618
feat(compile): persist Web Storage/KV in a per-app data directory#34618bartlomieju wants to merge 7 commits into
Conversation
Compiled binaries previously could not persist origin-bound storage: the default `Deno.openKv()` silently fell back to an in-memory database, and `localStorage` and `caches` threw `NotSupported`. The standalone runtime constructed the worker with an empty `StorageKeyResolver` and no `origin_data_folder_path`, so the storage key never resolved. This wires both pieces up and treats a compiled binary as a standalone app: persistent storage now lives under the platform's app data directory (`%LOCALAPPDATA%`, `~/Library/Application Support`, `$XDG_DATA_HOME`) keyed by an app name, instead of `DENO_DIR`. The app name is baked in at compile time from a new `--app-name` flag, falling back to the output file name, so it is stable across runs and even if the binary is later renamed. Binaries sharing an `--app-name` share a store; differing names are isolated. A single fixed storage key per app is used, so there is one store per app rather than per-origin namespacing. Fixes #24318.
The determinism test compiles the same source to two different output names (a_bin/b_bin) and asserts the binaries are byte-identical. Now that the output file name is baked in as the default app-name metadata, the two binaries legitimately differ. Pass an explicit --app-name to both compiles so the test still verifies path independence (the rename a->b).
bartlomieju
left a comment
bartlomieju
left a comment
There was a problem hiding this comment.
Reviewed the full diff and verified the storage-key invariant against the base runtime. This is a clean, well-scoped fix for #24318 with strong test coverage. A couple of notes below; nothing blocking except CI.
The critical invariant holds. cli/lib/worker.rs unwraps origin_data_folder_path whenever the storage key resolves to a value (// must be set if storage key resolver returns a value). This PR preserves that: in run.rs the resolver is non-empty only when origin_data_folder_path.is_some(), so no panic path is introduced.
No --location regression. run.rs always used StorageKeyResolver::empty() before, so location never drove storage in compiled binaries; dropping it from the storage path changes nothing observable. The location global is still wired via metadata.location.
Tests look good: KV/localStorage/caches persistence across cold processes, app-name sharing vs. isolation, and the determinism test correctly pins --app-name so the byte-identical guarantee survives now that the output name feeds the default app name. All hermetic via redirected HOME/XDG_DATA_HOME/LOCALAPPDATA.
One non-blocking suggestion inline about validating --app-name at compile time rather than sanitizing at runtime.
Note: CI shows no checks yet, so I can't confirm green; not approving on that basis alone.
Reject empty, , , or values containing path separators with a clear error in the binary writer, instead of best-effort sanitizing the name at runtime. The app name becomes a single directory component under the platform app data directory, so an unsanitized value could escape or collide with that directory. Removes the runtime guard in run.rs and adds an app_name_invalid spec test covering the rejected forms.
Broaden validate_app_name to reject Windows-reserved characters (<>:"|?*), control characters, trailing dots/spaces, and reserved device names (CON, NUL, COM1, ...) in addition to the existing empty/ dot/separator checks. The app name is baked at compile time but the binary may be cross-compiled, so the name must resolve to one unambiguous directory component on every target OS. Adds spec cases for a reserved character, a trailing dot, and a reserved device name. Clarify the storage-related comments that the review found misleading: the `caches` directory is temp-backed and only gated by a resolvable app-data dir (not stored under it), the `app:` storage-key prefix namespaces compiled apps from URL-derived keys in that shared cache dir, and the runtime app_name fallback only applies to binaries compiled before the metadata field existed.
Two failures introduced by the merge/review changes: - `compile_watch_paths_include_includes_and_icon` constructs a `CompileFlags` literal that main added after this branch forked, so it was missing the new `app_name` field (E0063). This only surfaced in `cargo test --lib`/clippy `--all-targets`, not `cargo check --bin`, since it is in a `#[cfg(test)]` block. - `validate_app_name` used a block expression directly in an `else if` condition, tripping `clippy::blocks_in_conditions`. Hoist the reserved device-name check into a `let` binding before the chain.
1 participant
In a
deno compile-d binary, origin-bound storage did not work. Thedefault
Deno.openKv()(called without an explicit path) silently fellback to an in-memory database, so data never survived across runs, and
both
localStorageand the WebcachesAPI threwNotSupported.The root cause was in the standalone runtime (
cli/rt/run.rs), whichbuilt the worker factory with
StorageKeyResolver::empty()andorigin_data_folder_path: None. With no storage key the worker resolvesboth
origin_storage_dirandcache_storage_dirtoNone; KV then opensan in-memory connection and the storage globals are disabled.
Rather than route this through
DENO_DIRthe waydeno rundoes, thistreats a compiled binary as a standalone application. Persistent storage
now lives under the platform's app data directory (
%LOCALAPPDATA%,~/Library/Application Support,$XDG_DATA_HOME), reusing the sameget_data_local_dir()resolution already used for self-extractingbinaries, keyed by an app name. This keeps a shipped binary from writing
user data into a Deno tooling cache that may not even exist on the target
machine.
The app name is the single source of storage identity. It is baked in at
compile time from a new
--app-nameflag, falling back to the output filename when the flag is omitted, so it is stable across runs and even if the
produced binary is later renamed. Recompiling with a different
--output/--app-namestarts a fresh store. Because the name (not themain-module URL) drives the directory, two binaries compiled with the same
--app-nameshare a store while differently named apps stay isolated, andthe previous footgun where two binaries that merely shared an executable
file name would collide is gone. A single fixed storage key is used per
app, so each app gets one store rather than per-origin namespacing;
--locationis no longer involved in storage. When the data directorycannot be resolved the storage key is left empty and the previous
in-memory fallback is preserved, which also avoids unwrapping a missing
data folder path in the worker.
The app name is carried in the compile
Metadataso the standaloneruntime can read it back;
--app-version(#29038) is a natural nextmember of this same app-identity metadata.
Spec tests under
tests/specs/compilecompile a script and exercise itacross cold processes:
kv_default_storage_pathproves a defaultDeno.openKv()survives between runs,local_storage_default_pathdoesthe same for
localStorage,web_cache_storageprovescachesisavailable, and
app_name_storageproves two binaries sharing an--app-nameshare a store while a differently named app does not. Eachtest redirects the platform app data directory into its temp dir to stay
hermetic.
Fixes #24318. Supersedes #33775 and #34576, folding in the
localStorage/cachescoverage from #33775 (thanks @CertainLach for theoriginal diagnosis of #18889/#10693).