Skip to content

build with jsoninter #1774

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

Closed
wants to merge 1 commit into from
Closed

build with jsoninter #1774

wants to merge 1 commit into from

Conversation

arun0009
Copy link
Contributor

Allowing users to replace default encoding/json with jsoniter and build binary by using build tags(Instructions added to README).

Resolves:
#1698
#1204

@aldas
Copy link
Contributor

aldas commented Feb 11, 2021

Not really sure if it is the best solution.

Underlying problem is that currently library user does not have way to choose JSON implementation by themselves (centrally). They may not even be interested in json = jsoniter.ConfigCompatibleWithStandardLibrary as I would assume the reason for using different implementation (whether jsoninter or not) is performance gains over "StandardLibrary-like" implementations.

Maybe it would be better just change all places where encoders/decoders are created with calls to functions (on that object) that return json encoder/decoder needed and user is able to replace this Echo defaultJSONsomething with their own choosing.

That way we would not depend another library (and their problems/vulnerabilities)

@aldas aldas requested review from lammel, pafuent, aldas and vishr February 11, 2021 11:58
@arun0009
Copy link
Contributor Author

@aldas - I like the idea of default json and a way to override it with anything user can pass when configuring echo. This way we don't have to include additional libraries into echo.

@pafuent
Copy link
Contributor

pafuent commented Feb 19, 2021

Maybe it would be better just change all places where encoders/decoders are created with calls to functions (on that object) that return json encoder/decoder needed and user is able to replace this Echo defaultJSONsomething with their own choosing.

I agree, it sounds a simpler solution that will give more freedom to Echo users, and will be less headaches for us 😉

@aldas
Copy link
Contributor

aldas commented Jun 2, 2021

I am closing this PR and we will be merging #1880 to allow different implementations

@aldas aldas closed this Jun 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants