Remove opportunistic loading of yajl/json_gem
#78
Conversation
Requiring `yajl/json_gem` causes `JSON.parse`, `JSON.generate`, `#to_json`, and others built-in methods to be overridden with the faster YAJL implementation. Faster is, as a rule, better so this seems like a positive change overall. However, because this change extends outside of `gelf-rb` and impacts any place JSON is parsed or generated using the standard library interface, it can have surprising and unexpected consequences. When I updated gelf-rb to version 3.1.0, parts of my app started to behave differently. Specifically, U+2028 and U+2029 characters stopped being escaped within JSON strings. For a Rails app, this can be a problem [1]. (This will only impact folks who use YAJL without `require 'yajl/json_gem'`.) To remedy this, `yajl/json_gem` is no longer required by default. This makes the optimization of JSON parsing the responsibility of the app/script/person requiring `gelf-rb`. [1] http://timelessrepo.com/json-isnt-a-javascript-subset
|
|
|
@AlekSi does this seem like a change useful to the project? Right now, we maintain an empty file to make sure |
|
I have no idea, I stopped maintaining this gem (and using it, and using Ruby) years ago. |
|
Well, perhaps it would be a good idea to merge the change? Does anyone else have the ability to merge this in? @AlekSi ? That would be nice. "I don't know, I don't maintain this" is very unhelpful and results in a bunch of useless forks getting created for a project. The impact of this change is pretty significant and I think merging this would make 3.1.x usable for a wider audience. |
|
I don't have access to merge this change. I'm not a maintainer for a long time already. Sorry. /cc @lennartkoopmann |
|
Alexey,
Thanks. I did open up a separate issue asking who can maintain. 👌👍🏻
T
… On Mar 5, 2020, at 09:15, Alexey Palazhchenko ***@***.***> wrote:
I don't have access to merge this change. I'm not a maintainer for a long time already. Sorry.
/cc @lennartkoopmann
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
Requiring
yajl/json_gemcausesJSON.parse,JSON.generate,#to_json, and other built-in methods to be overridden with the faster YAJL implementation. Faster is, as a rule, better so this seems like a positive change overall. However, because this change extends outside ofgelf-rband impacts any place JSON is parsed or generated using the standard library interface, it can have surprising and unexpected consequences.When I updated gelf-rb to version 3.1.0, parts of my app started to behave differently. Specifically, U+2028 and U+2029 characters stopped being escaped within JSON strings. For a Rails app, which already overrides the standard library interface, this can be a problem [1]. (This will only impact folks who use YAJL without
require 'yajl/json_gem'.)To remedy this,
yajl/json_gemis no longer required by default. This makes the optimization of JSON parsing the responsibility of the app/script/person requiringgelf-rb.[1] http://timelessrepo.com/json-isnt-a-javascript-subset