Skip to content

Update rtdb types #840

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 18, 2018
Merged

Update rtdb types #840

merged 2 commits into from
May 18, 2018

Conversation

jspri
Copy link
Contributor

@jspri jspri commented May 18, 2018

Updated some of the type definitions.

Fixes #306, #555.

@schmidt-sebastian
Copy link
Contributor

Thanks for sending this over. Due to how our repo is structured, these changes need to be replicated in one more place (https://github.com/firebase/firebase-js-sdk/blob/dd11cd17aada45a19732d6517a7d740e722546d2/packages/firebase/index.d.ts). I will do this myself once CI passes and this PR is merged.

schmidt-sebastian added a commit that referenced this pull request May 18, 2018
This is a follow-up PR to #840
@schmidt-sebastian schmidt-sebastian merged commit 3fa62ef into firebase:master May 18, 2018
@jspri
Copy link
Contributor Author

jspri commented May 21, 2018

Thanks!

@jspri jspri deleted the patch-1 branch May 21, 2018 01:35
@jamesdaniels
Copy link
Member

FYI while being more explicit is useful, mind that changes such as this are breaking changes for typescript users. https://travis-ci.org/angular/angularfire2/jobs/386001074

@jamesdaniels
Copy link
Member

@schmidt-sebastian please watch out for what should be major changes to the types in future PRs. Angularfire is still in RC still so I'm not stressing out too much but this could break the build for anyone else using TS.

IMO correct thing to do would be:

// TODO(semver): drop "string" in 6.0 release
type EventType = string | 'value' | 'child_added' | 'child_changed' | 'child_moved' | 'child_removed';

@firebase firebase locked and limited conversation to collaborators Oct 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Updates to Database Type Definitions
4 participants