Fix the type annotations in the RTDB Query.on/off/once API. #1204
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
schmidt-sebastian
changed the title
schmidt-sebastian
force-pushed
the
mrschmidt-nullability
branch
from
dffba65 to
caa91c8
Compare
schmidt-sebastian
changed the title
mikelehen
suggested changes
Sep 6, 2018
packages/database-types/index.d.ts
Outdated
| callback?: (a: DataSnapshot, b?: string | null) => any, | ||
| context?: Object | null | ||
| ): any; | ||
| callback?: (a: DataSnapshot, b?: string) => any, |
There was a problem hiding this comment.
b (aka prevChildName) will be null in the case of child_added events where there's no predecessor.
packages/database-types/index.d.ts
Outdated
| cancelCallbackOrContext?: Object | null, | ||
| context?: Object | null | ||
| ): (a: DataSnapshot | null, b?: string) => any; | ||
| callback: (a: DataSnapshot, b?: string) => any, |
There was a problem hiding this comment.
b (aka prevChildName) will be null in the case of child_added events where there's no predecessor.
| @@ -65,7 +65,7 @@ export class DataEvent implements Event { | |||
| | ' child_removed', | |||
| public eventRegistration: EventRegistration, | |||
| public snapshot: DataSnapshot, | |||
| public prevName?: string | null | |||
| public prevName?: string | |||
There was a problem hiding this comment.
Hm, you are obviously right here and I fixed the typing to allow for this value to be null, but I am a little confused why
strictNullChecks.
schmidt-sebastian
requested review from
Feiyang1,
hiranya911,
hsubox76 and
jsdt
as code owners
schmidt-sebastian
force-pushed
the
mrschmidt-nullability
branch
2 times, most recently
from
1e05d59 to
6ed946c
Compare
schmidt-sebastian
force-pushed
the
mrschmidt-nullability
branch
from
6ed946c to
4b9d19a
Compare
| @@ -65,7 +65,7 @@ export class DataEvent implements Event { | |||
| | ' child_removed', | |||
| public eventRegistration: EventRegistration, | |||
| public snapshot: DataSnapshot, | |||
| public prevName?: string | null | |||
| public prevName?: string | |||
There was a problem hiding this comment.
Hm, you are obviously right here and I fixed the typing to allow for this value to be null, but I am a little confused why
strictNullChecks.
|
FYI- Still LGTM after changelog addition. |
|
This will be merged into the Firebase I/O release as it contains minor breaking changes. |
|
@Feiyang1 Can you merge this with the version bump? |
schmidt-sebastian
added a commit
that referenced
this pull request
Apr 24, 2019
Feiyang1
pushed a commit
that referenced
this pull request
May 1, 2019
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes the nullability annotations in the RTDB Query.on/off API. The callback can be optional, but not the DataSnapshot. This is a breaking typing cleanup (as some types used to take "null"), but this updates the TS types to match the JS reference docs.
Fixes: #1188