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

JS::Object inherits BasicObject #541

Merged
merged 7 commits into from
Nov 1, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/gems/js/ext/js/js-core.c
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,7 @@ void Init_js() {
rb_define_module_function(rb_mJS, "global", _rb_js_global_this, 0);

i_to_js = rb_intern("to_js");
rb_cJS_Object = rb_define_class_under(rb_mJS, "Object", rb_cObject);
rb_cJS_Object = rb_define_class_under(rb_mJS, "Object", rb_cBasicObject);
VALUE rb_cJS_singleton = rb_singleton_class(rb_cJS_Object);
rb_define_alloc_func(rb_cJS_Object, jsvalue_s_allocate);
rb_define_method(rb_cJS_Object, "[]", _rb_js_obj_aref, 1);
Expand Down
45 changes: 33 additions & 12 deletions packages/gems/js/lib/js.rb
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,23 @@ def self.__async(future, &block)
end
end

class JS::Object
# Inherit BasicObject to prevent define coventional menthods. #Override the `Object#send` to give priority to `send` method of JavaScript.
#
# This is to make it easier to use JavaScript Objects with `send` method such as `WebSocket` and `XMLHttpRequest`.
# The JavaScript method call short-hand in `JS::Object` is implemented using `method_missing`.
# If JS::Object inherits from Object, the `send` method defined in Ruby will take precedence over the JavaScript `send` method.
# If you want to call the JavaScript `send` method, you must use the `call` method as follows:
#
# ws = JS.global[:WebSocket].new("ws://example.com")
# ws.call(:send, ["Hello, world! from Ruby"])
#
# This inheritation allows you to call the JavaScript `send` method with the following syntax:
#
# ws.send("Hello, world! from Ruby")



class JS::Object < BasicObject
# Create a JavaScript object with the new method
#
# The below examples show typical usage in Ruby
Expand All @@ -141,16 +157,16 @@ class JS::Object
#
def new(*args, &block)
args = args + [block] if block
JS.global[:Reflect].construct(self, args.to_js)
::JS.global[:Reflect].construct(self, args.to_js)
end

# Converts +self+ to an Array:
#
# JS.eval("return [1, 2, 3]").to_a.map(&:to_i) # => [1, 2, 3]
# JS.global[:document].querySelectorAll("p").to_a # => [[object HTMLParagraphElement], ...
def to_a
as_array = JS.global[:Array].from(self)
Array.new(as_array[:length].to_i) { as_array[_1] }
as_array = ::JS.global[:Array].from(self)
::Array.new(as_array[:length].to_i) { as_array[_1] }
end

# Provide a shorthand form for JS::Object#call
Expand All @@ -176,7 +192,7 @@ def method_missing(sym, *args, &block)
result = invoke_js_method(sym_str[0..-2].to_sym, *args, &block)
# Type coerce the result to boolean type
# to match the true/false determination in JavaScript's if statement.
return JS.global.Boolean(result) == JS::True
return ::JS.global.Boolean(result) == ::JS::True
end

invoke_js_method(sym, *args, &block)
Expand All @@ -186,7 +202,6 @@ def method_missing(sym, *args, &block)
#
# See JS::Object#method_missing for details.
def respond_to_missing?(sym, include_private)
return true if super
sym_str = sym.to_s
sym = sym_str[0..-2].to_sym if sym_str.end_with?("?")
self[sym].typeof == "function"
Expand All @@ -203,7 +218,7 @@ def respond_to_missing?(sym, include_private)
# end.await # => 42
def apply(*args, &block)
args = args + [block] if block
JS.global[:Reflect].call(:apply, self, JS::Undefined, args.to_js)
::JS.global[:Reflect].call(:apply, self, ::JS::Undefined, args.to_js)
end

# Await a JavaScript Promise like `await` in JavaScript.
Expand Down Expand Up @@ -233,8 +248,14 @@ def apply(*args, &block)
# JS.eval("return new Promise((ok, err) => err(new Error())").await # => raises JS::Error
def await
# Promise.resolve wrap a value or flattens promise-like object and its thenable chain
promise = JS.global[:Promise].resolve(self)
JS.promise_scheduler.await(promise)
promise = ::JS.global[:Promise].resolve(self)
::JS.promise_scheduler.await(promise)
end

# I don't know why, but I can't define the respond_to? method in refinements.
# I'm defining it here instead.
[:nil?, :is_a?, :raise, :respond_to?].each do |method|
Copy link
Member

Choose a reason for hiding this comment

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

Given that JS objects unlikely define methods suffixed with ?, it makes sense to me to define nil?, is_a?, and respond_to? for convenience and better compatibility with libraries without BasicObject support.
One question, where do we use raise?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see, it seems that there is no need to consider collisions of methods suffixed with ?.

We raise an exception when the method call fails.

raise NoMethodError,
"undefined method `#{sym}' for an instance of JS::Object"

define_method(method, ::Object.instance_method(method))
end

private
Expand All @@ -246,12 +267,12 @@ def invoke_js_method(sym, *args, &block)
return self.call(sym, *args, &block) if self[sym].typeof == "function"

# Check to see if a non-functional property exists.
if JS.global[:Reflect].call(:has, self, sym.to_s) == JS::True
raise TypeError,
if ::JS.global[:Reflect].call(:has, self, sym.to_s) == ::JS::True
raise ::TypeError,
"`#{sym}` is not a function. To reference a property, use `[:#{sym}]` syntax instead."
end

raise NoMethodError,
raise ::NoMethodError,
"undefined method `#{sym}' for an instance of JS::Object"
end
end
Expand Down
2 changes: 2 additions & 0 deletions packages/npm-packages/ruby-wasm-wasi/test/unit/test_error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
require "js"

class JS::TestError < Test::Unit::TestCase
using JsObjectTestable

def test_throw_error
e = assert_raise(JS::Error) { JS.eval("throw new Error('foo')") }
assert_match /^Error: foo/, e.message
Expand Down
2 changes: 2 additions & 0 deletions packages/npm-packages/ruby-wasm-wasi/test/unit/test_float.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
require "js"

class JS::TestFloat < Test::Unit::TestCase
using JsObjectTestable

def test_to_js
assert_equal (1.0).to_js, JS.eval("return 1.0;")
assert_equal (0.5).to_js, JS.eval("return 0.5;")
Expand Down
26 changes: 23 additions & 3 deletions packages/npm-packages/ruby-wasm-wasi/test/unit/test_object.rb
Original file line number Diff line number Diff line change
Expand Up @@ -368,9 +368,29 @@ def test_respond_to_missing?
object = JS.eval(<<~JS)
return { foo() { return true; } };
JS
assert_true object.respond_to?(:foo)
assert_true object.respond_to?(:new)
assert_false object.respond_to?(:bar)
assert_true object.__send__(:respond_to_missing?, :foo, false)
assert_false object.__send__(:respond_to_missing?, :bar, false)

# new is method of JS::Object
assert_false object.__send__(:respond_to_missing?, :new, false)

# send is not implemented in JS::Object,
# because JS::Object is a subclass of JS::BaseObject
assert_false object.__send__(:respond_to_missing?, :send, false)
end

def test_send_method_for_javascript_object_with_send_method
object = JS.eval(<<~JS)
return { send(message) { return message; } };
JS
assert_equal "hello", object.send('hello').to_s
end

def test_send_method_for_javascript_object_without_send_method
object = JS.eval(<<~JS)
return { write(message) { return message; } };
JS
assert_raise(NoMethodError) { object.send('hello') }
end

def test_member_get
Expand Down
12 changes: 12 additions & 0 deletions packages/npm-packages/ruby-wasm-wasi/tools/run-test-unit.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,18 @@ const test = async (instantiate) => {

await vm.evalAsync(`
require 'test/unit'

# Define the methods to be used for unit testing assertions.
# Use refinements to limit the scope of influence.
require 'pp'
module JsObjectTestable
refine JS::Object do
[:object_id, :pretty_inspect].each do |method|
Copy link
Member

Choose a reason for hiding this comment

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

If those definitions are just for testing, it might be better to fix testing libraries in the long term. I just opened a test-unit fix for BasicObject support. test-unit/test-unit#262

But it's fine for now. Could you add FIXME comments mentioning the fix PR and potential future fix?

define_method(method, ::Object.instance_method(method))
end
end
end

require_relative '${rootTestFile}'
ok = Test::Unit::AutoRunner.run
exit(1) unless ok
Expand Down
Loading