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

fredgalvao commented 7 years ago

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 https://github.com/phonegap/phonegap-plugin-push/pull/1736#issuecomment-303879546 , see source for more context.

jacquesdev commented 6 years ago

@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 commented 6 years ago

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

tomercagan commented 6 years ago

@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 commented 6 years ago

@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[bot] commented 6 years ago

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.

fredgalvao commented 6 years ago

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.

phonegap / phonegap-plugin-push