Skip to content
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

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

Closed
eli-zhang opened this issue Jan 25, 2021 · 6 comments · Fixed by #151
Closed

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

eli-zhang opened this issue Jan 25, 2021 · 6 comments · Fixed by #151
Assignees

Comments

@eli-zhang
Copy link

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?

@alex-friedl
Copy link
Collaborator

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

@alex-friedl alex-friedl self-assigned this Feb 4, 2021
@alex-friedl
Copy link
Collaborator

@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 ?

@eli-zhang
Copy link
Author

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!

@alex-friedl
Copy link
Collaborator

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

@eli-zhang
Copy link
Author

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?

@alex-friedl
Copy link
Collaborator

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants