refactor(firestore): modernize internal proto call sites and eliminate local declarations (#9076) by quirogas · Pull Request #9076 · googleapis/google-cloud-node

quirogas · 2026-08-04T08:07:51Z

Part 4 of stacked disentanglement (#8928). Transition @google-cloud/firestore to directly consume @google-cloud/firestore-api as its native Single Source of Truth for TypeScript declarations, removing static checked-in local proto types.

Background & Motivation

Historically, dev/protos/update.sh compiled static TypeScript interfaces (firestore_v1_proto_api.d.ts) using custom build transformations:

Used --force-enum-string to convert protobuf numeric enums into string literal unions ('ASCENDING' | 'DESCENDING').
Executed a Perl regex to replace 64-bit integer Long references with string.

While this simplified early static method signatures, it created dependency on static checked-in declaration files that risk drifting out of sync as upstream protobuf definitions evolve over time. Transitioning directly to @google-cloud/firestore-api provides continuous automated type generation without build script maintenance.

Summary of Changes

Eliminate Static Local Types: Complete removal of dev/protos/firestore_v1_proto_api.d.ts, dev/protos/firestore_v1_proto_api.js, and dev/protos/v1.json, making @google-cloud/firestore-api our native Single Source of Truth.
Bundle Protocol Isolation: Extract custom offline bundle persistence interfaces (bundle.proto), which belong strictly to the handwritten SDK wrapper and are independent of GAPIC models, into standalone local modules (dev/src/bundle-proto).
Call-Site & Comparator Modernization: Update internal query dictionaries (directionOperators, comparisonOperators) and sorting algorithms to natively consume GAPIC enum objects and 64-bit Long structures.

Backward Compatibility Guarantee

All internal sorting and query comparator algorithms implement dual matching (orderBy.direction === 'ASCENDING' || orderBy.direction === api.StructuredQuery.Direction.ASCENDING) to support both literal strings and numeric GAPIC enums interchangeably.
Verified that during HTTP REST fallback execution (fallback: true), proto3-json-serializer inspects GAPIC reflection models and automatically translates numeric integer enum codes back into standard JSON string schema names before transmitting payloads over HTTP, guaranteeing zero regressions for REST consumers.

Internal: b/531788771

📚 Stack Navigation Index

Layer 1 (Base Disentanglement & Runtime Forwarding): feat(firestore): decouple handwritten sdk wrapper from embedded gapic clients #8928
Layer 2 (Complete Decoupling & Dead Code Cleanup): feat(firestore)!: complete gapic decoupling, adopt subpath imports, and remove legacy proto code (#9074) #9074
Layer 3 (Automated Presubmit Bundle Size Guard): chore(firestore): add automated presubmit serverless bundle size regression guard (#9075) #9075
Layer 4 (Call-Site Modernization & Eliminate Local Protos): refactor(firestore): modernize internal proto call sites and eliminate local declarations (#9076) #9076

… callsite modernization (#8928)

gemini-code-assist

Code Review

This pull request introduces a new TypeScript declaration file, bundle.d.ts, defining the custom Firestore bundle persistence protocol types. The review feedback focuses on improving type safety across these declarations. Specifically, it is recommended to replace several any types for time-related fields (createTime and readTime) with a dedicated Timestamp interface, define that Timestamp interface, and restrict the limitType field to the specific string union values 'FIRST' | 'LAST'.

gemini-code-assist · 2026-08-04T08:08:59Z

+  }
+  export interface BundleMetadata {
+    id?: string | null;
+    createTime?: any | null;


Using any for createTime reduces type safety. We should use a Timestamp interface instead.

Suggested change

createTime?: any | null;

createTime?: Timestamp | null;

…e local declarations (#8928) - Remove dev/protos/firestore_v1_proto_api.* and dev/protos/v1.json to transition @google-cloud/firestore-api into our automated Single Source of Truth for TypeScript declarations - Extract custom offline bundle persistence interface definitions into standalone dev/src/bundle-proto.* modules - Update query operator dictionaries (directionOperators, comparisonOperators) and internal comparators to consume native GAPIC enum objects and 64-bit Long structures - Verify 100% clean compilation and passing unit test suites across watch, partition-query, aggregate-query, and transaction operations

…from Layer 3 (#9076)

… with firestore-api@0.4.0 (#9076) - Connect standalone bundle-proto module directly to firestore-api@0.4.0 GAPIC message roots and automate copy during postcompile - Normalize unary and composite field filter operator serializations to cleanly emit canonical GAPIC numeric enums - Update unit test builder helpers across query, vector-query, and bundle verification suites to ensure 100% passing test compatibility against official firestore-api@0.4.0 release

…yer 3 (#9076)

…s shell compatibility (#9076) - Remove minifyProtoJson execution from postcompile script as static runtime GAPIC .js/.json files are now minified upstream inside @google-cloud/firestore-api - Strip POSIX stream error redirection syntax (2>/dev/null) to prevent command failures on Windows cmd and PowerShell CI runners - Preserve directory initialization and raw bundle.proto schema copy steps required by conformance test suites

MarkDuckworth · 2026-08-05T20:59:04Z

 */

-import * as protos from '../../protos/firestore_v1_proto_api';
+import * as protos from "@google-cloud/firestore-api/build/protos/protos";


is /build in the path intentional? seems like the entrypoint should be @google-cloud/firestore-api/protos/protos. Same across most files in this PR

looking back at the change to exports in firestore-api, I guess I missed this as a reviewer. we can change this later since firestore-api is not yet v1.0

MarkDuckworth · 2026-08-05T21:10:21Z

        {
          parent: query.toProto().parent,
-          structuredQuery: query.toProto().structuredQuery,
+          structuredQuery: require("@google-cloud/firestore-api/build/protos/protos").google.firestore.v1.StructuredQuery.fromObject(query.toProto().structuredQuery).toJSON(),


Is the require("@google-cloud/firestore-api/build/protos/protos") necessary here given we already have the import at the top of the file?

MarkDuckworth · 2026-08-05T21:10:27Z

        {
          parent: newQuery.toProto().parent,
-          structuredQuery: newQuery.toProto().structuredQuery,
+          structuredQuery: require("@google-cloud/firestore-api/build/protos/protos").google.firestore.v1.StructuredQuery.fromObject(newQuery.toProto().structuredQuery).toJSON(),


MarkDuckworth · 2026-08-05T21:34:06Z

  constructor(
    private filters: FilterInternal[],
-    private operator: api.StructuredQuery.CompositeFilter.Operator,
+    private operator: api.StructuredQuery.CompositeFilter.Operator | any,


This applies across the board regarding parameters with enum types that have been extended as ... | any. It would be safer to use EnumType | keyof typeof EnumType.

api.StructuredQuery.CompositeFilter.Operator | keyof typeof api.StructuredQuery.CompositeFilter.Operator

MarkDuckworth · 2026-08-05T21:36:05Z

   * @internal
   */
-  public _getOperator(): api.StructuredQuery.CompositeFilter.Operator {
+  public _getOperator(): api.StructuredQuery.CompositeFilter.Operator | any {


Drop the ... | any

MarkDuckworth · 2026-08-05T22:55:25Z

+  }
+  export interface BundledQuery {
+    parent?: string | null;
+    structuredQuery?: any | null;


Suggested change

structuredQuery?: any | null;

structuredQuery?: google.firestore.v1.IStructuredQuery | null;

MarkDuckworth · 2026-08-05T22:56:04Z

+  }
+  export interface BundledDocumentMetadata {
+    name?: string | null;
+    readTime?: any | null;


Suggested change

readTime?: any | null;

readTime?: google.protobuf.ITimestamp | null;

MarkDuckworth · 2026-08-05T22:56:16Z

+  export interface NamedQuery {
+    name?: string | null;
+    bundledQuery?: BundledQuery | null;
+    readTime?: any | null;


Suggested change

readTime?: any | null;

readTime?: google.protobuf.ITimestamp | null;

MarkDuckworth · 2026-08-05T22:57:16Z

+  export interface BundledQuery {
+    parent?: string | null;
+    structuredQuery?: any | null;
+    limitType?: string | null;


Suggested change

limitType?: string | null;

limitType?: 'FIRST' | 'LAST' | null;

MarkDuckworth · 2026-08-05T22:57:48Z

+    createTime?: any | null;
+    version?: number | null;
+    totalDocuments?: number | null;
+    totalBytes?: number | null;


Suggested change

totalBytes?: number | null;

totalBytes?: number | string | null;

refactor(firestore): extract bundle serialization interface and begin…

e781fa2

… callsite modernization (#8928)

product-auto-label Bot added the api: firestore Issues related to the Firestore API. label Aug 4, 2026

gemini-code-assist Bot reviewed Aug 4, 2026

View reviewed changes

quirogas changed the title ~~refactor(firestore): extract bundle serialization interface and begin callsite modernization (#8928)~~ refactor(firestore): modernize internal proto call sites and remove local declarations (#8928) Aug 4, 2026

quirogas mentioned this pull request Aug 4, 2026

feat(firestore): decouple handwritten sdk wrapper from embedded gapic clients #8928

Open

quirogas changed the title ~~refactor(firestore): modernize internal proto call sites and remove local declarations (#8928)~~ refactor(firestore): modernize internal proto call sites and eliminate local declarations (#9076) Aug 5, 2026

This was referenced Aug 5, 2026

feat(firestore)!: complete gapic decoupling, adopt subpath imports, and remove legacy proto code (#9074) #9074

Open

chore(firestore): add automated presubmit serverless bundle size regression guard (#9075) #9075

Open

quirogas added 4 commits August 5, 2026 10:59

refactor(firestore): sync dependencies and bundle check improvements …

97af719

…from Layer 3 (#9076)

refactor(firestore): sync conformance harness proto discovery from La…

c55d54e

…yer 3 (#9076)

MarkDuckworth reviewed Aug 5, 2026

View reviewed changes

	structuredQuery?: any \| null;
	structuredQuery?: google.firestore.v1.IStructuredQuery \| null;

	readTime?: any \| null;
	readTime?: google.protobuf.ITimestamp \| null;

	limitType?: string \| null;
	limitType?: 'FIRST' \| 'LAST' \| null;

	totalBytes?: number \| null;
	totalBytes?: number \| string \| null;

Uh oh!

Conversation

quirogas commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Background & Motivation

Summary of Changes

Backward Compatibility Guarantee

📚 Stack Navigation Index

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

quirogas commented Aug 4, 2026 •

edited

Loading