-
Notifications
You must be signed in to change notification settings - Fork 431
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Drop MultiJson in favor of stdlib JSON
- Loading branch information
1 parent
2afb881
commit b952ae0
Showing
4 changed files
with
6 additions
and
22 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
b952ae0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a way to use jbuilder with oj without multi_json?
b952ae0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use OJ instead of standard JSON? in the core jbuilder, or at least with some flag
As far as I know OJ is much faster than JSON library
b952ae0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should work:
config/initializers/oj.rb
require 'oj'
Oj.optimize_rails
b952ae0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Jan, 2 comments:
1- I think the require line is not needed, as bundler requires the gem by default, right?
2- I am still very curious to know why we switched to the ruby native JSON encoder instead of the multiJson or Oj if someone knows the reason
b952ae0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The purpose of
multi_json
is to allow the implementer to decide which JSON library to use, and avoids this exact issue. I think the documentation removed in this PR was a bit misleading about whatmulti_json
is for. Half a billion downloads speak pretty loudly.