Add optional indexNames scoping to InMemoryProvider put() - #87
Merged
Eliran Eretz-Kedosha (eliranek1) merged 9 commits intoAug 11, 2026
Merged
Conversation
added 3 commits
August 5, 2026 11:45
When a caller fetches data through a single index and caches the results
via put(), InMemoryStore previously seeded every index on the store with
those items, even indexes that were never queried. Combined with
memory-cursor range bookkeeping in consumers, this can cause a later
query on a different index to incorrectly believe it has full coverage
and skip the underlying DB, producing gaps ("islands") in the returned
data.
put() now accepts an optional indexNames?: string[] parameter. When
provided, brand-new items (not already tracked in the store) are only
written into the primary key plus the specified index(es). Items that
are already tracked continue to be kept in sync across every index they
were previously part of, so already-cached data never goes stale.
Omitting indexNames preserves the exact original behavior (write to all
indexes), so all existing callers are unaffected.
DbStore/DbProvider interfaces and the public put() shortcut are updated
to thread the new parameter through. IndexedDbProvider is intentionally
left unchanged since it doesn't need index scoping (native/managed
indexing).
Added a test covering: new items scoped to the specified index only,
already-tracked items staying in sync across all indexes on subsequent
scoped puts, and backward compatibility when indexNames is omitted.
Move indexNames off the shared DbStore/DbProvider surface so it is a compile-time error to pass it to any provider other than InMemoryProvider: - Revert DbStore.put() and the base DbProvider.put() shortcut to their original (no indexNames) signatures. - Change DbProvider._getStoreTransaction from private to protected so subclasses can reuse it. - Add a put(storeName, itemOrItems, indexNames?) override directly on InMemoryProvider that casts to InMemoryStore to reach the wider, store-level put() (unchanged). This guarantees indexNames scoping can only ever affect the in-memory cache, never the real database (e.g. IndexedDbProvider), since callers now need a reference typed as InMemoryProvider -- not the generic DbProvider -- to use the parameter at all.
amitshankar-msft
approved these changes
Aug 7, 2026
Copilot started work on behalf of
Eliran Eretz-Kedosha (eliranek1)
August 7, 2026 20:29
View session
Copilot started work on behalf of
Eliran Eretz-Kedosha (eliranek1)
August 7, 2026 20:31
View session
Addresses PR review feedback from amitshankar-msft: convert the plain comment blocks documenting InMemoryStore.put()'s indexNames parameter and DbProvider._getStoreTransaction()'s protected visibility rationale into JSDoc format for IDE tooltip support. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 009bd0a8-f98f-4048-a261-e0a627813a00
Also converts the InMemoryProvider.put() shortcut override comment (documenting indexNames scoping) to JSDoc format for consistency with InMemoryStore.put() and _getStoreTransaction(). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 009bd0a8-f98f-4048-a261-e0a627813a00
# Conflicts: # package.json
amitshankar-msft
approved these changes
Aug 7, 2026
Jérémie Poisson (jell0wed)
left a comment
Jérémie Poisson (jell0wed)
left a comment
Collaborator
There was a problem hiding this comment.
Consider maybe exporting a specialized InMemoryProviderWithScopedIndexPut type as part of this package instead of making the indexNames accessible through the public api interface
type InMemoryProviderWithScopedIndexPut = InMemoryProvider & {
put(
storeName: string,
itemOrItems: ItemType | ItemType[],
indexNames?: string[]
): Promise<void>;
};
…exPutProvider Address Jeremie's PR review comment about the scoped-index put() overload being reachable via a direct cast on the public InMemoryProvider class, which made it too easy for consumers to accidentally break the all-indexes-stay-in-sync invariant. - Renamed InMemoryProvider's scoped put overload to putInIndexAfterGet_DoNotUse(storeName, itemOrItems, indexNames), with indexNames now required (not optional) since scoping is the entire point of this method. - Added an exported IScopedIndexPutProvider interface and an asScopedIndexPutProvider(provider) type-guard/cast helper as the only sanctioned way to discover/reach this capability -- InMemoryProvider does not declare 'implements IScopedIndexPutProvider', so it doesn't show up on the class surface itself. - Split InMemoryStore's internal put implementation into a plain put() (interface-compliant with DbStore, always populates every index), putInIndexAfterGet_DoNotUse() (scoped), and a shared private _putInternal() so both call sites reuse the same logic. - Updated the _getStoreTransaction() comment and the scoped-put test to use the new name/helper instead of an InMemoryProvider cast. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 009bd0a8-f98f-4048-a261-e0a627813a00
Previously, a scoped put via putInIndexAfterGet_DoNotUse only skipped populating unrelated indexes for brand-new items. If the item already existed in the store, the old removal step swept every index on the schema before re-adding, effectively broadcasting the scoped put into every index anyway. Now both the removal and the re-add use the same indexesToTouch list (derived from the scoping hint when provided, else all schema indexes), so an index that wasn't part of this call is never disturbed either way -- it's neither seeded with new data nor evicted of data it already had. This closes the last gap where scoped puts for existing items could still create/heal islands in indexes they weren't meant to touch. Real (unscoped) put() calls are unaffected: _removeFromIndices' new indexesToRemoveFrom parameter defaults to every schema index, so full writes keep resyncing every index as before. Updated putInIndexAfterGet_DoNotUse's JSDoc and the existing unit test to reflect the new both-directions scoping behavior, and added coverage for the "island healed by a real unscoped put" invariant.
Jeremie flagged that putInIndexAfterGet_DoNotUse could leave a stale copy of an item in any index that wasn't part of the caller's scoped indexNames list, since each InMemoryIndex holds its own independent copy of an item's data. When an existing item is being overwritten, always remove the stale copy from every index it's actually cached by (not just the requested ones), then repopulate the union of "indexes that had it" and the caller's requested indexes. New items (never cached before) are unaffected and still only populate the requested/scoped index(es), preserving the original anti-"memory island" guarantee. - InMemoryIndex.remove() now returns whether it actually removed an entry, so callers can tell which indexes held the item. - _removeFromIndices() returns the list of indexes an item was actually removed from. - _putInternal (backing both put() and putInIndexAfterGet_DoNotUse()) uses that list to compute which indexes to repopulate. Added a regression test exercising Jeremie's exact scenario: an item cached across 3 indexes via a normal put(), then a scoped put via 2 of them with changed data -- asserts the 3rd (untouched by indexNames) index reflects the new data instead of the old, stale copy.
Jérémie Poisson (jell0wed)
approved these changes
Aug 11, 2026
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
3 participants
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.
Problem
Consumers (e.g. Teams' CDL sync layer) fetch data through a single index and cache the results in the in-memory provider via
store.put().InMemoryStore.put()unconditionally seeded every index on the store with each put item, including indexes that were never queried. Combined with range-cursor bookkeeping in consumers that track "have I loaded this range for this index" per index, this can cause a later query on a different index to incorrectly believe it already has full coverage in memory and skip the underlying DB read — producing gaps ("islands") in the returned data.Fix
put()now accepts an optionalindexNames?: string[]parameter:indexNamesis provided.indexNames, so already-cached data never goes stale.indexNames: preserves the exact original behavior (write to all indexes) — fully backward compatible with all existing callers.DbStore/DbProviderinterfaces and the publicput()shortcut are updated to thread the new parameter through.IndexedDbProvideris intentionally left unchanged since native/browser-managed indexing doesn't need this scoping (a narrower implementation signature is valid against the wider interface).Testing
ObjectStoreProvider.spec.ts(memory providers only) covering: new items scoped to the specified index only, already-tracked items staying in sync across all indexes on subsequent scoped puts, and backward compatibility whenindexNamesis omitted.npx tsc(full build) passes with zero errors.