Skip to content

fix(firestore): correct misleading cross-database reference log - #10250

Open
llamington wants to merge 1 commit into
firebase:mainfrom
llamington:fix/cross-db-reference-log-message
Open

fix(firestore): correct misleading cross-database reference log#10250
llamington wants to merge 1 commit into
firebase:mainfrom
llamington:fix/cross-db-reference-log-message

Conversation

@llamington

@llamington llamington commented Aug 3, 2026

Copy link
Copy Markdown

Fixes the log in AbstractUserDataWriter.convertDocumentKey so it stops naming the reference's target as though that document contained the reference.

Filed first as #10249, which has the details and a repro.

Before:

Document targets/the-target contains a document reference within a different database (demo-project-b/(default)) which is not supported. ...

After:

A document reference to targets/the-target refers to a different database (demo-project-b/(default)), which is not supported. ...

targets/the-target is what the reference points at. The document that actually holds it (containers/the-container in the repro) was never named, because both the database id and the document key are parsed from the reference value itself.

Testing

Nothing asserts this string anywhere in the repo, and only log copy changes, so no behaviour is affected.

I haven't added a test. Asserting on log wording would make any future rewording brittle, and the assertion actually worth having (that the log names the containing document) isn't satisfied by this change. Happy to add one if you'd prefer.

API changes

None. AbstractUserDataWriter is @internal and doesn't appear in common/api-review/firestore.api.md. No signatures change, and firestore-compat picks this up automatically since it calls convertDocumentKey rather than reimplementing it.

Naming the containing document would be the better fix, but it looks disproportionate for a diagnostic string: the key would have to be threaded through convertValueconvertObjectconvertObjectMap/convertArrayconvertReference (protected abstract, so every subclass changes) → convertDocumentKey, across both firestore and firestore-compat. It would also have to be optional, since aggregate_types.ts and pipeline-result.ts hold a writer for results that aren't document fields. Glad to attempt that instead if you'd rather.

Changeset included, patch bump on @firebase/firestore.

@llamington
llamington requested review from a team as code owners August 3, 2026 20:32
@changeset-bot

changeset-bot Bot commented Aug 3, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 56fffeb

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@firebase/firestore Patch
firebase Patch
@firebase/firestore-compat Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@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 corrects an error log message in @firebase/firestore when a document reference points to a different database. The log message was previously misleading, stating that the document itself contained the reference, whereas it is actually the target of the reference. A corresponding changeset has also been added. There are no review comments, so I have no feedback to provide.

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