diff --git a/.changeset/shiny-donkeys-repeat.md b/.changeset/shiny-donkeys-repeat.md new file mode 100644 index 000000000..12c4ee8d1 --- /dev/null +++ b/.changeset/shiny-donkeys-repeat.md @@ -0,0 +1,5 @@ +--- +'@tanstack/db': patch +--- + +Fix: `materialize(q…findOne())` resolved to `null` instead of `undefined` when the correlation key itself was null. A null correlation key never reached the includes materialization step, so the compiler's `null` select placeholder leaked into the result, violating the typed `T | undefined` contract. Null correlation keys now resolve to the empty materialized value (`undefined` for `findOne()`, `[]` for arrays, `` for `concat`). diff --git a/packages/db/src/query/compiler/index.ts b/packages/db/src/query/compiler/index.ts index b7779a21d..95ec616ba 100644 --- a/packages/db/src/query/compiler/index.ts +++ b/packages/db/src/query/compiler/index.ts @@ -346,6 +346,12 @@ export function compileQuery( getRouting: (nsRow: any) => { correlationKey: unknown parentContext: Record | null + /** + * Set when this row does not participate in the include at all + * (conditional-select guards failed or no routing info was found), + * as opposed to participating with a null correlation key. + */ + skipped?: boolean } }> = [] for (const { sourceAlias, include } of sourceIncludes) { @@ -388,12 +394,13 @@ export function compileQuery( fieldName, getRouting: (nsRow: any) => { if (!matchesConditionalSelectGuards(compiledGuards, nsRow)) { - return { correlationKey: null, parentContext: null } + return { correlationKey: null, parentContext: null, skipped: true } } return ( nsRow[sourceAlias]?.[INCLUDES_ROUTING]?.[include.fieldName] ?? { correlationKey: null, parentContext: null, + skipped: true, } ) }, @@ -427,12 +434,17 @@ export function compileQuery( fieldName, getRouting: (nsRow: any) => { if (!matchesConditionalSelectGuards(compiledGuards, nsRow)) { - return { correlationKey: null, parentContext: null } + return { + correlationKey: null, + parentContext: null, + skipped: true, + } } return ( nsRow[INCLUDES_ROUTING]?.[include.fieldName] ?? { correlationKey: null, parentContext: null, + skipped: true, } ) }, @@ -641,7 +653,11 @@ export function compileQuery( fieldName, getRouting: (nsRow: any) => { if (!matchesConditionalSelectGuards(compiledRoutingGuards, nsRow)) { - return { correlationKey: null, parentContext: null } + return { + correlationKey: null, + parentContext: null, + skipped: true, + } } const parentContext: Record> = {} for (const proj of compiledProjs) { @@ -667,7 +683,11 @@ export function compileQuery( fieldName, getRouting: (nsRow: any) => { if (!matchesConditionalSelectGuards(compiledRoutingGuards, nsRow)) { - return { correlationKey: null, parentContext: null } + return { + correlationKey: null, + parentContext: null, + skipped: true, + } } return { correlationKey: compiledCorrelation(nsRow), @@ -760,7 +780,11 @@ export function compileQuery( map(([key, namespacedRow]: any) => { const routing: Record< string, - { correlationKey: unknown; parentContext: Record | null } + { + correlationKey: unknown + parentContext: Record | null + skipped?: boolean + } > = {} for (const { fieldName, getRouting } of includesRoutingFns) { routing[fieldName] = getRouting(namespacedRow) diff --git a/packages/db/src/query/live/collection-config-builder.ts b/packages/db/src/query/live/collection-config-builder.ts index 4d80b3fe1..d1108cf6a 100644 --- a/packages/db/src/query/live/collection-config-builder.ts +++ b/packages/db/src/query/live/collection-config-builder.ts @@ -1912,6 +1912,20 @@ function flushIncludesState( // Parent rows may already be materialized in the live collection by the // time includes state is flushed, so update the stored row as well. + const storedParent = parentCollection.get(parentKey as any) + if (storedParent && storedParent !== parentResult) { + setIncludedValue(storedParent, state.resultPath, childValue) + } + } else if (routing && !routing.skipped && materializesInline(state)) { + // The correlation key itself is null/undefined, so no child row + // can ever match and no child pipeline output will arrive for + // this parent. Without this branch the compiler's `null` select + // placeholder leaks into the result (#1706). Resolve the field + // to its empty materialization instead: `undefined` for + // findOne(), `[]` for arrays, `` for concat. + const childValue = materializeIncludedValue(state, undefined) + setIncludedValue(parentResult, state.resultPath, childValue) + const storedParent = parentCollection.get(parentKey as any) if (storedParent && storedParent !== parentResult) { setIncludedValue(storedParent, state.resultPath, childValue) diff --git a/packages/db/tests/query/includes.test.ts b/packages/db/tests/query/includes.test.ts index 2eba6f122..2dcb47968 100644 --- a/packages/db/tests/query/includes.test.ts +++ b/packages/db/tests/query/includes.test.ts @@ -6573,6 +6573,116 @@ describe(`includes subqueries`, () => { expect(orphan.project).toBeUndefined() }) + // Regression tests for #1706: a null correlation key must resolve to the + // empty materialized value (`undefined` for findOne(), `[]` for arrays), + // not leak the compiler's `null` select placeholder into the result. + describe(`null correlation key (#1706)`, () => { + type Author = { id: number; name: string } + type Post = { id: number; title: string; authorId: number | null } + + function createAuthorAndPostCollections(posts: Array) { + const authorCollection = createCollection( + localOnlyCollectionOptions({ + getKey: (author) => author.id, + initialData: [{ id: 1, name: `Ada` }], + }), + ) + const postCollection = createCollection( + localOnlyCollectionOptions({ + getKey: (post) => post.id, + initialData: posts, + }), + ) + return { authorCollection, postCollection } + } + + it(`findOne() with a null correlation key yields undefined`, async () => { + const { authorCollection, postCollection } = + createAuthorAndPostCollections([ + { id: 1, title: `fk matches an author`, authorId: 1 }, + { id: 2, title: `fk set, but no author matches`, authorId: 999 }, + { id: 3, title: `fk is null`, authorId: null }, + ]) + + const postsWithAuthor = createLiveQueryCollection((q) => + q.from({ post: postCollection }).select(({ post }) => ({ + ...post, + author: materialize( + q + .from({ author: authorCollection }) + .where(({ author }) => eq(author.id, post.authorId)) + .findOne(), + ), + })), + ) + await postsWithAuthor.preload() + + expect( + stripVirtualPropsDeep((postsWithAuthor.get(1) as any).author), + ).toEqual({ id: 1, name: `Ada` }) + expect((postsWithAuthor.get(2) as any).author).toBeUndefined() + expect((postsWithAuthor.get(3) as any).author).toBeUndefined() + expect((postsWithAuthor.get(3) as any).author).not.toBeNull() + }) + + it(`array materialize with a null correlation key yields []`, async () => { + const { authorCollection, postCollection } = + createAuthorAndPostCollections([ + { id: 3, title: `fk is null`, authorId: null }, + ]) + + const postsWithAuthors = createLiveQueryCollection((q) => + q.from({ post: postCollection }).select(({ post }) => ({ + id: post.id, + authors: materialize( + q + .from({ author: authorCollection }) + .where(({ author }) => eq(author.id, post.authorId)) + .select(({ author }) => ({ + id: author.id, + name: author.name, + })), + ), + })), + ) + await postsWithAuthors.preload() + + expect((postsWithAuthors.get(3) as any).authors).toEqual([]) + }) + + it(`updating the correlation key to null re-emits undefined`, async () => { + const { authorCollection, postCollection } = + createAuthorAndPostCollections([ + { id: 1, title: `starts with a matching fk`, authorId: 1 }, + ]) + + const postsWithAuthor = createLiveQueryCollection((q) => + q.from({ post: postCollection }).select(({ post }) => ({ + ...post, + author: materialize( + q + .from({ author: authorCollection }) + .where(({ author }) => eq(author.id, post.authorId)) + .findOne(), + ), + })), + ) + await postsWithAuthor.preload() + + expect( + stripVirtualPropsDeep((postsWithAuthor.get(1) as any).author), + ).toEqual({ id: 1, name: `Ada` }) + + postCollection.update(1, (draft) => { + draft.authorId = null + }) + await flushPromises() + + expect((postsWithAuthor.get(1) as any).author).toBeUndefined() + expect((postsWithAuthor.get(1) as any).author).not.toBeNull() + }) + }) + it(`inserting the matching child re-emits parent with populated singleton`, async () => { // Start with an issue whose project doesn't exist yet issues.utils.begin()