search

rails / jbuilder

Jbuilder: generate JSON objects with a Builder-style DSL
MIT License
4.34k stars 440 forks source link

Remove OpenStruct references from Jbuilder #567

Closed mtsmfm closed 2 months ago

mtsmfm commented 6 months ago

While I was looking at https://github.com/rails/jbuilder/pull/562, I noticed OpenStruct never responses map and count method.

`docker run rubylang/all-ruby ./all-ruby -e 'require "ostruct"; p [:map, :count].map {|m| OpenStruct.new({}).respond_to?(m) }'` ``` ruby-1.6.0 [false, false] ... ruby-3.3.1 [false, false] ``` ``` ruby-0.49 -e:1: syntax error -e:1: syntax error -e:1: syntax error -e:1: syntax error -e:1: syntax error -e:1: syntax error -e:1: syntax error -e:1: syntax error -e:1: syntax error exit 9 ruby-0.50 -e:1: syntax error -e:1: syntax error -e:1: syntax error -e:1: syntax error -e:1: syntax error -e:1: syntax error -e:1: syntax error -e:1: syntax error -e:1: syntax error exit 9 ruby-0.51 -e:1: syntax error exit 1 ... ruby-0.76 -e:1: syntax error exit 1 ruby-0.95 -e:1:in `require': No such file to load -- ostruct exit 1 ruby-0.99.4-961224 -e:1:in `require': No such file to load -- ostruct from -e:1 exit 1 ruby-1.0-961225 -e:1:in `require': No such file to load -- ostruct from -e:1 exit 1 ruby-1.0-971002 -e:1:in `require': LoadError: No such file to load -- ostruct from -e:1 exit 1 ... ruby-1.0-971225 -e:1:in `require': LoadError: No such file to load -- ostruct from -e:1 exit 1 ruby-1.1a0 -e:1:in `require': LoadError| No such file to load -- ostruct from -e:1 exit 1 ruby-1.1a1 -e:1:in `require': LoadError| No such file to load -- ostruct from -e:1 exit 1 ruby-1.1a2 -e:1:in `require': LoadError:No such file to load -- ostruct from -e:1 exit 1 ruby-1.1a3 -e:1:in `require': LoadError: No such file to load -- ostruct from -e:1 exit 1 ... ruby-1.1a9 -e:1:in `require': LoadError: No such file to load -- ostruct from -e:1 exit 1 ruby-1.1b0 -e:1:in `require': No such file to load -- ostruct (LoadError) from -e:1 exit 1 ruby-1.1b1 -e:1:in `require': No such file to load -- ostruct (LoadError) from -e:1 exit 1 ruby-1.1b2 -e:1: undefined iterator `map' for [8017, 8025](Array) (NameError) exit 1 ruby-1.1b3 -e:1: undefined iterator `map' for [8017, 8025](Array) (NameError) exit 1 ruby-1.1b4 -e:1: undefined iterator `map' for [8017, 8025] (NameError) exit 1 ruby-1.1b5 -e:1: undefined iterator `map' for [8025, 8033] (NameError) exit 1 ruby-1.1b6 -e:1: undefined iterator `map' for [8001, 8009] (NameError) exit 1 ruby-1.1b7 -e:1: undefined iterator `map' for [8065, 8073] (NameError) exit 1 ruby-1.1b8 -e:1: undefined iterator `map' for [8081, 8089] (NameError) exit 1 ruby-1.1b9 -e:1: undefined iterator `map' for [8081, 8089] (NameError) exit 1 ruby-1.1b9_01 -e:1: undefined iterator `map' for [8121, 8129] (NameError) exit 1 ... ruby-1.1b9_03 -e:1: undefined iterator `map' for [8121, 8129] (NameError) exit 1 ruby-1.1b9_04 -e:1: undefined iterator `map' for [8145, 8153] (NameError) exit 1 ruby-1.1b9_05 -e:1: undefined iterator `map' for [8161, 8169] (NameError) exit 1 ruby-1.1b9_06 -e:1: undefined iterator `map' for [8177, 8185] (NameError) exit 1 ruby-1.1b9_07 -e:1: undefined iterator `map' for [8241, 8249] (NameError) exit 1 ruby-1.1b9_08 -e:1: undefined iterator `map' for [8249, 8257] (NameError) exit 1 ruby-1.1b9_09 -e:1: undefined iterator `map' for [8241, 8249] (NameError) exit 1 ruby-1.1b9_10 -e:1: undefined iterator `map' for [8265, 8273] (NameError) exit 1 ruby-1.1b9_11 -e:1: undefined iterator `map' for [8281, 8289] (NameError) exit 1 ruby-1.1b9_12 -e:1: undefined iterator `map' for [8289, 8297] (NameError) exit 1 ... ruby-1.1b9_14 -e:1: undefined iterator `map' for [8289, 8297] (NameError) exit 1 ruby-1.1b9_15 -e:1: undefined iterator `map' for [8305, 8313] (NameError) exit 1 ruby-1.1b9_16 -e:1: undefined iterator `map' for [8321, 8329] (NameError) exit 1 ruby-1.1b9_17 /tmp/rbeOxo3r:1: undefined iterator `map' for [8353, 8361] (NameError) exit 1 ruby-1.1b9_18 /tmp/rbLXgKTu:1: undefined iterator `map' for [8361, 8369] (NameError) exit 1 ruby-1.1b9_19 /tmp/rbzff2Rx:1: NameError exit 1 ruby-1.1b9_20 /tmp/rbusengG:1: NameError exit 1 ruby-1.1b9_21 /tmp/rb6y6OiI:1: undefined iterator `map' for [8361, 8369] (NameError) exit 1 ruby-1.1b9_22 /tmp/rb2ECrEP:1: undefined iterator `map' for [8361, 8369] (NameError) exit 1 ruby-1.1b9_23 /tmp/rbRjC3NS:1: undefined iterator `map' for [8377, 8385] (NameError) exit 1 ruby-1.1b9_24 /tmp/rbe7hi1Y:1: undefined iterator `map' for [8393, 8401] (NameError) exit 1 ruby-1.1b9_25 /tmp/rbd1IlwZ:1: undefined iterator `map' for [8393, 8401] (NameError) exit 1 ruby-1.1b9_26 /tmp/rbaCV6E5:1: undefined iterator `map' for [8417, 8425] (NameError) exit 1 ruby-1.1b9_27 /tmp/rbtsOvQc:1: undefined iterator `map' for [8417, 8425] (NameError) exit 1 ruby-1.1b9_28 /tmp/rboLvSeg:1: undefined iterator `map' for [8417, 8425] (NameError) exit 1 ruby-1.1b9_29 /tmp/rb2jEekm:1: undefined iterator `map' for [8433, 8441] (NameError) exit 1 ruby-1.1b9_30 /tmp/rbKwAYsn:1: undefined iterator `map' for [8433, 8441] (NameError) exit 1 ruby-1.1b9_31 /tmp/rbxT9q7t:1: undefined iterator `map' for [8433, 8441] (NameError) exit 1 ruby-1.1b9_32 /tmp/rbqIT5Yz:1: undefined iterator `map' for [8433, 8441] (NameError) exit 1 ruby-1.1c0 /tmp/rbx3kWwB:1: undefined iterator `map' for [8433, 8441] (NameError) exit 1 ruby-1.1c1 /tmp/rbkA6tQG:1: undefined iterator `map' for [8433, 8441] (NameError) exit 1 ruby-1.1c2 /tmp/rbjBlWzO:1: undefined iterator `map' for [8433, 8441] (NameError) exit 1 ruby-1.1c3 /tmp/rbhSOmQO:1: undefined iterator `map' for [8433, 8441] (NameError) exit 1 ruby-1.1c4 /tmp/rbcyRhDW:1: undefined iterator `map' for [8433, 8441] (NameError) exit 1 ruby-1.1c5 /tmp/rbDCXRs0:1: undefined iterator `map' for [8433, 8441] (NameError) exit 1 ruby-1.1c6 /tmp/rbGaLFU3:1: undefined iterator `map' for [8161, 8169] (NameError) exit 1 ruby-1.1c7 /tmp/rbEJdNUa:1: undefined iterator `map' for [8169, 8177] (NameError) exit 1 ruby-1.1c8 /tmp/rbqSgfCg:1: undefined iterator `map' for [8737, 8745] (NameError) exit 1 ruby-1.1c9 /tmp/rb8cxY4g:1: undefined iterator `map' for [8169, 8177] (NameError) exit 1 ruby-1.1d0 /tmp/rbE2S8lo:1: undefined iterator `map' for [8241, 8249] (NameError) exit 1 ruby-1.1d1 /tmp/rbVN0Fet:1: undefined iterator `map' for [8241, 8249] (NameError) exit 1 ruby-1.2 /tmp/rbZn9F4u:1: undefined iterator `map' for [8169, 8177] (NameError) exit 1 ruby-1.2.1 /tmp/rbgpMTvB:1: undefined iterator `map' for [8169, 8177] (NameError) exit 1 ruby-1.2.2 /tmp/rbsm7VeF:1: undefined iterator `map' for [8169, 8177] (NameError) exit 1 ruby-1.2.3 /tmp/rbfnaYEL:1: undefined iterator `map' for [8169, 8177] (NameError) exit 1 ruby-1.2.4 /tmp/rbXrp0aQ:1: undefined iterator `map' for [8169, 8177] (NameError) exit 1 ruby-1.2.5 /tmp/rbw3le0S:1: undefined iterator `map' for [8169, 8177] (NameError) exit 1 ruby-1.2.6 /tmp/rbuoXmWY:1: undefined iterator `map' for [8169, 8177] (NameError) exit 1 ruby-1.3 /tmp/rbH4Vxa2:1: undefined iterator `map' for [8241, 8249] (NameError) exit 1 ruby-1.3.1-990215 /tmp/rbJiv876:1: undefined iterator `map' for [8409, 8417] (NameError) exit 1 ruby-1.3.1-990224 /tmp/rbGx9f0d:1: undefined iterator `map' for [8417, 8425] (NameError) exit 1 ruby-1.3.1-990225 /tmp/rbpkHQFh:1: undefined iterator `map' for [8417, 8425] (NameError) exit 1 ruby-1.3.1-990311 /tmp/rbEhjCXo:1: undefined iterator `map' for [8473, 8481] (NameError) exit 1 ruby-1.3.1-990315 /tmp/rbrcYwYq:1: undefined iterator `map' for [8497, 8505] (NameError) exit 1 ruby-1.3.1-990324 /tmp/rb5U9Lmu:1: undefined iterator `map' for [8513, 8521] (NameError) exit 1 ruby-1.3.2-990402 /tmp/rbJp4WPy:1: undefined iterator `map' for [8529, 8537] (NameError) exit 1 ruby-1.3.2-990405 /tmp/rbmhN9zD:1: undefined iterator `map' for [8529, 8537] (NameError) exit 1 ruby-1.3.2-990408 /tmp/rbDtyMjL:1: undefined iterator `map' for [8529, 8537] (NameError) exit 1 ruby-1.3.2-990413 /tmp/rbzOHQlP:1: undefined iterator `map' for [8529, 8537] (NameError) exit 1 ruby-1.3.3-990430 /tmp/rbFjs8TT:1: undefined iterator `map' for [8537, 8545] (NameError) exit 1 ruby-1.3.3-990507 /tmp/rbP7X9VX:1: undefined iterator `map' for [8537, 8545] (NameError) exit 1 ruby-1.3.3-990513 /tmp/rbuVw2S3:1: undefined iterator `map' for [8537, 8545] (NameError) exit 1 ruby-1.3.3-990518 /tmp/rbkTUut6:1: undefined iterator `map' for [8545, 8553] (NameError) exit 1 ruby-1.3.4-990531 /tmp/rbAKdS4b:1: undefined iterator `map' for [8553, 4225] (NameError) exit 1 ruby-1.3.4-990611 /tmp/rb1IMDCf:1: undefined iterator `map' for [8585, 4233] (NameError) exit 1 ruby-1.3.4-990624 /tmp/rbHKJQ8k:1: undefined iterator `map' for [8585, 4233] (NameError) exit 1 ruby-1.3.4-990625 /tmp/rbatRIFp:1: undefined iterator `map' for [8585, 4233] (NameError) exit 1 ruby-1.3.5 /tmp/rbV1nV7t:1: undefined method `map' for [8585, 4233]:Array (NameError) exit 1 ruby-1.3.6 /tmp/rbmbWPQy:1: undefined method `map' for [8585, 4233]:Array (NameError) exit 1 ruby-1.3.7 /tmp/rbx0D9fG:1: undefined method `map' for [8593, 4233]:Array (NameError) exit 1 ruby-1.4.0 /tmp/rbTiBWPH:1: undefined method `map' for [8593, 4233]:Array (NameError) exit 1 ruby-1.4.1 /tmp/rbatNtEO:1: undefined method `map' for [8593, 4233]:Array (NameError) exit 1 ruby-1.4.2 /tmp/rbbhuiuQ:1: undefined method `map' for [8593, 4233]:Array (NameError) exit 1 ruby-1.4.3 /tmp/rbW4ACmW:1: undefined method `map' for [8593, 4233]:Array (NameError) exit 1 ruby-1.4.4 /tmp/rb5aEcr3:1: undefined method `map' for [8593, 4233]:Array (NameError) exit 1 ruby-1.4.5 /tmp/rbjJF5T6:1: undefined method `map' for [8601, 4233]:Array (NameError) exit 1 ruby-1.4.6 /tmp/rbJxuXgb:1: undefined method `map' for [8601, 4233]:Array (NameError) exit 1 ruby-1.6.0 [false, false] ... ruby-3.3.1 [false, false] ```

So, even we remove OpenStruct related codes completely, Jbuilder can handle them

require 'bundler/inline'

gemfile do
  source 'https://rubygems.org'

  if ENV['FORK']
    gem 'jbuilder', path: '.'
  else
    gem 'jbuilder'
  end
end

puts "Jbuilder::NON_ENUMERABLES: #{Jbuilder.const_defined?("NON_ENUMERABLES")}"

require 'ostruct'

ostruct = OpenStruct.new(a: 1)
struct = Struct.new(:a).new(a: 1)

array = [{a: 1}]

result = Jbuilder.new do |json|
  json.array array, :a
  json.struct struct, :a
  json.ostruct ostruct, :a
  json.array_struct [struct], :a
  json.array_ostruct [ostruct], :a
end.target!

if JSON.parse(result) == {"array"=>[{"a"=>1}], "struct"=>{"a"=>1}, "ostruct"=>{"a"=>1}, "array_struct"=>[{"a"=>1}], "array_ostruct"=>[{"a"=>1}]}
  puts 'Passed'
else
  puts 'Failed'
end

__END__
$ ruby foo.rb
Jbuilder::NON_ENUMERABLES: true
Passed

$ FORK=1 ruby foo.rb
Jbuilder::NON_ENUMERABLES: false
Passed

But if an OpenStruct instance has both map and count fields, it would be broken.

require 'bundler/inline'

gemfile do
  source 'https://rubygems.org'

  if ENV['FORK']
    gem 'jbuilder', path: '.'
  else
    gem 'jbuilder'
  end
end

puts "Jbuilder::NON_ENUMERABLES: #{Jbuilder.const_defined?("NON_ENUMERABLES")}"

require 'ostruct'

ostruct = OpenStruct.new(a: 1, count: 1, map: 1)

result = Jbuilder.new do |json|
  json.ostruct ostruct, :a
end.target!

if JSON.parse(result) == {"ostruct"=>{"a"=>1}}
  puts 'Passed'
else
  puts 'Failed'
end

__END__

$ ruby broken.rb
Jbuilder::NON_ENUMERABLES: true
Passed

$ FORK=1 ruby broken.rb
Jbuilder::NON_ENUMERABLES: false
/workspaces/jbuilder/lib/jbuilder.rb:339:in `-': Array can't be coerced into Integer (TypeError)

    end - [BLANK]
          ^^^^^^^
        from /workspaces/jbuilder/lib/jbuilder.rb:339:in `_map_collection'
        from /workspaces/jbuilder/lib/jbuilder.rb:217:in `array!'
        from /workspaces/jbuilder/lib/jbuilder.rb:56:in `block in set!'
        from /workspaces/jbuilder/lib/jbuilder.rb:345:in `_scope'
        from /workspaces/jbuilder/lib/jbuilder.rb:56:in `set!'
        from /workspaces/jbuilder/lib/jbuilder.rb:70:in `method_missing'
        from broken.rb:20:in `block in <main>'
        from /workspaces/jbuilder/lib/jbuilder.rb:21:in `initialize'
        from broken.rb:19:in `new'
        from broken.rb:19:in `<main>'

So, strictly speaking, this change is a breaking change.

What do you think?

If you don't agree with this change, you can just close this PR. I just want to tell you another possibility.

mtsmfm commented 6 months ago

With Ruby 3.4, a newly created Rails application immediately shows a warning about ostruct

$ docker run --rm -it ruby:3.4.0-preview1 bash -c "gem install rails && rails new foo && cd foo && echo test && bin/rails r 'puts %(hi)'"
(snip)
test
/usr/local/bundle/gems/bootsnap-1.18.3/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:30: warning: /usr/local/lib/ruby/3.4.0+0/ostruct.rb was loaded from the standard library, but will no longer be part of the default gems since Ruby 3.5.0. Add ostruct to your Gemfile or gemspec.
hi

With this patch, such warning won't be appeared since it completely removes the require thing for ostruct.

$ docker run --rm -it ruby:3.4.0-preview1 bash -c "gem install rails && rails new foo && cd foo && sed -i 's;gem \"jbuilder\";gem \"jbuilder\", github: \"mtsmfm/jbuilder\", branch: \"remove-ostruct\";' Gemfile && bundle install && echo test && bin/rails r 'puts %(hi)'"
(snip)
test
hi
yahonda commented 5 months ago

As a author of https://github.com/rails/jbuilder/pull/562 , I think this pull request should be better because #562 would support Ruby 3.5 that will remove ostruct. however, it will still show the warning below when Ruby 3.4 user executes rails new with Jbuilder.

`ostruct.rb was loaded from the standard library, but will no longer be part of the default gems since Ruby 3.5.0. Add ostruct to your Gemfile or gemspec.
yahonda commented 5 months ago

I would also like to hear from other maintainers.

mattbrictson commented 2 months ago

it will still show the warning below when Ruby 3.4

It looks like Ruby 3.3.5 (released today) emits the warning as well.

/Users/mbrictson/.rbenv/versions/3.3.5/lib/ruby/3.3.0/bundled_gems.rb:75: warning: /Users/mbrictson/.rbenv/versions/3.3.5/lib/ruby/3.3.0/ostruct.rb was loaded from the standard library, but will no longer be part of the default gems starting from Ruby 3.5.0. You can add ostruct to your Gemfile or gemspec to silence this warning. Also please contact the author of jbuilder-2.12.0 to request adding ostruct into its gemspec.

yahonda commented 2 months ago

It looks like Ruby 3.3.5 (released today) emits the warning as well.

According to https://bugs.ruby-lang.org/issues/20737 , this warning will not appear in Ruby 3.3.6 and future 3.3 versions of Rubies.