Require JSON #65
Merged
Require JSON #65
Conversation
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
While writing an example program for #64 I noticed an error when not requiring JSON from the standard library: ``` gems/workos-0.9.1/lib/workos/client.rb:65:in `post_request': undefined method `to_json' for #<Hash:0x00007fa22e11b4f8> (NoMethodError) ``` Example: ```ruby require 'date' require 'workos' WorkOS.key = ENV.fetch('WORKOS_API_KEY') event = { action_type: 'r', action: 'user.login_succeeded', actor_id: 'user_01DEQWZNQT8Y47HDPSJKQS1J3F', actor_name: 'WorkOS Ruby Gem Test', group: 'foo-corp.com', latitude: '40.676300', longitude: '-73.949200', location: '65.215.8.114', occurred_at: DateTime.now.iso8601, target_id: 'user_01DEQWZNQT8Y47HDPSJKQS1J3F', target_name: 'WorkOS Ruby Gem Test' } WorkOS::AuditTrail.create_event(event: event) ``` I believe the code worked in my Rails app as a side effect of other dependencies and in the workos-ruby test suite due to `simplecov` gem depending on `json` gem as a development dependency. Files with `JSON` or `to_json` need to `require 'json'` for example programs like the snippets in API reference to work standalone: https://workos.com/docs/reference/audit-trail/event/publish Reduced further: ```ruby {}.to_json # undefined method `to_json' for {}:Hash (NoMethodError) JSON.parse("{}") # uninitialized constant JSON (NameError) ```
blairworkos
approved these changes
Dec 9, 2020
There was a problem hiding this comment.
We already require the JSON gem in the following files:
- https://github.com/workos-inc/workos-ruby/blob/master/lib/workos/connection.rb
- https://github.com/workos-inc/workos-ruby/blob/master/lib/workos/organization.rb
- https://github.com/workos-inc/workos-ruby/blob/master/lib/workos/profile.rb
I don't see an issue in adding it to the others. WDYT @willmanduffy or @marktran ?
willmanduffy
approved these changes
Dec 9, 2020
There was a problem hiding this comment.
This definitely works to resolve this issue. @croaky, I'm curious, is your use case also resolved if we put the require inside of lib/workos.rb once? That way all files in the WorkOS module should get access too.
When end users `require 'workos'`, the `lib` directory is added to Ruby's `$LOAD_PATH` by `workos.gemspec`: ```ruby spec.require_paths = ['lib'] ``` https://guides.rubygems.org/specification-reference/#require_paths= In a program typical of the WorkOS docs that begins like this... ``` require 'workos' WorkOS.key = ENV.fetch('WORKOS_API_KEY') ``` ...then, the `lib/workos.rb` file is loaded. This file's `WorkOS` module `autoloads` dependencies such as `WorkOS::Connection`. https://www.rubydoc.info/stdlib/core/Kernel%3Aautoload Previously, only dependencies like `SSO` that loaded `Connection`, which `require 'json'` would require the JSON library. So, continuing the above program... ```ruby event = # ... WorkOS::AuditTrail.create_event(event: event) ``` Was never loading any modules that required `'json'`.
|
Hi @willmanduffy, definitely! I pushed that change as a follow-up commit to the branch and added some detail about what I believe is mechanically happening to the commit message. |
blairworkos
pushed a commit
that referenced
this pull request
Jan 29, 2021
* Require JSON While writing an example program for #64 I noticed an error when not requiring JSON from the standard library: ``` gems/workos-0.9.1/lib/workos/client.rb:65:in `post_request': undefined method `to_json' for #<Hash:0x00007fa22e11b4f8> (NoMethodError) ``` Example: ```ruby require 'date' require 'workos' WorkOS.key = ENV.fetch('WORKOS_API_KEY') event = { action_type: 'r', action: 'user.login_succeeded', actor_id: 'user_01DEQWZNQT8Y47HDPSJKQS1J3F', actor_name: 'WorkOS Ruby Gem Test', group: 'foo-corp.com', latitude: '40.676300', longitude: '-73.949200', location: '65.215.8.114', occurred_at: DateTime.now.iso8601, target_id: 'user_01DEQWZNQT8Y47HDPSJKQS1J3F', target_name: 'WorkOS Ruby Gem Test' } WorkOS::AuditTrail.create_event(event: event) ``` I believe the code worked in my Rails app as a side effect of other dependencies and in the workos-ruby test suite due to `simplecov` gem depending on `json` gem as a development dependency. Files with `JSON` or `to_json` need to `require 'json'` for example programs like the snippets in API reference to work standalone: https://workos.com/docs/reference/audit-trail/event/publish Reduced further: ```ruby {}.to_json # undefined method `to_json' for {}:Hash (NoMethodError) JSON.parse("{}") # uninitialized constant JSON (NameError) ``` * Move JSON require to lib/workos.rb When end users `require 'workos'`, the `lib` directory is added to Ruby's `$LOAD_PATH` by `workos.gemspec`: ```ruby spec.require_paths = ['lib'] ``` https://guides.rubygems.org/specification-reference/#require_paths= In a program typical of the WorkOS docs that begins like this... ``` require 'workos' WorkOS.key = ENV.fetch('WORKOS_API_KEY') ``` ...then, the `lib/workos.rb` file is loaded. This file's `WorkOS` module `autoloads` dependencies such as `WorkOS::Connection`. https://www.rubydoc.info/stdlib/core/Kernel%3Aautoload Previously, only dependencies like `SSO` that loaded `Connection`, which `require 'json'` would require the JSON library. So, continuing the above program... ```ruby event = # ... WorkOS::AuditTrail.create_event(event: event) ``` Was never loading any modules that required `'json'`.
blairworkos
pushed a commit
that referenced
this pull request
Mar 3, 2021
* Require JSON While writing an example program for #64 I noticed an error when not requiring JSON from the standard library: ``` gems/workos-0.9.1/lib/workos/client.rb:65:in `post_request': undefined method `to_json' for #<Hash:0x00007fa22e11b4f8> (NoMethodError) ``` Example: ```ruby require 'date' require 'workos' WorkOS.key = ENV.fetch('WORKOS_API_KEY') event = { action_type: 'r', action: 'user.login_succeeded', actor_id: 'user_01DEQWZNQT8Y47HDPSJKQS1J3F', actor_name: 'WorkOS Ruby Gem Test', group: 'foo-corp.com', latitude: '40.676300', longitude: '-73.949200', location: '65.215.8.114', occurred_at: DateTime.now.iso8601, target_id: 'user_01DEQWZNQT8Y47HDPSJKQS1J3F', target_name: 'WorkOS Ruby Gem Test' } WorkOS::AuditTrail.create_event(event: event) ``` I believe the code worked in my Rails app as a side effect of other dependencies and in the workos-ruby test suite due to `simplecov` gem depending on `json` gem as a development dependency. Files with `JSON` or `to_json` need to `require 'json'` for example programs like the snippets in API reference to work standalone: https://workos.com/docs/reference/audit-trail/event/publish Reduced further: ```ruby {}.to_json # undefined method `to_json' for {}:Hash (NoMethodError) JSON.parse("{}") # uninitialized constant JSON (NameError) ``` * Move JSON require to lib/workos.rb When end users `require 'workos'`, the `lib` directory is added to Ruby's `$LOAD_PATH` by `workos.gemspec`: ```ruby spec.require_paths = ['lib'] ``` https://guides.rubygems.org/specification-reference/#require_paths= In a program typical of the WorkOS docs that begins like this... ``` require 'workos' WorkOS.key = ENV.fetch('WORKOS_API_KEY') ``` ...then, the `lib/workos.rb` file is loaded. This file's `WorkOS` module `autoloads` dependencies such as `WorkOS::Connection`. https://www.rubydoc.info/stdlib/core/Kernel%3Aautoload Previously, only dependencies like `SSO` that loaded `Connection`, which `require 'json'` would require the JSON library. So, continuing the above program... ```ruby event = # ... WorkOS::AuditTrail.create_event(event: event) ``` Was never loading any modules that required `'json'`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While writing an example program for
#64
I noticed an error when not requiring JSON from the standard library:
Example:
I believe the code worked in my Rails app as a side effect of other
dependencies and in the workos-ruby test suite due to
simplecovgemdepending on
jsongem as a development dependency.Files with
JSONorto_jsonneed torequire 'json'for exampleprograms like the snippets in API reference to work standalone:
https://workos.com/docs/reference/audit-trail/event/publish
Reduced further: