appfeel/node-pushnotifications

title-loc-args and loc-args are expected to be arrays of strings for APN

Closed this issue · 6 comments

According to apple documentation, localization args like title-loc-args and loc-args are expected to be an array of strings, not a string of an array of strings. However, FCM documentation says the expected type is a JSON array of strings. I wasn't able to get the localization arguments working when using a JSON array of strings for APN, but it works when I use a regular array of strings. Can we JSON.parse the string to an array (which would not change the type signature) in the sendAPN.js file?

Hello @eli-zhang. Thanks for reporting this! I will look into this as soon as I find some time

@eli-zhang I think I understand the issue now, please correct me if I am mistaken:

So your proposed solution is to let the input type remain as string but parse that string to a JSON array in sendAPN and pass it as is in sendGCM ?

Yep, that's correct!

I actually spun up a fork of the repo to have a working version until you got around to this issue.
So far changing 'title-loc-args': data.titleLocArgs to 'title-loc-args': JSON.parse(data.titleLocArgs || "[]") and 'loc-args': data.locArgs || data.bodyLocArgs to 'loc-args': JSON.parse(data.locArgs || data.bodyLocArgs || "[]") in lib/sendAPN.js seems to have fixed the issue and the fix works properly for both Apple and Android.

It also doesn't break any type definitions in the @types/node-pushnotifications package!

@eli-zhang Can you test #151 and let me know if it works as expected on your end?

Tested and it works as expected! Thanks for resolving this.
As a side note - when testing your changes I had to clone your repo and change the folder names because your src folder is included in .npmignore. Is this intentional and is there a better way to go about testing changes?

@eli-zhang Thx for testing, I released the changes with v1.6.1

Not sure why the npmignore would influence the contents of the cloned directory? The src folder is included there because it is not needed for the published npm package but it should be included when you do a git clone