Skip to content
This repository has been archived by the owner on Sep 4, 2020. It is now read-only.

Keep docs up-to-date (es6, fcm-node, json/quote) #1740

Open
fredgalvao opened this issue May 25, 2017 · 6 comments
Open

Keep docs up-to-date (es6, fcm-node, json/quote) #1740

fredgalvao opened this issue May 25, 2017 · 6 comments

Comments

@fredgalvao
Copy link
Collaborator

Expected Behaviour

  • Use standard modern javascript on the node.js examples
  • Use standard libraries from Firebase on the node.js examples
  • Use standard json notation when needed, and use homogeneous javascript object literal notation when needed.

Actual Behaviour

  • Lots of var usage
  • Usage of third party package fcm-node, when we now have official support from Firebase
  • Mixed quoted/unquoted object literal notation in JSON and javascript contexts

We should review the docs to make use of a more modern dialect of javascript when refering node.js context. var -> let is just the most visible change, there might be others.

Firebase node.js package

One of the forks of the fcm-node package now mentions the existence of a module officialy from Firebase that provides the functionality offered by fcm-node:

Warning: on February 2, 2017, the Firebase Team released the admin.messaging() service to their node.js admin module. This new service makes this module kind of deprecated (source)

Should the examples on payload be changed accordingly?
Docs on the new official node.js API is here

Copied verbatim from #1736 (comment) , see source for more context.

@fredgalvao fredgalvao changed the title Keep docs up-to-date Keep docs up-to-date (es6, fcm-node, json/quote) May 25, 2017
@fredgalvao fredgalvao added this to the Release 2.0.0 milestone May 25, 2017
@macdonst macdonst modified the milestones: Release 2.0.0, Release 2.1.0 Sep 6, 2017
@jacquesdev
Copy link
Contributor

@fredgalvao - is it possible that we can move the second task to a seperate issue?

  • Use standard libraries from Firebase on the node.js examples

Happy to update the es6 and json code in the examples, but have no idea about Firebase have never used it before.

@fredgalvao
Copy link
Collaborator Author

@jacquesdev Please do! There's nothing that prevents these inner tasks to be broken and done separately.

@tomercagan
Copy link

tomercagan commented Nov 11, 2017

@fredgalvao -about official Firebase lib - I started using it and it is quite opinionated about how the message is sent - it currently supports only string properties so anything else (include, for example, the ledColor) must be stringy-fied before sending.

I'd be happy to share my experience/code as I gain familiarity with admin-fcm (not a great markdown writer or an expert node js - but whatever I can do help, I'd happily try to...

Maybe a different issues is indeed in order?

P.S. I search for both admin-fcm and "value must be string" (which is the error I get using the official library) but didn't find any issue here (and in SO either). If there is such issue - sorry for "spamming" here (and let me know what it is so I can learn).

@fredgalvao
Copy link
Collaborator Author

@tomercagan I'm not at all familiar with the details regarding fcm-node, however I might agree that said project being so (mis)opinionated might be an issue for some attributes on our API. However, I'd guess that we could still document most of the usages with some version or workaround using fcm-node or whatever is the most up-to-date lib.

not a great markdown writer

Don't worry about that if you want to help document the plugin. We can always review and improve on formatting on a pull request, for example.

Maybe a different issues is indeed in order?

We can always break this into smaller issues and separate PRs, there's no holding back.

@stale
Copy link

stale bot commented Jun 3, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jun 3, 2018
@fredgalvao
Copy link
Collaborator Author

The js code was rewritten for a transpiler in mind, and the json/js portions of the doc have been reviewed multiple times since then.
@macdonst do you still think we should update the docs to use a more up-to-date node lib (in the examples)? Or is the current usage on par? This is the last point left, and if it's not an issue, then we can close this.

@stale stale bot removed the wontfix label Jun 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

4 participants