Skip to content

Added non-null properties to database.ThenableReference#8800

Merged
hsubox76 merged 3 commits intofirebase:mainfrom
bookcreator:thenable-reference-types
Feb 27, 2025
Merged

Added non-null properties to database.ThenableReference#8800
hsubox76 merged 3 commits intofirebase:mainfrom
bookcreator:thenable-reference-types

Conversation

@rhodgkins
Copy link
Contributor

@rhodgkins rhodgkins commented Feb 18, 2025 •
edited
Loading

When using push to add data a ThenableReference is returned, this reference can never have an undefined key (or parent) because it's never going to be the root reference.

@changeset-bot
Copy link

changeset-bot bot commented Feb 18, 2025 •
edited
Loading

🦋 Changeset detected

Latest commit: 82c34b2

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

This PR includes changesets to release 4 packages
Name Type
@firebase/database-compat Patch
@firebase/database-types Patch
@firebase/database Patch
firebase 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

@rhodgkins rhodgkins force-pushed the thenable-reference-types branch from 28a00af to 8aeac68 Compare February 18, 2025 18:03
@rhodgkins

This comment was marked as resolved.

Sign in to view
@rhodgkins rhodgkins changed the title Added non-null properties to Database types Added non-null properties to database.ThenableReference Feb 18, 2025
@rhodgkins

This comment was marked as resolved.

Sign in to view
@hsubox76
Copy link
Contributor

hsubox76 commented Feb 19, 2025

I have seen that error before but don't remember the cause. I can't reproduce it with the current main branch, even on a fresh clone. What version of Node do you have? I'm using 22.13.1. You're only missing 3 commits from main that don't seem like they would affect anything so rebasing probably won't fix it.

@rhodgkins rhodgkins requested a review from a team as a code owner February 19, 2025 19:00
@rhodgkins
Copy link
Contributor Author

rhodgkins commented Feb 19, 2025

Eurgh typical, setup the container again to try again from fresh and now it's fine - I think was running the same node version (20.13.1) as before too... Oh well - thanks for your help anyway!

Do I need to include the firebase package in the change set?

@hsubox76
Copy link
Contributor

hsubox76 commented Feb 20, 2025

Changeset seems fine. I will see if a RTDB SDK maintainer can review for RTDB specific changes.

@hsubox76 hsubox76 requested a review from markarndt February 26, 2025 18:16
@hsubox76 hsubox76 merged commit 113c965 into firebase:main Feb 27, 2025
30 of 34 checks passed
@hsubox76
Copy link
Contributor

hsubox76 commented Feb 27, 2025

Thanks for the PR! We already cut the staging release for the current release so it might be another 3 weeks until this rolls out to NPM.

@google-oss-bot google-oss-bot mentioned this pull request Mar 18, 2025
Merged
@firebase firebase locked and limited conversation to collaborators Mar 30, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@maneesht maneesht maneesht approved these changes

@hsubox76 hsubox76 hsubox76 approved these changes

@markarndt markarndt markarndt approved these changes

@egilmorez egilmorez Awaiting requested review from egilmorez egilmorez is a code owner

@aashishpatil-g aashishpatil-g Awaiting requested review from aashishpatil-g aashishpatil-g is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@rhodgkins @hsubox76 @maneesht @markarndt