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

Empty message content throws E_MISSING_MESSAGE #57

Closed
mateubo opened this issue Jul 3, 2017 · 7 comments
Closed

Empty message content throws E_MISSING_MESSAGE #57

mateubo opened this issue Jul 3, 2017 · 7 comments

Comments

@mateubo
Copy link
Contributor

mateubo commented Jul 3, 2017

Hi,
cldrjs get method throws E_MISSING_MESSAGE error when message content is an empty string.
{ "someProp": "" }.
I'm aware that an empty string is a "falsy" value, but still, it is a valid message content and it should be returned.

@mateubo mateubo changed the title Empty message content reported as Empty message content throws E_MISSING_MESSAGE Jul 3, 2017
@rxaviers
Copy link
Owner

rxaviers commented Jul 3, 2017

That might be a globalize thing since cldrjs doesn't throw that error.

@rxaviers rxaviers closed this as completed Jul 3, 2017
@rxaviers
Copy link
Owner

rxaviers commented Jul 3, 2017

Please, could you file an issue there?

@mateubo
Copy link
Contributor Author

mateubo commented Jul 3, 2017

Ah, sorry, the error is indeed thrown by globalize, but the problem is in cldrjs, specifically src/item/lookup.js and quite general if statements there that treat undefined, null, "" in the exactly the same way.

@rxaviers
Copy link
Owner

rxaviers commented Jul 3, 2017

Oh, ok. Reopening then... It indeed should return undefined if the path doesn't exist, but the actual value otherwise. Feel free you like to submit a PR and thanks!

@mateubo
Copy link
Contributor Author

mateubo commented Jul 12, 2017

Hi, I submitted a PR for this issue, would you consider accepting it and deploying a new release?

@rxaviers
Copy link
Owner

Sorry for the delay, just reviewed it.

@mateubo
Copy link
Contributor Author

mateubo commented Jul 21, 2017

I'm sorry for pushing this - I know we are in the middle of the holiday season, please review an updated PR whenever you have the time. Thanks!

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

No branches or pull requests

2 participants