rubyjs / therubyracer

Embed the V8 Javascript Interpreter into Ruby
1.67k stars 190 forks source link

Failed to load gem for Ruby 2.4.0 (related to the new Integer class) #428

Closed ollie closed 7 years ago

ollie commented 7 years ago

Hello,

I wanted to give our Rails app a try on Ruby 2.4.0 and got this error:

Gem Load Error is: wrong argument type Class (expected Module)
Backtrace for gem load error is:
/Users/olda/.rbenv/versions/2.4.0/lib/ruby/gems/2.4.0/gems/therubyracer-0.12.2/lib/v8/conversion.rb:26:in `include'
/Users/olda/.rbenv/versions/2.4.0/lib/ruby/gems/2.4.0/gems/therubyracer-0.12.2/lib/v8/conversion.rb:26:in `block (2 levels) in <top (required)>'
/Users/olda/.rbenv/versions/2.4.0/lib/ruby/gems/2.4.0/gems/therubyracer-0.12.2/lib/v8/conversion.rb:22:in `class_eval'
/Users/olda/.rbenv/versions/2.4.0/lib/ruby/gems/2.4.0/gems/therubyracer-0.12.2/lib/v8/conversion.rb:22:in `block in <top (re
...

So I investigated.

# conversion.rb
for type in [Class, Object, Array, Hash, String, Symbol, Time, Proc, Method, Fixnum] do
  type.class_eval do
    puts "#{type.inspect} -> #{V8::Conversion.const_get(type.name).inspect}"
    include V8::Conversion.const_get(type.name)
  end
end

Ruby 2.4.0 prints:

Class -> V8::Conversion::Class
Object -> V8::Conversion::Object
Array -> V8::Conversion::Array
Hash -> V8::Conversion::Hash
String -> V8::Conversion::String
Symbol -> V8::Conversion::Symbol
Time -> V8::Conversion::Time
Proc -> V8::Conversion::Proc
Method -> V8::Conversion::Method
Integer -> Integer

Ruby 2.3.0 prints:

Class -> V8::Conversion::Class
Object -> V8::Conversion::Object
Array -> V8::Conversion::Array
Hash -> V8::Conversion::Hash
String -> V8::Conversion::String
Symbol -> V8::Conversion::Symbol
Time -> V8::Conversion::Time
Proc -> V8::Conversion::Proc
Method -> V8::Conversion::Method
Fixnum -> V8::Conversion::Fixnum

Is this the correct way to handle the new Integer class? (I copy-pasted the Fixnum one.)

# lib/v8/conversion/integer.rb
class V8::Conversion
  module Integer
    def to_ruby
      self
    end

    def to_v8
      self.to_f.to_v8
    end
  end
end
# lib/v8.rb
require "v8/version"

require 'v8/weak'
require 'v8/init'
require 'v8/error'
require 'v8/stack'
require 'v8/conversion/fundamental'
require 'v8/conversion/indentity'
require 'v8/conversion/reference'
require 'v8/conversion/primitive'
require 'v8/conversion/code'
require 'v8/conversion/class'
require 'v8/conversion/object'
require 'v8/conversion/time'
require 'v8/conversion/hash'
require 'v8/conversion/array'
require 'v8/conversion/proc'
require 'v8/conversion/method'
require 'v8/conversion/symbol'
require 'v8/conversion/string'
require 'v8/conversion/fixnum'
require 'v8/conversion/integer' # <-- Added this.
require 'v8/conversion'
require 'v8/access/names'
require 'v8/access/indices'
require 'v8/access/invocation'
require 'v8/access'
require 'v8/context'
require 'v8/object'
require 'v8/array'
require 'v8/function'

I can make a PR if it's deemed worthy. Thanks.

ignisf commented 7 years ago

Hello,

Ruby 2.4 support is already in: https://github.com/cowboyd/therubyracer/compare/ec6adc8986ac746782bf629f41ef0b92c600d8a1...2f1127bed45f03b91f3c92469be26399d3c1bcd8

ollie commented 7 years ago

Right, I didn't check the master. Okay, thanks.

ignisf commented 7 years ago

No problem, hopefully we'll release soon

ollie commented 7 years ago

Um, don't you think it would be a good idea to keep one of these issues open in the mean time so people don't create tons of new issues?

ignisf commented 7 years ago

Good idea, https://github.com/cowboyd/therubyracer/issues/430