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

Allow raw/native JSON in JS config object, e.g. checkIgnore function #821

Merged
merged 1 commit into from
Feb 18, 2019

Conversation

waltjones
Copy link
Contributor

@waltjones waltjones commented Feb 13, 2019

Fixes #648
Fixes #710

Use Rollbar::JSON::Value to pass native JSON values, without enclosing string quotes.

Example:

Rollbar.configure do |config|
  config.js_enabled = true
  config.js_options = {
    accessToken: "POST_CLIENT_ITEM_ACCESS_TOKEN",
    checkIgnore: Rollbar::JSON::Value.new('function(){ alert("bar") }')
  }
end

Ruby JSON.generate vs. ActiveSupport #to_json and #as_json

When serializing the js_options object, the middleware now uses Ruby JSON.generate instead of ActiveSupport's #to_json.

Early versions of ActiveSupport could use the #encode_json method to get a similar ability to inject values without enclosing quote marks.

ActiveSupport before version 4.1.0 supported the #encode_json method, and then the activesupport-json_encoder gem did for Rails < 5.x. For Rails 5.x, the desired functionality doesn't appear achievable via ActiveSupport. For all versions, the dependencies and requirements are fragmented, and the functionality would not work at all on a Rack app without ActiveSupport.

The native Ruby JSON#generate works consistently across all versions of Ruby and imposes no dependencies on users of the gem. The encoder requirement is only on the generation of the js_options object in the middleware and has no impact on other JSON objects in the gem, or on the host application.

Copy link
Contributor

@ArturMoczulski ArturMoczulski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :) Feel free to merge

@waltjones waltjones merged commit 1f1c736 into master Feb 18, 2019
@waltjones waltjones deleted the wj-js-check-ignore branch March 8, 2019 18:44
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

Successfully merging this pull request may close these issues.

None yet

2 participants