rustwasm / wasm-bindgen

Facilitating high-level interactions between Wasm modules and JavaScript
https://rustwasm.github.io/docs/wasm-bindgen/
Apache License 2.0
7.78k stars 1.07k forks source link

Support prototype-less objects #1366

Open RReverser opened 5 years ago

RReverser commented 5 years ago

This would be a breaking change, but, I think, Object.keys, Object.entries, Object.setPrototypeOf etc. need to be changed to somehow support prototype-less objects.

In JavaScript it's common to have object values without prototype used as dictionaries created by either Object.create(null) or a corresponding literal form { __proto__: null, ... }.

These don't have Object in their prototype chain, so you can't call methods from Object.prototype on them (e.g. .to_string() won't work), and dyn_ref fails as expected.

However, you should still be able to call Object.keys, Object.values, Object.seal and other static methods, because they support such objects as well as any others.

So we need to somehow separate these two types to allow arbitrary objects in static methods, and only prototyped objects in methods that accept &self / &this.

RReverser commented 5 years ago

@alexcrichton I've also just noticed that current implementation of Object::try_from is also wrong due to this issue (it assumes that any object inherits from Object prototype, but it's not true).

alexcrichton commented 5 years ago

I'll admit that I don't think I know enough about JS to know what to do about this issue, but a PR would be most welcome to fix the state of affairs!

RReverser commented 5 years ago

I guess the hardest part of this would be to figure out naming in a way that wouldn't be too confusing (in JS it's object as a value type vs Object as a class).

Pauan commented 5 years ago

I believe the correct check for whether something is an object or not is Object(foo) === foo (note the lack of new, which is very important!)

RReverser commented 5 years ago

That performs unnecessary boxing of primitives and I'm not sure what you're gaining over current JsValue::is_object.

My suggestion was only about splitting interfaces, but keeping the checks as they already are.

Pauan commented 5 years ago

@RReverser There's some things which are objects but aren't typeof "object", such as regexps (in some browsers), and functions.

So at the very least, it would need to be changed to a blacklist (anything that isn't a primitive is assumed to be an object).

But that has its own share of issues, such as if ECMAScript adds in a new primitive (which they did, with Symbol).

So Object(foo) === foo is the most reliable way of detecting objects, since it actually uses the ToObject primitive.

If your concern is about the boxing, it can do a typeof check first, and only do the Object(foo) check if it isn't a primitive.

Come to think of it, we may even want to allow for primitives, because doing things like Object.keys("foo") is perfectly valid. So in that case we should actually only be excluding null and undefined.

My suggestion was only about splitting interfaces, but keeping the checks as they already are.

Yes, I understood that, I wasn't replying to your most recent post, but to the concept overall.

The point of this issue is to expand the support for using the Object.foo methods. So making sure that the "is it an object?" check is correct is important for that (otherwise we'll just end up in the same situation again later of "I want to use Object methods but it doesn't work")

RReverser commented 5 years ago

There's some things which are objects but aren't typeof "object", such as regexps (in some browsers), and functions.

I think the last time this happened was in IE5 (6?) and we already don't support these browsers due to other APIs we rely on. I'd rather target spec-compliant implementations.

Come to think of it, we may even want to allow for primitives, because doing things like Object.keys("foo") is perfectly valid.

This is a good point. But given that JsString / Number / Boolean all extend from Object (that is, automatically Deref to it), this should still work as it does now?

Pauan commented 5 years ago

I think the last time this happened was in IE5 (6?) and we already don't support these browsers due to other APIs we rely on. I'd rather target spec-compliant implementations.

That's a fair point, though my point about functions still stands.

This is a good point. But given that JsString / Number / Boolean all extend from Object (that is, automatically Deref to it), this should still work as it does now?

Yes, but it's rather strange: they claim to inherit from Object, and you can use Object methods on them, and you can cast them to Object with into(), but you can't use dyn_into(), and instanceof returns false!

RReverser commented 5 years ago

That's a fair point, though my point about functions still stands.

I guess functions are similar to other primitives mentioned below in this regard (in that they are not objects, but they inherit from Object).

but you can't use dyn_into(), and instanceof returns false!

This is true. I'm not sure what would be the best way to go about this yet.

For the "base prototype-less object" type the is_type_of check could be as simple as checking that the value is not null/undefined - then it's guaranteed to either be an object or a primitive that inherits an Object and so all the methods that don't care about prototypes like Object::keys would work as expected.

However, is_type_of on the actual Object as a class might be a bit more complicated for the reasons you mentioned above...

RReverser commented 5 years ago

but you can't use dyn_into(), and instanceof returns false!

This is true. I'm not sure what would be the best way to go about this yet.

On the other hand, same applies to working with these values in JavaScript itself too, so maybe it's not a big deal?

Pauan commented 5 years ago

I guess functions are similar to other primitives mentioned below in this regard (in that they are not objects, but they inherit from Object).

Actually, unlike the primitives, functions are true objects: you can set properties on them, and they retain them:

function foo() {}
foo.x = 5;

var bar = "bar";
bar.x = 5;

console.log(foo.x, bar.x);

On the other hand, same applies to working with these values in JavaScript itself too, so maybe it's not a big deal?

Not quite, since JS doesn't need dyn_into, since it's dynamically typed. So this is really a Rust-specific problem. (However I agree that type checking is pretty crazy/inconsistent in JS in general)

I'm not sure what would be the best way to go about this yet.

I'm not entirely sure either, but given the existence of the primitives and things like Object.create(null), my thinking is that we should treat Object as being similar to JsString.

In other words, JsString doesn't actually care about or use the String prototype, so Object shouldn't care about the prototype either.

So Object would basically just mean "something which quacks like an object", as opposed to being strictly tied to the Object class.

EqualMa commented 1 week ago

Currently Object::try_from(val) checks JsValue::is_object(val) which checks typeof val == "object" && val !== null, while val.dyn_ref::<Object>() checks <Object as JsCast>::is_type_of(val) which checks val instanceof Object.

This causes different behaviors for the following types:

functions objects that doesn't inherit Object
dyn_ref::<Object>
Object::try_from

Examples of objects that doesn't inherit Object

  • Object.create(null)
  • Object.create(Object.create(null)).

js_sys::Object has some methods that rely on Object.prototype, e.g. obj.constructor().

I think we should align behaviors of Object::try_from and <Object as JsCast>::is_type_of so that they both accepts js functions but rejects js objects that doesn't inherit Object (which is, every js value that val instanceof Object is true).

Futhermore, we could:

As for JsString, there are two kinds of inconsistency:

To make things consistent, we could:

The above solutions assume wasm_bindgen allows extends = <structural type> but I don't know whether it's allowed. The wasm-bindgen book documents extends = Class as in the JS class hierarchy sense. Maybe wasm_bindgen could introduce structural extends with a new attribute implements = Interface.