Skip to content

fix(@angular/build): preserve binary values in the SQLite cache store - #33947

Merged
alan-agius4 merged 1 commit into
angular:mainfrom
manInit:fix-sqlite-cache-binary-values
Aug 27, 2026
Merged

fix(@angular/build): preserve binary values in the SQLite cache store#33947
alan-agius4 merged 1 commit into
angular:mainfrom
manInit:fix-sqlite-cache-binary-values

Conversation

@manInit

@manInit manInit commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

PR Checklist

Please check to confirm your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Issue Number: #33841

SqliteCacheStore persists values with JSON.stringify / JSON.parse. Some cached values are binary:

  • JavaScriptTransformer is constructed with Cache<Uint8Array> and stores the raw worker output under the jstransformer namespace.
  • CachedLoadResultEntry.contents is typed string | Uint8Array.

A JSON round trip cannot represent a typed array, so those values come back from disk as plain objects:

key:   jstransformer:000e84ece3a6f3f24cd37f3c37b7f8e94942ee88c0b1dd1cfc660e5c3fd71b07
value: {"0":105,"1":109,"2":112,"3":111,"4":114,"5":116,…}

createCachedLoad() then returns that object as the result of build.onLoad({ filter: /\.[cm]?js$/ }, …) in angular/compiler-plugin.ts, and esbuild fails the build:

✘ [ERROR] "contents" must be a string or a Uint8Array [plugin angular-compiler]

The first build always succeeds — the value is served from the in-memory cache layer and is only corrupted once it has been read back from disk — so the failure starts on the second build against a warm cache.

createPersistentCacheStore() only reaches the SQLite store when LMDB fails to load, which is why this is not hit on most machines. The trigger I ran into is the prebuilt @lmdb/lmdb-linux-x64@3.5.6 binary requiring GLIBC_2.33 on a host with glibc 2.31 (Ubuntu 20.04); Debian 11 and RHEL/CentOS 8 have the same glibc. That matches the reports in #33841, which was closed as not reproducible — the team could not reproduce it because LMDB loads fine on newer distributions, and the reporter's "fix" was upgrading the build agent from Ubuntu 20.04 to Ubuntu 24.04, i.e. moving back onto the LMDB path.

Reproduction independent of the host, forcing the store through the existing environment option:

rm -rf .angular
NG_BUILD_CACHE_STORE=sqlite ng build   # ok
NG_BUILD_CACHE_STORE=sqlite ng build   # ✘ "contents" must be a string or a Uint8Array

What is the new behavior?

Values are serialized with the V8 structured clone API (serialize / deserialize from node:v8), which supports typed arrays natively and matches how the LMDB store already behaves. node:sqlite accepts a Uint8Array parameter as a BLOB and returns a Uint8Array, so the round trip is lossless, and the value column is declared BLOB accordingly.

Old cache entries need no migration. They contain JSON text, deserialize throws on them, and the existing catch in get() already reports that as a cache miss, so those entries are recreated and overwritten. The cache path is also namespaced by package version (.angular/cache/<VERSION>/).

Added regression coverage in sqlite-cache-store_spec.ts: a Uint8Array round trip, a Uint8Array nested in an object (the shape of CachedLoadResultEntry), a round trip across two store instances (the on-disk path, which is where the bug actually shows), and a check that an entry which cannot be deserialized is treated as a cache miss. Binary round tripping was not covered before. All four fail on main and pass with this change.

The existing size-pruning test still passes unchanged — length() on a BLOB column returns the byte length, and the serialized payloads stay within the same 25 byte limit. I updated its comment to match the new byte counts.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Considered and rejected: keeping JSON with a replacer/reviver and base64. It inflates the cache by roughly a third and is easy to get wrong, since Buffer.prototype.toJSON runs before the replacer.

Separately, and not part of this PR: createPersistentCacheStore() swallows the LMDB load error entirely before falling back to SQLite. Logging the reason for the fallback would have made this much easier to diagnose — the build just fails later in an unrelated place. Happy to open a follow-up issue or PR for that if it is wanted.

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request updates the SqliteCacheStore to use V8 structured clone serialization (serialize and deserialize from node:v8) instead of JSON stringification. This preserves binary data, such as Uint8Array outputs, which would otherwise be corrupted by JSON round-trips. The SQLite schema is updated to store values as BLOB instead of TEXT, and a fallback is implemented to treat legacy JSON-serialized entries as cache misses. Comprehensive unit tests have been added to verify binary preservation and legacy fallback behavior. There are no review comments, and I have no additional feedback to provide.

@alan-agius4 alan-agius4 added the target: patch This PR is targeted for the next patch release label Aug 27, 2026
Comment thread packages/angular/build/src/tools/esbuild/sqlite-cache-store.ts Outdated
Comment thread packages/angular/build/src/tools/esbuild/sqlite-cache-store.ts Outdated
Comment thread packages/angular/build/src/tools/esbuild/sqlite-cache-store_spec.ts Outdated
The SQLite cache store serialized values with `JSON.stringify` and read them back
with `JSON.parse`. Several cached values contain binary data: the JavaScript
transformer stores its worker output as a `Uint8Array` (`Cache<Uint8Array>`), and
`CachedLoadResultEntry.contents` is typed as `string | Uint8Array`. A JSON round
trip cannot represent typed arrays, so those values came back from disk as plain
objects (`{"0":105,"1":109,...}`) and were handed to esbuild as load result
contents, failing the build with `"contents" must be a string or a Uint8Array`.

The failure only appeared from the second build onwards, because the first build
serves the value from the in-memory cache layer and the value is only corrupted
once it is read back from disk.

Values are now persisted using the V8 structured clone serialization API
(`node:v8`), which supports typed arrays natively and matches the behavior of the
LMDB store. The `value` column is declared as `BLOB` accordingly. SQLite column
types are dynamic, so a stored value is checked at runtime before it is
deserialized; values that are not binary, or whose payload is corrupt, are
treated as a cache miss and recreated.

The store is only reached when LMDB fails to load, which is why this went
unnoticed on most systems. A common trigger is a prebuilt `@lmdb/lmdb-linux-x64`
binary requiring a newer glibc than the host provides, for example on Ubuntu
20.04, Debian 11, or RHEL/CentOS 8. The fallback can also be selected explicitly
with `NG_BUILD_CACHE_STORE=sqlite`.

Closes angular#33841
@manInit
manInit force-pushed the fix-sqlite-cache-binary-values branch from ea5f8d0 to 2954209 Compare August 27, 2026 09:43

@alan-agius4 alan-agius4 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, thanks

@alan-agius4 alan-agius4 added the action: merge The PR is ready for merge by the caretaker label Aug 27, 2026
@alan-agius4
alan-agius4 merged commit 135662d into angular:main Aug 27, 2026
40 checks passed
@alan-agius4

Copy link
Copy Markdown
Collaborator

This PR was merged into the repository. The changes were merged into the following branches:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action: merge The PR is ready for merge by the caretaker area: @angular/build target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants