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

Consider replacing another-json dependency with native JSON #291

Closed
fred-wang opened this issue Nov 16, 2016 · 3 comments
Closed

Consider replacing another-json dependency with native JSON #291

fred-wang opened this issue Nov 16, 2016 · 3 comments

Comments

@fred-wang
Copy link
Contributor

It seems that the another-json dependency is only used in the matrix-js-sdk/lib/crypto/index.js file. Modern web engines have native support for JSON:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON#Browser_compatibility

I think it would be great to provide an option to use native JSON instead of native-json, similar to what is done for "request". That will allow to reduce a bit the dep tree, see #244

@fred-wang
Copy link
Contributor Author

Interestingly, other core files in matrix-js-sdk/lib/crypto/ already uses native JSON support, so I'm not sure why it is different for matrix-js-sdk/lib/crypto/index.js

@fred-wang
Copy link
Contributor Author

OK, I guess the rationale to use another-json is to have a "canonical form":
"Properties of non-array objects are not guaranteed to be stringified in any particular order. Do not rely on ordering of properties within the same object within the stringification."
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#Description

@kegsay
Copy link
Member

kegsay commented Nov 16, 2016

Yup, this is exactly why we use another-json.

@kegsay kegsay closed this as completed Nov 16, 2016
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