-
Notifications
You must be signed in to change notification settings - Fork 927
Fix deprecation documentation #5286
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
Conversation
|
Changeset File Check
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be backticked, or will it be rendered as code font without them?
Size Analysis Report |
packages/firebase/index.d.ts
Outdated
@@ -7419,7 +7419,7 @@ declare namespace firebase.messaging { | |||
* | |||
* @param callback The function to handle the push message. | |||
* | |||
* @deprecated onBackgroundMessage(nextOrObserver: firebase.NextFn<MessagePayload>| | |||
* @deprecated Use onBackgroundMessage(nextOrObserver: firebase.NextFn<MessagePayload>| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, will this fix the way this item gets conflated with the following Observer-Unsubscribe item?
Ideally we'd have a way to gen the docs and eyeball that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those all belong together, the Observer and error are part of the signature and the Unsubscribe is the return type. There could be some spaces before the |
and after the comma after errorFn,
. But actually, do we need the whole signature here, can we just say "Use onBackgroundMessage()" or "Use {@link firebase.messaging.Messaging.onBackgroundMessage}"?
Docs can be generated locally by running
yarn
yarn docgen:js
at the root of the repo.
After running that, the affected html file will be found at scripts/docgen/html/js/firebase.messaging.Messaging.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added some spaces.
Don't know if should use the full method signature of {@link firebase.messaging.Messaging.onBackgroundMessage}
. If later, need to change other ones.
Deprecated method doc should start with "Use ...".
This will fix: b/170682742