Skip to content

Firestore DML fixes - #9565

Merged
MarkDuckworth merged 8 commits into
feat-node-dmlfrom
markduckworth/node-dml-api-fixes
Oct 14, 2026
Merged

MarkDuckworth merged 8 commits into
feat-node-dmlfrom
markduckworth/node-dml-api-fixes

Conversation

@MarkDuckworth

Copy link
Copy Markdown
Contributor

Follow-up to #9118. This aligns the DML and literals pipeline APIs with the existing Pipeline API conventions, fixes several encoding and validation bugs in the literals stage, and documents how insert/upsert choose their target documents and what atomic does.

Fixes API problems that the proposal review missed, before the API ships. It also fixes literals inputs that the SDK previously dropped silently, crashed on, or encoded in a way the backend rejected.

Changes

API shape

  • literals(documents) + literals(options), with documents required in LiteralsStageOptions. Why: other stages take their required argument directly and also offer an options overload that carries it. The backend rejects a literals stage with no documents, so documents shouldn't be optional.
  • update: removed the Record<string, Expression> | Map overload and added update(options: UpdateStageOptions) with an optional transformedFields. Why: no other stage accepts a map of fields; they take AliasedExpressions. Every stage should also have an options overload. This removed overload was not in the API proposal or tests.
  • delete() + delete(options: DeleteStageOptions). Why: every stage should have an options overload, e.g. for rawOptions. This follows the database() precedent for stages with no required parameters.
  • insert() + insert(options), replacing a single optional-options signature. Why: this matches the database() precedent and the other DML stages.
  • upsert(), upsert(field, ...fields) and upsert(options); removed upsert(fields[], options?). Why: this matches the variadic convention used by select(), addFields(), etc. The removed overload could pass fields in two places, and the code had to merge them.
  • Removed the deprecated transforms option from UpsertStageOptions. Why: it was a pre-release name for additionalFields, added and renamed within feat(firestore): add DML stages, literals source, and atomic execution option to Node SDK pipelines #9118. It was never released in Node or Web, so there's nothing to deprecate.
  • literals is only on PipelineSource now, not on Pipeline. Why: literals is a source stage. It was never tested in a non-first position, and the Web SDK only exposes it on PipelineSource.
  • The Delete and Update stages now forward rawOptions. Why: they were silently dropping them, unlike every other stage.
    literals encoding and validation
  • FieldValue sentinels (serverTimestamp(), increment(), arrayUnion(), delete(), ...) in literal documents now throw Function literals() called with invalid data. FieldValue.serverTimestamp() can only be used with update() and set() (found in field <path>). Why: a pipeline has no way to represent sentinels. Before this change they were silently dropped, nested data was lost, and a document containing only a sentinel failed with an opaque 13 INTERNAL serialization error. The message matches the Web SDK. Validation is now recursive, which also covers nested user data. currentTimestamp() is the server-side alternative to serverTimestamp().
  • Nested objects and arrays in literal documents are automatically wrapped in map() / array() expressions. Why: the backend only evaluates expressions in the top-level fields of a literal document, so {a: {b: add(...)}} and {arr: [add(...)]} failed with Value type is not supported: FUNCTION_VALUE. Each document is still sent as a top-level mapValue. Its fields are converted with valueToDefaultExpr, the same conversion used for expression arguments and rawStage().
  • undefined in literal documents is rejected unless ignoreUndefinedProperties is enabled. Why: this keeps the previous serializer behaviour, which the new encoding would otherwise have bypassed by silently dropping undefined.

…line API conventions

Follow the existing Pipeline stage convention of `stage(requiredArg)` /
variadic convenience overloads plus a single `stage(options)` overload,
where the options object carries the required params.

- literals (PipelineSource and Pipeline): remove the
  `(documents, options?)` overload; `documents` is now required in
  LiteralsStageOptions.
- update: remove the `Record<string, Expression> | Map` overload (not in
  the API proposal, no precedent, and indistinguishable at runtime from
  an options object); add `update(options: UpdateStageOptions)` with
  `transformedFields`.
- delete: add `delete(options: DeleteStageOptions)` so rawOptions can be
  provided.
- upsert: remove the `(additionalFields[], options?)` overload; use
  `upsert({additionalFields, ...})` instead.
…verloads

Follow the precedent set by database() for stages with no required
params, consistent with delete().
…tageOptions

transforms was an earlier name for additionalFields that was never
released in the Node or Web SDKs, so the deprecated alias is
unnecessary API surface.
…neSource only

literals is a source stage (LiteralsSource). It was only ever used and
tested as the first stage via PipelineSource, and the Web SDK exposes it
only on PipelineSource.
…ents

FieldValue sentinels (serverTimestamp(), increment(), arrayUnion(),
arrayRemove(), delete(), ...) cannot be represented in an encoded
pipeline. Previously they were silently dropped from literal documents
(and a document containing only sentinels failed with an INTERNAL
serialization error). Literal documents are now validated recursively
and sentinels throw the same error as the Web SDK:

  Function literals() called with invalid data. <method>() can only be
  used with update() and set() (found in field <path>)

Nested values with user data are now also validated, not just top-level
fields.
hasUserData() dereferences its argument, so a null value in a literal
document crashed _validateUserData with a TypeError.
…ents

The backend only evaluates expressions in the top-level fields of a
literal document, so nested expressions (e.g. {a: {b: add(...)}}) failed
with "Value type is not supported: FUNCTION_VALUE". Encode each document
as a top-level mapValue whose fields are converted with
valueToDefaultExpr, so nested objects and arrays become map() and array()
expressions, matching rawStage() and expression arguments.

Undefined values are now rejected unless ignoreUndefinedProperties is
enabled, preserving the previous serializer behavior.
…ic option

Describe how collection and documentIdExpression determine the target
document, that a string documentIdExpression is a field name (use
constant() for a fixed ID), that upsert requires collection when
documentIdExpression is set, and what atomic does. Behavior verified
against nightly; adds system tests for the field-name semantics and the
upsert collection requirement.
@MarkDuckworth MarkDuckworth changed the title Firestore DML fixess Firestore DML fixes Oct 7, 2026
@MarkDuckworth
MarkDuckworth marked this pull request as ready for review October 7, 2026 23:35
@MarkDuckworth
MarkDuckworth requested review from a team as code owners October 7, 2026 23:35

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

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 refactors several Firestore pipeline stages—including literals, delete, update, insert, and upsert—to accept options objects, updating their signatures, type definitions, and tests accordingly. The review feedback focuses on improving robustness and decoupling: it suggests moving the selectablesToMap conversion into the UpdateStage constructor to keep the Pipeline class clean, adding safe default fallbacks to prevent runtime crashes when options are undefined, and strengthening validation in the literals stage to ensure each document is a plain object.

Comment thread handwritten/firestore/dev/src/pipelines/pipelines.ts
Comment thread handwritten/firestore/dev/src/pipelines/stage.ts
Comment thread handwritten/firestore/dev/src/pipelines/pipelines.ts
Comment thread handwritten/firestore/dev/src/pipelines/stage.ts
Comment thread handwritten/firestore/dev/src/pipelines/stage.ts
@MarkDuckworth
MarkDuckworth merged commit 83ed77d into feat-node-dml Oct 14, 2026
25 of 26 checks passed
@MarkDuckworth
MarkDuckworth deleted the markduckworth/node-dml-api-fixes branch October 14, 2026 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant