From b44fddff5611bb9338f3f2962f14a5dc371eb2a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Wed, 16 Mar 2022 17:07:46 +0100 Subject: [PATCH 01/22] Adding a cast to Value operator on ReturnType --- src/js_types.hpp | 2 ++ src/node/node_return_value.hpp | 7 ++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/js_types.hpp b/src/js_types.hpp index 7ffd98f0a4..ac28a29317 100644 --- a/src/js_types.hpp +++ b/src/js_types.hpp @@ -533,6 +533,8 @@ struct ReturnValue { void set(uint32_t); void set_null(); void set_undefined(); + + operator ValueType() const; }; template diff --git a/src/node/node_return_value.hpp b/src/node/node_return_value.hpp index 2a143573c2..3b0ce26302 100644 --- a/src/node/node_return_value.hpp +++ b/src/node/node_return_value.hpp @@ -41,7 +41,7 @@ class ReturnValue { { } - Napi::Value ToValue() + Napi::Value ToValue() const { // guard check. env.Empty() values cause node to fail in obscure places, so return undefined instead if (m_value.IsEmpty()) { @@ -117,6 +117,11 @@ class ReturnValue { set_undefined(); } } + + operator Napi::Value() const + { + return ToValue(); + } }; } // namespace js From 93b322ee8cd0784325a863efd9e6218236e96f72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Wed, 16 Mar 2022 17:09:34 +0100 Subject: [PATCH 02/22] Adding integration tests of class constructors --- .../tests/src/tests/class-models.ts | 30 ++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/integration-tests/tests/src/tests/class-models.ts b/integration-tests/tests/src/tests/class-models.ts index 8bf6797fc6..4b604d5577 100644 --- a/integration-tests/tests/src/tests/class-models.ts +++ b/integration-tests/tests/src/tests/class-models.ts @@ -17,9 +17,10 @@ //////////////////////////////////////////////////////////////////////////// import { expect } from "chai"; - import Realm from "realm"; +import { openRealmBefore } from "../hooks"; + describe("Class models", () => { describe("as schema element", () => { beforeEach(() => { @@ -72,4 +73,31 @@ describe("Class models", () => { new Realm({ schema: [Person] }); }); }); + + describe("#constructor", () => { + class Person extends Realm.Object { + name!: string; + static schema: Realm.ObjectSchema = { + name: "Person", + properties: { name: "string?" }, + }; + } + + openRealmBefore({ schema: [Person] }); + + it("creates objects", function (this: RealmContext) { + this.realm.write(() => { + // Expect no persons in the database + const persons = this.realm.objects("Person"); + expect(persons.length).equals(0); + + const person = new Person(this.realm); + expect(person).instanceOf(Person); + expect(person).instanceOf(Realm.Object); + // Expect the first element to be the object we just added + expect(persons.length).equals(1); + expect(persons[0]._objectId()).equals(person._objectId()); + }); + }); + }); }); From 3d2f6189a1b8207c128e87c25c72fdcd1a861cc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Wed, 16 Mar 2022 17:11:01 +0100 Subject: [PATCH 03/22] Made the test reporter print stack --- tests/spec/helpers/reporters.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/spec/helpers/reporters.js b/tests/spec/helpers/reporters.js index f7090c8e0a..f9906c27b4 100644 --- a/tests/spec/helpers/reporters.js +++ b/tests/spec/helpers/reporters.js @@ -31,5 +31,8 @@ jasmine.getEnv().addReporter( spec: { displayPending: true, }, + summary: { + displayStacktrace: true, + }, }), ); From bfac8f3afd8588507aace89d2da72715a158cc45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Wed, 16 Mar 2022 17:12:04 +0100 Subject: [PATCH 04/22] Implemented Realm.Object constructor --- src/js_app.hpp | 4 ++-- src/js_class.hpp | 2 +- src/js_realm.hpp | 4 ++-- src/js_realm_object.hpp | 22 +++++++++++++++++++++- 4 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/js_app.hpp b/src/js_app.hpp index 65a7b7d97e..37e2caab25 100644 --- a/src/js_app.hpp +++ b/src/js_app.hpp @@ -71,7 +71,7 @@ class AppClass : public ClassDefinition { static inline std::string platform_os = "unknown-os"; static inline std::string platform_version = "?.?.?"; - static void constructor(ContextType, ObjectType, Arguments&); + static void constructor(ContextType, ObjectType, Arguments&, ObjectType); static FunctionType create_constructor(ContextType); static ObjectType create_instance(ContextType, SharedApp); @@ -124,7 +124,7 @@ inline typename T::Object AppClass::create_instance(ContextType ctx, SharedAp } template -void AppClass::constructor(ContextType ctx, ObjectType this_object, Arguments& args) +void AppClass::constructor(ContextType ctx, ObjectType this_object, Arguments& args, ObjectType constructor) { static const String config_id = "id"; static const String config_base_url = "baseUrl"; diff --git a/src/js_class.hpp b/src/js_class.hpp index f42ecbfa9b..7d955469b5 100644 --- a/src/js_class.hpp +++ b/src/js_class.hpp @@ -67,7 +67,7 @@ struct Arguments { }; template -using ConstructorType = void(typename T::Context, typename T::Object, Arguments&); +using ConstructorType = void(typename T::Context, typename T::Object, Arguments&, typename T::Object); template using ArgumentsMethodType = void(typename T::Context, typename T::Object, Arguments&, ReturnValue&); diff --git a/src/js_realm.hpp b/src/js_realm.hpp index d5448bb970..d3576db4e6 100644 --- a/src/js_realm.hpp +++ b/src/js_realm.hpp @@ -363,7 +363,7 @@ class RealmClass : public ClassDefinition> { #endif // static methods - static void constructor(ContextType, ObjectType, Arguments&); + static void constructor(ContextType, ObjectType, Arguments&, ObjectType); static SharedRealm create_shared_realm(ContextType, realm::Realm::Config, bool, ObjectDefaultsMap&&, ConstructorMap&&); static bool get_realm_config(ContextType ctx, size_t argc, const ValueType arguments[], realm::Realm::Config&, @@ -775,7 +775,7 @@ bool RealmClass::get_realm_config(ContextType ctx, size_t argc, const ValueTy } template -void RealmClass::constructor(ContextType ctx, ObjectType this_object, Arguments& args) +void RealmClass::constructor(ContextType ctx, ObjectType this_object, Arguments& args, ObjectType constructor) { set_internal>(ctx, this_object, nullptr); realm::Realm::Config config; diff --git a/src/js_realm_object.hpp b/src/js_realm_object.hpp index 199e21bf2e..c4af5193d5 100644 --- a/src/js_realm_object.hpp +++ b/src/js_realm_object.hpp @@ -60,6 +60,7 @@ struct RealmObjectClass : ClassDefinition> { using FunctionType = typename T::Function; using ObjectType = typename T::Object; using ValueType = typename T::Value; + using Context = js::Context; using String = js::String; using Value = js::Value; using Object = js::Object; @@ -69,6 +70,7 @@ struct RealmObjectClass : ClassDefinition> { static ObjectType create_instance(ContextType, realm::js::RealmObject); + static void constructor(ContextType, ObjectType, Arguments&, ObjectType); static void get_property(ContextType, ObjectType, const String&, ReturnValue&); static bool set_property(ContextType, ObjectType, const String&, ValueType); static std::vector get_property_names(ContextType, ObjectType); @@ -162,6 +164,24 @@ typename T::Object RealmObjectClass::create_instance(ContextType ctx, realm:: } } +template +void RealmObjectClass::constructor(ContextType ctx, ObjectType this_object, Arguments& args, + ObjectType constructor) +{ + args.validate_between(1, 2); + auto realm = Value::validated_to_object(ctx, args[0], "realm"); + auto values = Value::is_undefined(ctx, args[1]) ? Object::create_empty(ctx) : args[1]; + // Create an object + std::vector create_args{constructor, values}; + Arguments create_arguments{ctx, create_args.size(), create_args.data()}; + ReturnValue result{ctx}; + RealmClass::create(ctx, realm, create_arguments, result); + // Move the internal from the constructed object onto this_object + auto realm_object = Value::validated_to_object(ctx, result); + auto internal = get_internal>(ctx, realm_object); + set_internal>(ctx, this_object, internal); +} + template void RealmObjectClass::get_property(ContextType ctx, ObjectType object, const String& property_name, ReturnValue& return_value) @@ -372,7 +392,7 @@ void RealmObjectClass::add_listener(ContextType ctx, ObjectType this_object, auto callback = Value::validated_to_function(ctx, args[0]); Protected protected_callback(ctx, callback); Protected protected_this(ctx, this_object); - Protected protected_ctx(Context::get_global_context(ctx)); + Protected protected_ctx(Context::get_global_context(ctx)); auto token = realm_object->add_notification_callback([=](CollectionChangeSet const& change_set, std::exception_ptr exception) { From e7b2cc5f554818026082a32c6143dad9e45aaa7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Wed, 16 Mar 2022 17:12:46 +0100 Subject: [PATCH 05/22] Added node implementation and fixed legacy tests --- src/node/node_class.hpp | 53 +++++++++++------------------------------ tests/js/list-tests.js | 28 +++++----------------- tests/js/realm-tests.js | 24 +++++++++---------- 3 files changed, 31 insertions(+), 74 deletions(-) diff --git a/src/node/node_class.hpp b/src/node/node_class.hpp index 3f379c32bb..ebb761da9f 100644 --- a/src/node/node_class.hpp +++ b/src/node/node_class.hpp @@ -52,6 +52,7 @@ namespace node { Napi::FunctionReference ObjectGetOwnPropertyDescriptor; node::Protected ExternalSymbol; +Napi::FunctionReference ObjectCreate; Napi::FunctionReference ObjectSetPrototypeOf; Napi::FunctionReference GlobalProxy; Napi::FunctionReference FunctionBind; @@ -63,6 +64,11 @@ static void node_class_init(Napi::Env env) ObjectSetPrototypeOf = Napi::Persistent(setPrototypeOf); ObjectSetPrototypeOf.SuppressDestruct(); + auto create = env.Global().Get("Object").As().Get("create").As(); + ObjectCreate = Napi::Persistent(create); + ObjectCreate.SuppressDestruct(); + + auto getOwnPropertyDescriptor = env.Global().Get("Object").As().Get("getOwnPropertyDescriptor").As(); ObjectGetOwnPropertyDescriptor = Napi::Persistent(getOwnPropertyDescriptor); @@ -1274,7 +1280,8 @@ Napi::Object ObjectWrap::create_instance_by_schema(Napi::Env env, Nap } Napi::External externalValue = Napi::External::New(env, internal, internal_finalizer); - instance = schemaObjectConstructor.New({}); + Napi::Object constructorPrototype = schemaObjectConstructor.Get("prototype").As(); + instance = ObjectCreate.Call({constructorPrototype}).As(); instance.Set(externalSymbol, externalValue); } else { @@ -1297,8 +1304,8 @@ Napi::Object ObjectWrap::create_instance_by_schema(Napi::Env env, Nap if (schemaExists) { schemaObjectType = schemaObjects->at(schemaName); schemaObjectConstructor = schemaObjectType->constructor.Value(); - - instance = schemaObjectConstructor.New({}); + Napi::Object constructorPrototype = constructor.Get("prototype").As(); + instance = ObjectCreate.Call({constructorPrototype}).As(); Napi::External externalValue = Napi::External::New(env, internal, internal_finalizer); instance.Set(externalSymbol, externalValue); @@ -1313,46 +1320,13 @@ Napi::Object ObjectWrap::create_instance_by_schema(Napi::Env env, Nap std::vector properties = create_napi_property_descriptors(env, constructorPrototype, schema, false /*redefine*/); - Napi::Function realmObjectClassConstructor = ObjectWrap::create_constructor(env); - bool isInstanceOfRealmObjectClass = constructorPrototype.InstanceOf(realmObjectClassConstructor); - - // Skip if the user defined constructor inherited the RealmObjectClass. All RealmObjectClass members are - // available already. - if (!isInstanceOfRealmObjectClass) { - // setup all RealmObjectClass methods to the prototype of the object - for (auto& pair : s_class.methods) { - // don't redefine if exists - if (!constructorPrototype.HasOwnProperty(pair.first)) { - auto descriptor = Napi::PropertyDescriptor::Function( - env, constructorPrototype, Napi::String::New(env, pair.first) /*name*/, &method_callback, - napi_default | realm::js::PropertyAttributes::DontEnum, (void*)pair.second /*callback*/); - properties.push_back(descriptor); - } - } - - for (auto& pair : s_class.properties) { - // don't redefine if exists - if (!constructorPrototype.HasOwnProperty(pair.first)) { - napi_property_attributes napi_attributes = - napi_default | - (realm::js::PropertyAttributes::DontEnum | realm::js::PropertyAttributes::DontDelete); - auto descriptor = Napi::PropertyDescriptor::Accessor( - Napi::String::New(env, pair.first) /*name*/, napi_attributes, - (void*)&pair.second /*callback*/); - properties.push_back(descriptor); - } - } - } - // define the properties on the prototype of the schema object constructor if (properties.size() > 0) { constructorPrototype.DefineProperties(properties); } - instance = schemaObjectConstructor.New({}); - if (!instance.InstanceOf(schemaObjectConstructor)) { - throw Napi::Error::New(env, "Realm object constructor must not return another value"); - } + instance = ObjectCreate.Call({constructorPrototype}).As(); + Napi::External externalValue = Napi::External::New(env, internal, internal_finalizer); instance.Set(externalSymbol, externalValue); @@ -1435,10 +1409,11 @@ Napi::Value ObjectWrap::constructor_callback(const Napi::CallbackInfo if (reinterpret_cast(s_class.constructor) != nullptr) { auto arguments = get_arguments(info); node::Arguments args{env, arguments.size(), arguments.data()}; - s_class.constructor(env, info.This().As(), args); + s_class.constructor(env, info.This().As(), args, info.NewTarget().As()); return scope.Escape(env.Null()); // return a value to comply with Napi::FunctionCallback } else { + // XXX: Remove this now that we declare a constructor for the RealmObjectClass bool isRealmObjectClass = std::is_same>::value; if (isRealmObjectClass) { return scope.Escape(env.Null()); // return a value to comply with Napi::FunctionCallback diff --git a/tests/js/list-tests.js b/tests/js/list-tests.js index 3f3103918e..9eeeea8a17 100644 --- a/tests/js/list-tests.js +++ b/tests/js/list-tests.js @@ -1840,58 +1840,42 @@ module.exports = { testClassObjectCreation: function () { class TodoItem extends Realm.Object { - constructor(description) { - super(); - this.id = new ObjectId(); - this.description = description; - this.done = false; + constructor(realm, description) { + super(realm, { done: false, description }); } } TodoItem.schema = { name: "TodoItem", properties: { - id: "objectId", description: "string", done: { type: "bool", default: false }, deadline: "date?", }, - primaryKey: "id", }; class TodoList extends Realm.Object { - constructor(name) { - super(); - this.id = new ObjectId(); - this.name = name; - this.items = []; + constructor(realm, name) { + super(realm, { name }); } } TodoList.schema = { name: "TodoList", properties: { - id: "objectId", name: "string", items: "TodoItem[]", }, - primaryKey: "id", }; const realm = new Realm({ schema: [TodoList, TodoItem] }); realm.write(() => { const list = realm.create(TodoList, { - id: new ObjectId(), name: "MyTodoList", }); - TestCase.assertThrowsContaining(() => { - list.items.push(new TodoItem("Fix that bug")); - }, "Cannot reference a detached instance of Realm.Object"); - - TestCase.assertThrowsContaining(() => { - realm.create(TodoItem, new TodoItem("Fix that bug")); - }, "Cannot create an object from a detached Realm.Object instance"); + list.items.push(new TodoItem(realm, "Fix that bug")); + realm.create(TodoItem, new TodoItem(realm, "Fix that bug")); }); }, }; diff --git a/tests/js/realm-tests.js b/tests/js/realm-tests.js index 3445cd4743..0e5f3688c1 100644 --- a/tests/js/realm-tests.js +++ b/tests/js/realm-tests.js @@ -88,8 +88,8 @@ module.exports = { let constructorCalled = false; //test class syntax support class Car extends Realm.Object { - constructor() { - super(); + constructor(realm) { + super(realm); constructorCalled = true; } } @@ -128,7 +128,7 @@ module.exports = { let realm = new Realm({ schema: [Car, Car2] }); realm.write(() => { let car = realm.create("Car", { make: "Audi", model: "A4", kilometers: 24 }); - TestCase.assertTrue(constructorCalled); + TestCase.assertFalse(constructorCalled); TestCase.assertEqual(car.make, "Audi"); TestCase.assertEqual(car.model, "A4"); TestCase.assertEqual(car.kilometers, 24); @@ -144,21 +144,21 @@ module.exports = { constructorCalled = false; let car1 = realm.create("Car", { make: "VW", model: "Touareg", kilometers: 13 }); - TestCase.assertTrue(constructorCalled); + TestCase.assertFalse(constructorCalled); TestCase.assertEqual(car1.make, "VW"); TestCase.assertEqual(car1.model, "Touareg"); TestCase.assertEqual(car1.kilometers, 13); TestCase.assertInstanceOf(car1, Realm.Object, "car1 not an instance of Realm.Object"); let car2 = realm.create("Car2", { make: "Audi", model: "A4", kilometers: 24 }); - TestCase.assertTrue(calledAsConstructor); + TestCase.assertFalse(calledAsConstructor); TestCase.assertEqual(car2.make, "Audi"); TestCase.assertEqual(car2.model, "A4"); TestCase.assertEqual(car2.kilometers, 24); TestCase.assertInstanceOf(car2, Realm.Object, "car2 not an instance of Realm.Object"); let car2_1 = realm.create("Car2", { make: "VW", model: "Touareg", kilometers: 13 }); - TestCase.assertTrue(calledAsConstructor); + TestCase.assertFalse(calledAsConstructor); TestCase.assertEqual(car2_1.make, "VW"); TestCase.assertEqual(car2_1.model, "Touareg"); TestCase.assertEqual(car2_1.kilometers, 13); @@ -1140,20 +1140,18 @@ module.exports = { let object = realm.create("CustomObject", { intCol: 1 }); TestCase.assertTrue(object instanceof CustomObject); TestCase.assertTrue(Object.getPrototypeOf(object) == CustomObject.prototype); - TestCase.assertEqual(customCreated, 1); + TestCase.assertEqual(customCreated, 0); // Should be able to create object by passing in constructor. object = realm.create(CustomObject, { intCol: 2 }); TestCase.assertTrue(object instanceof CustomObject); TestCase.assertTrue(Object.getPrototypeOf(object) == CustomObject.prototype); - TestCase.assertEqual(customCreated, 2); + TestCase.assertEqual(customCreated, 0); }); - TestCase.assertThrowsContaining(() => { - realm.write(() => { - realm.create("InvalidObject", { intCol: 1 }); - }); - }, "Realm object constructor must not return another value"); + realm.write(() => { + realm.create("InvalidObject", { intCol: 1 }); + }); // Only the original constructor should be valid. function InvalidCustomObject() {} From e1b0a6400b3bcc0ab7e0f837a70aa33373538e6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Thu, 17 Mar 2022 15:51:49 +0100 Subject: [PATCH 06/22] Read from this.constructor instead of new.target --- src/js_app.hpp | 4 ++-- src/js_class.hpp | 2 +- src/js_realm.hpp | 4 ++-- src/js_realm_object.hpp | 19 ++++++++++++------- src/node/node_class.hpp | 8 +++----- 5 files changed, 20 insertions(+), 17 deletions(-) diff --git a/src/js_app.hpp b/src/js_app.hpp index 37e2caab25..65a7b7d97e 100644 --- a/src/js_app.hpp +++ b/src/js_app.hpp @@ -71,7 +71,7 @@ class AppClass : public ClassDefinition { static inline std::string platform_os = "unknown-os"; static inline std::string platform_version = "?.?.?"; - static void constructor(ContextType, ObjectType, Arguments&, ObjectType); + static void constructor(ContextType, ObjectType, Arguments&); static FunctionType create_constructor(ContextType); static ObjectType create_instance(ContextType, SharedApp); @@ -124,7 +124,7 @@ inline typename T::Object AppClass::create_instance(ContextType ctx, SharedAp } template -void AppClass::constructor(ContextType ctx, ObjectType this_object, Arguments& args, ObjectType constructor) +void AppClass::constructor(ContextType ctx, ObjectType this_object, Arguments& args) { static const String config_id = "id"; static const String config_base_url = "baseUrl"; diff --git a/src/js_class.hpp b/src/js_class.hpp index 7d955469b5..f42ecbfa9b 100644 --- a/src/js_class.hpp +++ b/src/js_class.hpp @@ -67,7 +67,7 @@ struct Arguments { }; template -using ConstructorType = void(typename T::Context, typename T::Object, Arguments&, typename T::Object); +using ConstructorType = void(typename T::Context, typename T::Object, Arguments&); template using ArgumentsMethodType = void(typename T::Context, typename T::Object, Arguments&, ReturnValue&); diff --git a/src/js_realm.hpp b/src/js_realm.hpp index d3576db4e6..d5448bb970 100644 --- a/src/js_realm.hpp +++ b/src/js_realm.hpp @@ -363,7 +363,7 @@ class RealmClass : public ClassDefinition> { #endif // static methods - static void constructor(ContextType, ObjectType, Arguments&, ObjectType); + static void constructor(ContextType, ObjectType, Arguments&); static SharedRealm create_shared_realm(ContextType, realm::Realm::Config, bool, ObjectDefaultsMap&&, ConstructorMap&&); static bool get_realm_config(ContextType ctx, size_t argc, const ValueType arguments[], realm::Realm::Config&, @@ -775,7 +775,7 @@ bool RealmClass::get_realm_config(ContextType ctx, size_t argc, const ValueTy } template -void RealmClass::constructor(ContextType ctx, ObjectType this_object, Arguments& args, ObjectType constructor) +void RealmClass::constructor(ContextType ctx, ObjectType this_object, Arguments& args) { set_internal>(ctx, this_object, nullptr); realm::Realm::Config config; diff --git a/src/js_realm_object.hpp b/src/js_realm_object.hpp index c4af5193d5..9fe331adcf 100644 --- a/src/js_realm_object.hpp +++ b/src/js_realm_object.hpp @@ -70,7 +70,7 @@ struct RealmObjectClass : ClassDefinition> { static ObjectType create_instance(ContextType, realm::js::RealmObject); - static void constructor(ContextType, ObjectType, Arguments&, ObjectType); + static void constructor(ContextType, ObjectType, Arguments&); static void get_property(ContextType, ObjectType, const String&, ReturnValue&); static bool set_property(ContextType, ObjectType, const String&, ValueType); static std::vector get_property_names(ContextType, ObjectType); @@ -165,21 +165,26 @@ typename T::Object RealmObjectClass::create_instance(ContextType ctx, realm:: } template -void RealmObjectClass::constructor(ContextType ctx, ObjectType this_object, Arguments& args, - ObjectType constructor) +void RealmObjectClass::constructor(ContextType ctx, ObjectType this_object, Arguments& args) { + // Parse aguments args.validate_between(1, 2); + auto constructor = Object::validated_get_object(ctx, this_object, "constructor"); auto realm = Value::validated_to_object(ctx, args[0], "realm"); auto values = Value::is_undefined(ctx, args[1]) ? Object::create_empty(ctx) : args[1]; + // Create an object std::vector create_args{constructor, values}; Arguments create_arguments{ctx, create_args.size(), create_args.data()}; ReturnValue result{ctx}; RealmClass::create(ctx, realm, create_arguments, result); - // Move the internal from the constructed object onto this_object - auto realm_object = Value::validated_to_object(ctx, result); - auto internal = get_internal>(ctx, realm_object); - set_internal>(ctx, this_object, internal); + ObjectType realm_object_object = Value::validated_to_object(ctx, result); + + // Copy the internal from the constructed object onto this_object + auto realm_object = get_internal>(ctx, realm_object_object); + // The finalizer on the ObjectWrap (applied inside of set_internal) will delete the `new_realm_object` + auto new_realm_object = new realm::js::RealmObject(*realm_object); + set_internal>(ctx, this_object, new_realm_object); } template diff --git a/src/node/node_class.hpp b/src/node/node_class.hpp index ebb761da9f..d01cf6447b 100644 --- a/src/node/node_class.hpp +++ b/src/node/node_class.hpp @@ -1285,6 +1285,7 @@ Napi::Object ObjectWrap::create_instance_by_schema(Napi::Env env, Nap instance.Set(externalSymbol, externalValue); } else { + Napi::Object constructorPrototype = constructor.Get("prototype").As(); // creating a RealmObject with user defined constructor bool schemaExists = schemaObjects->count(schemaName); @@ -1304,7 +1305,6 @@ Napi::Object ObjectWrap::create_instance_by_schema(Napi::Env env, Nap if (schemaExists) { schemaObjectType = schemaObjects->at(schemaName); schemaObjectConstructor = schemaObjectType->constructor.Value(); - Napi::Object constructorPrototype = constructor.Get("prototype").As(); instance = ObjectCreate.Call({constructorPrototype}).As(); Napi::External externalValue = Napi::External::New(env, internal, internal_finalizer); @@ -1314,7 +1314,6 @@ Napi::Object ObjectWrap::create_instance_by_schema(Napi::Env env, Nap } schemaObjectConstructor = constructor; - Napi::Object constructorPrototype = constructor.Get("prototype").As(); // get all properties from the schema std::vector properties = @@ -1385,8 +1384,7 @@ typename ClassType::Internal* ObjectWrap::get_internal(Napi::Env env, } template -void ObjectWrap::set_internal(Napi::Env env, const Napi::Object& object, - typename ClassType::Internal* internal) +void ObjectWrap::set_internal(Napi::Env env, Napi::Object& object, typename ClassType::Internal* internal) { bool isRealmObjectClass = std::is_same>::value; if (isRealmObjectClass) { @@ -1409,7 +1407,7 @@ Napi::Value ObjectWrap::constructor_callback(const Napi::CallbackInfo if (reinterpret_cast(s_class.constructor) != nullptr) { auto arguments = get_arguments(info); node::Arguments args{env, arguments.size(), arguments.data()}; - s_class.constructor(env, info.This().As(), args, info.NewTarget().As()); + s_class.constructor(env, info.This().As(), args); return scope.Escape(env.Null()); // return a value to comply with Napi::FunctionCallback } else { From dd3671b105501a2aed9dd54803f712bc1183475c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Thu, 17 Mar 2022 15:53:19 +0100 Subject: [PATCH 07/22] Fixing JSC implementation --- src/js_types.hpp | 4 +-- src/jsc/jsc_class.hpp | 71 ++++++++++++++++++++++------------------ src/jsc/jsc_object.hpp | 5 ++- src/node/node_class.hpp | 2 +- src/node/node_object.hpp | 3 +- 5 files changed, 46 insertions(+), 39 deletions(-) diff --git a/src/js_types.hpp b/src/js_types.hpp index ac28a29317..002819ce74 100644 --- a/src/js_types.hpp +++ b/src/js_types.hpp @@ -461,7 +461,7 @@ struct Object { static typename ClassType::Internal* get_internal(ContextType ctx, const ObjectType&); template - static void set_internal(ContextType ctx, const ObjectType&, typename ClassType::Internal*); + static void set_internal(ContextType ctx, ObjectType&, typename ClassType::Internal*); static ObjectType create_from_app_error(ContextType, const app::AppError&); static ValueType create_from_optional_app_error(ContextType, const util::Optional&); @@ -567,7 +567,7 @@ REALM_JS_INLINE typename ClassType::Internal* get_internal(typename T::Context c } template -REALM_JS_INLINE void set_internal(typename T::Context ctx, const typename T::Object& object, +REALM_JS_INLINE void set_internal(typename T::Context ctx, typename T::Object& object, typename ClassType::Internal* ptr) { Object::template set_internal(ctx, object, ptr); diff --git a/src/jsc/jsc_class.hpp b/src/jsc/jsc_class.hpp index 984bf55454..c0a46dc117 100644 --- a/src/jsc/jsc_class.hpp +++ b/src/jsc/jsc_class.hpp @@ -155,6 +155,7 @@ class ObjectWrap { } static Internal* get_internal(JSContextRef ctx, const JSObjectRef& object); + static void set_internal(JSContextRef ctx, JSObjectRef& instance, typename ClassType::Internal* internal); static void on_context_destroy(JSContextRef ctx, std::string realmPath); @@ -200,9 +201,6 @@ class ObjectWrap { return false; } - static void set_internal_property(JSContextRef ctx, JSObjectRef& instance, - typename ClassType::Internal* internal); - static void define_schema_properties(JSContextRef ctx, JSObjectRef constructorPrototype, const realm::ObjectSchema& schema, bool redefine); static void define_accessor_for_schema_property(JSContextRef ctx, JSObjectRef& target, jsc::String* name); @@ -624,16 +622,25 @@ typename ClassType::Internal* ObjectWrap::get_internal(JSContextRef c } template -void ObjectWrap::set_internal_property(JSContextRef ctx, JSObjectRef& instance, - typename ClassType::Internal* internal) +void ObjectWrap::set_internal(JSContextRef ctx, JSObjectRef& instance, + typename ClassType::Internal* internal) { - // create a JS object that has a finializer to delete the internal reference - JSObjectRef internalObject = JSObjectMake(ctx, m_internalValueClass, new ObjectWrap(internal)); - const jsc::String* externalName = get_cached_property_name("_external"); - auto attributes = realm::js::PropertyAttributes::ReadOnly | realm::js::PropertyAttributes::DontDelete | - realm::js::PropertyAttributes::DontEnum; - Object::set_property(ctx, instance, *externalName, internalObject, attributes); + bool isRealmObjectClass = std::is_same>::value; + if (isRealmObjectClass) { + // create a JS object that has a finializer to delete the internal reference + JSObjectRef internalObject = JSObjectMake(ctx, m_internalValueClass, new ObjectWrap(internal)); + const jsc::String* externalName = get_cached_property_name("_external"); + auto attributes = realm::js::PropertyAttributes::ReadOnly | realm::js::PropertyAttributes::DontDelete | + realm::js::PropertyAttributes::DontEnum; + if (internal) { + Object::set_property(ctx, instance, *externalName, internalObject, attributes); + } + } + else { + auto wrap = static_cast*>(JSObjectGetPrivate(instance)); + *wrap = internal; + } } static inline JSObjectRef try_get_prototype(JSContextRef ctx, JSObjectRef object) @@ -683,13 +690,6 @@ bool ObjectWrap::has_instance(JSContextRef ctx, JSValueRef value) proto = try_get_prototype(ctx, proto); } - // handle RealmObjects using user defined ctors without extending RealmObject. - // In this case we just check for existing internal value to identify RealmObject instances - auto internal = ObjectWrap::get_internal(ctx, object); - if (internal != nullptr) { - return true; - } - // if there is no RealmObjectClass on the prototype chain and the object does not have existing internal value // then this is not an RealmObject instance return false; @@ -777,8 +777,7 @@ inline JSObjectRef ObjectWrap::create_instance_by_schema(JSContextRef definition.className = schema.name.c_str(); JSClassRef schemaClass = JSClassCreate(&definition); schemaObjectConstructor = JSObjectMakeConstructor(ctx, schemaClass, nullptr); - value = Object::get_property(ctx, schemaObjectConstructor, "prototype"); - constructorPrototype = Value::to_object(ctx, value); + constructorPrototype = Object::validated_get_object(ctx, schemaObjectConstructor, "prototype"); JSObjectSetPrototype(ctx, constructorPrototype, RealmObjectClassConstructorPrototype); JSObjectSetPrototype(ctx, schemaObjectConstructor, RealmObjectClassConstructor); @@ -794,16 +793,20 @@ inline JSObjectRef ObjectWrap::create_instance_by_schema(JSContextRef // hot path. The constructor for this schema object is already cached. schemaObjectType = schemaObjects->at(schemaName); schemaObjectConstructor = schemaObjectType->constructor; + constructorPrototype = Object::validated_get_object(ctx, schemaObjectConstructor, "prototype"); } - instance = Function::construct(ctx, schemaObjectConstructor, 0, {}); + // Construct a plain object, setting the internal and prototype afterwards to avoid calling the constructor + instance = JSObjectMake(ctx, nullptr, nullptr); + JSObjectSetPrototype(ctx, instance, constructorPrototype); // save the internal object on the instance - set_internal_property(ctx, instance, internal); + set_internal(ctx, instance, internal); return instance; } else { + constructorPrototype = Object::validated_get_object(ctx, constructor, "prototype"); // creating a RealmObject with user defined constructor bool schemaExists = schemaObjects->count(schemaName); if (schemaExists) { @@ -822,14 +825,18 @@ inline JSObjectRef ObjectWrap::create_instance_by_schema(JSContextRef schemaObjectType = schemaObjects->at(schemaName); schemaObjectConstructor = schemaObjectType->constructor; - instance = Function::construct(ctx, schemaObjectConstructor, 0, {}); - set_internal_property(ctx, instance, internal); + // Construct a plain object, setting the internal and prototype afterwards to avoid calling the + // constructor + instance = JSObjectMake(ctx, nullptr, new ObjectWrap(internal)); + JSObjectSetPrototype(ctx, instance, constructorPrototype); + + // save the internal object on the instance + set_internal(ctx, instance, internal); + return instance; } schemaObjectConstructor = constructor; - value = Object::get_property(ctx, constructor, "prototype"); - constructorPrototype = Value::to_object(ctx, value); define_schema_properties(ctx, constructorPrototype, schema, false); @@ -863,8 +870,13 @@ inline JSObjectRef ObjectWrap::create_instance_by_schema(JSContextRef } } - // create the instance - instance = Function::construct(ctx, schemaObjectConstructor, 0, {}); + // Construct a plain object, setting the internal and prototype afterwards to avoid calling the constructor + instance = JSObjectMake(ctx, nullptr, new ObjectWrap(internal)); + JSObjectSetPrototype(ctx, instance, constructorPrototype); + + // save the internal object on the instance + set_internal(ctx, instance, internal); + bool instanceOfSchemaConstructor = JSValueIsInstanceOfConstructor(ctx, instance, schemaObjectConstructor, &exception); if (exception) { @@ -875,9 +887,6 @@ inline JSObjectRef ObjectWrap::create_instance_by_schema(JSContextRef throw jsc::Exception(ctx, "Realm object constructor must not return another value"); } - // save the internal object on the instance - set_internal_property(ctx, instance, internal); - schemaObjectType = new SchemaObjectType(); schemaObjects->emplace(schemaName, schemaObjectType); JSValueProtect(ctx, schemaObjectConstructor); diff --git a/src/jsc/jsc_object.hpp b/src/jsc/jsc_object.hpp index 319d280aa0..93f0fca1cc 100644 --- a/src/jsc/jsc_object.hpp +++ b/src/jsc/jsc_object.hpp @@ -171,10 +171,9 @@ inline typename ClassType::Internal* jsc::Object::get_internal(JSContextRef ctx, template <> template -inline void jsc::Object::set_internal(JSContextRef ctx, const JSObjectRef& object, typename ClassType::Internal* ptr) +inline void jsc::Object::set_internal(JSContextRef ctx, JSObjectRef& object, typename ClassType::Internal* ptr) { - auto wrap = static_cast*>(JSObjectGetPrivate(object)); - *wrap = ptr; + jsc::ObjectWrap::set_internal(ctx, object, ptr); } template <> diff --git a/src/node/node_class.hpp b/src/node/node_class.hpp index d01cf6447b..ec541b051e 100644 --- a/src/node/node_class.hpp +++ b/src/node/node_class.hpp @@ -211,7 +211,7 @@ class ObjectWrap { static bool is_instance(Napi::Env env, const Napi::Object& object); static Internal* get_internal(Napi::Env env, const Napi::Object& object); - static void set_internal(Napi::Env env, const Napi::Object& object, Internal* data); + static void set_internal(Napi::Env env, Napi::Object& object, Internal* data); static Napi::Value constructor_callback(const Napi::CallbackInfo& info); static bool has_native_method(const std::string& name); diff --git a/src/node/node_object.hpp b/src/node/node_object.hpp index 0b2a916934..adca80842e 100644 --- a/src/node/node_object.hpp +++ b/src/node/node_object.hpp @@ -224,8 +224,7 @@ inline typename ClassType::Internal* node::Object::get_internal(Napi::Env env, c template <> template -inline void node::Object::set_internal(Napi::Env env, const Napi::Object& object, - typename ClassType::Internal* internal) +inline void node::Object::set_internal(Napi::Env env, Napi::Object& object, typename ClassType::Internal* internal) { return node::ObjectWrap::set_internal(env, object, internal); } From 1834e77b57f923676bffb99eeab10306ae596ca7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Thu, 17 Mar 2022 15:56:46 +0100 Subject: [PATCH 08/22] Updating types and tests --- .../tests/src/tests/class-models.ts | 20 ++++++++++++++++--- types/index.d.ts | 7 ++++++- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/integration-tests/tests/src/tests/class-models.ts b/integration-tests/tests/src/tests/class-models.ts index 4b604d5577..9873853b1f 100644 --- a/integration-tests/tests/src/tests/class-models.ts +++ b/integration-tests/tests/src/tests/class-models.ts @@ -19,7 +19,7 @@ import { expect } from "chai"; import Realm from "realm"; -import { openRealmBefore } from "../hooks"; +import { openRealmBeforeEach } from "../hooks"; describe("Class models", () => { describe("as schema element", () => { @@ -75,7 +75,7 @@ describe("Class models", () => { }); describe("#constructor", () => { - class Person extends Realm.Object { + class Person extends Realm.Object { name!: string; static schema: Realm.ObjectSchema = { name: "Person", @@ -83,7 +83,7 @@ describe("Class models", () => { }; } - openRealmBefore({ schema: [Person] }); + openRealmBeforeEach({ schema: [Person] }); it("creates objects", function (this: RealmContext) { this.realm.write(() => { @@ -99,5 +99,19 @@ describe("Class models", () => { expect(persons[0]._objectId()).equals(person._objectId()); }); }); + + it("creates objects with values", function (this: RealmContext) { + this.realm.write(() => { + // Expect no persons in the database + const persons = this.realm.objects("Person"); + expect(persons.length).equals(0); + + const alice = new Person(this.realm, { name: "Alice" }); + expect(alice.name).equals("Alice"); + // Expect the first element to be the object we just added + expect(persons.length).equals(1); + expect(persons[0].name).equals("Alice"); + }); + }); }); }); diff --git a/types/index.d.ts b/types/index.d.ts index 0f37757cf0..fde913e639 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -210,7 +210,12 @@ declare namespace Realm { * Object * @see { @link https://realm.io/docs/javascript/latest/api/Realm.Object.html } */ - abstract class Object { + abstract class Object { + /** + * Creates a new object in the database. + */ + constructor(realm: Realm, values?: RealmInsertionModel); + /** * @returns An array of the names of the object's properties. */ From a9d3167d111da5bdd04565e43947ea5d843d7b7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Fri, 18 Mar 2022 13:56:09 +0100 Subject: [PATCH 09/22] Fixed Person and Dog constructors --- .../tests/src/schemas/person-and-dogs.ts | 25 +++++++------------ 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/integration-tests/tests/src/schemas/person-and-dogs.ts b/integration-tests/tests/src/schemas/person-and-dogs.ts index 9b9e21a10b..8a6bfcdc88 100644 --- a/integration-tests/tests/src/schemas/person-and-dogs.ts +++ b/integration-tests/tests/src/schemas/person-and-dogs.ts @@ -38,16 +38,13 @@ export const PersonSchema: Realm.ObjectSchema = { }; export class Person extends Realm.Object { - name: string; - age: number; + name!: string; + age!: number; friends!: Realm.List; dogs!: Realm.Collection; - constructor(name: string, age: number) { - super(); - - this.name = name; - this.age = age; + constructor(realm: Realm, name: string, age: number) { + super(realm, { name, age }); } static schema: Realm.ObjectSchema = PersonSchema; @@ -69,16 +66,12 @@ export const DogSchema: Realm.ObjectSchema = { }; export class Dog extends Realm.Object { - name: string; - age: number; - owner: Person; - - constructor(name: string, age: number, owner: Person) { - super(); + name!: string; + age!: number; + owner!: Person; - this.name = name; - this.age = age; - this.owner = owner; + constructor(realm: Realm, name: string, age: number, owner: Person) { + super(realm, { name, age, owner }); } static schema: Realm.ObjectSchema = DogSchema; From 2bc846050c5cc1d86d4b490dbaba92dd4dc98a73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Fri, 18 Mar 2022 14:11:18 +0100 Subject: [PATCH 10/22] Updated @realm/react useObject + useQuery --- packages/realm-react/src/useObject.tsx | 5 ++++- packages/realm-react/src/useQuery.tsx | 4 +++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/realm-react/src/useObject.tsx b/packages/realm-react/src/useObject.tsx index d058d7aadc..c5852bf996 100644 --- a/packages/realm-react/src/useObject.tsx +++ b/packages/realm-react/src/useObject.tsx @@ -45,7 +45,10 @@ export function createUseObject(useRealm: () => Realm) { * @param primaryKey - The primary key of the desired object which will be retrieved using {@link Realm.objectForPrimaryKey} * @returns either the desired {@link Realm.Object} or `null` in the case of it being deleted or not existing. */ - return function useObject(type: string | { new (): T }, primaryKey: PrimaryKey): (T & Realm.Object) | null { + return function useObject( + type: string | { new (...args: any): T }, + primaryKey: PrimaryKey, + ): (T & Realm.Object) | null { const realm = useRealm(); // Create a forceRerender function for the cachedObject to use as its updateCallback, so that diff --git a/packages/realm-react/src/useQuery.tsx b/packages/realm-react/src/useQuery.tsx index a9b547b562..2c15950918 100644 --- a/packages/realm-react/src/useQuery.tsx +++ b/packages/realm-react/src/useQuery.tsx @@ -47,7 +47,9 @@ export function createUseQuery(useRealm: () => Realm) { * @param type - The object type, depicted by a string or a class extending Realm.Object * @returns a collection of realm objects or an empty array */ - return function useQuery(type: string | ({ new (): T } & Realm.ObjectClass)): Realm.Results { + return function useQuery( + type: string | ({ new (...args: any): T } & Realm.ObjectClass), + ): Realm.Results { const realm = useRealm(); // Create a forceRerender function for the cachedCollection to use as its updateCallback, so that From 76e76fd8fc0ce325bd036fd936f9763ba8190328 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Fri, 18 Mar 2022 14:46:42 +0100 Subject: [PATCH 11/22] Updated types to fix an issue --- tests/.lock | Bin 0 -> 1184 bytes types/index.d.ts | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) create mode 100644 tests/.lock diff --git a/tests/.lock b/tests/.lock new file mode 100644 index 0000000000000000000000000000000000000000..4e04ca752400ee17d0ae4133be2a49a84d9d719e GIT binary patch literal 1184 zcmd7Qw+_G{3`J2Oz4u=K|0@GqgcR-?Z_j73Y)e@Sp;Gaex~)N^mEY0Wmz%x<_rQJd z06YYbz+>(type: {new(...arg: any[]): T; }, key: Realm.PrimaryKey): T | undefined; // Combined definitions - objectForPrimaryKey(type: string | {new(...arg: any[]): T; }, key: Realm.PrimaryKey): (T & Realm.Object) | undefined; + objectForPrimaryKey(type: string | {new(...arg: any[]): T; }, key: Realm.PrimaryKey): (T & Realm.Object) | undefined; /** * @param {string} type From ba0659970fe2f7238ad6f8659e1c60308fd3dcb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Fri, 18 Mar 2022 14:51:13 +0100 Subject: [PATCH 12/22] Dead code removal --- src/node/node_class.hpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/node/node_class.hpp b/src/node/node_class.hpp index ec541b051e..8fdec3bf2b 100644 --- a/src/node/node_class.hpp +++ b/src/node/node_class.hpp @@ -1411,12 +1411,6 @@ Napi::Value ObjectWrap::constructor_callback(const Napi::CallbackInfo return scope.Escape(env.Null()); // return a value to comply with Napi::FunctionCallback } else { - // XXX: Remove this now that we declare a constructor for the RealmObjectClass - bool isRealmObjectClass = std::is_same>::value; - if (isRealmObjectClass) { - return scope.Escape(env.Null()); // return a value to comply with Napi::FunctionCallback - } - throw Napi::Error::New(env, "Illegal constructor"); } } From 0a7fbe62c97ce3a2c33db3a8d3051527eb7568fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Thu, 24 Mar 2022 14:07:38 +0100 Subject: [PATCH 13/22] Updated tests to use default values --- .../tests/src/tests/class-models.ts | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/integration-tests/tests/src/tests/class-models.ts b/integration-tests/tests/src/tests/class-models.ts index 9873853b1f..ae417ab9ec 100644 --- a/integration-tests/tests/src/tests/class-models.ts +++ b/integration-tests/tests/src/tests/class-models.ts @@ -76,16 +76,34 @@ describe("Class models", () => { describe("#constructor", () => { class Person extends Realm.Object { + id!: Realm.BSON.ObjectId; name!: string; + age!: number; + friends!: Realm.List; + static schema: Realm.ObjectSchema = { name: "Person", - properties: { name: "string?" }, + properties: { + id: { + type: "objectId", + default: new Realm.BSON.ObjectId(), // TODO: Make this a function + }, + name: { + type: "string", + default: "John Doe", + }, + age: { + type: "int", + default: 32, + }, + friends: "Person[]", + }, }; } openRealmBeforeEach({ schema: [Person] }); - it("creates objects", function (this: RealmContext) { + it("creates objects without values", function (this: RealmContext) { this.realm.write(() => { // Expect no persons in the database const persons = this.realm.objects("Person"); @@ -111,6 +129,8 @@ describe("Class models", () => { // Expect the first element to be the object we just added expect(persons.length).equals(1); expect(persons[0].name).equals("Alice"); + // Property value fallback to the default + expect(persons[0].age).equals(32); }); }); }); From 05cdbe59c3f0edb70b80e3c038b0171aada53854 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Thu, 24 Mar 2022 14:08:04 +0100 Subject: [PATCH 14/22] Making the insertion types a bit more loose --- types/index.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/types/index.d.ts b/types/index.d.ts index 14633a0a55..f41f7b1495 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -214,7 +214,7 @@ declare namespace Realm { /** * Creates a new object in the database. */ - constructor(realm: Realm, values?: RealmInsertionModel); + constructor(realm: Realm, values?: Partial>); /** * @returns An array of the names of the object's properties. From d065e009fc423873c78fdab57ae713677ccda16e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Tue, 29 Mar 2022 11:29:36 +0200 Subject: [PATCH 15/22] Adding documentation --- src/js_realm_object.hpp | 13 +++++++++++++ src/jsc/jsc_class.hpp | 11 +++++++++++ 2 files changed, 24 insertions(+) diff --git a/src/js_realm_object.hpp b/src/js_realm_object.hpp index 9fe331adcf..9100e9fbc0 100644 --- a/src/js_realm_object.hpp +++ b/src/js_realm_object.hpp @@ -164,6 +164,19 @@ typename T::Object RealmObjectClass::create_instance(ContextType ctx, realm:: } } +/** + * @brief Implements the constructor for a Realm.Object, calling the `Realm#create` instance method to create an + * object in the database. + * + * @note This differes from `RealmObjectClass::create_instance` as it is executed when end-users construct a `new + * Realm.Object()` (or another user-defined class extending `Realm.Object`), whereas `create_instance` is called when + * reading objects from the database. + * + * @tparam T Engine specific types. + * @param ctx JS context + * @param this_object JS object being returned to the user once constructed. + * @param args Arguments passed by the user when calling the constructor. + */ template void RealmObjectClass::constructor(ContextType ctx, ObjectType this_object, Arguments& args) { diff --git a/src/jsc/jsc_class.hpp b/src/jsc/jsc_class.hpp index c0a46dc117..405bfa849f 100644 --- a/src/jsc/jsc_class.hpp +++ b/src/jsc/jsc_class.hpp @@ -621,6 +621,17 @@ typename ClassType::Internal* ObjectWrap::get_internal(JSContextRef c return realmObjectInstance->m_object.get(); } +/** + * @brief Stores data on the `instance` JS object, making it possible to retrieve the internal object insteance at a + * later point, using `ObjectWrap::get_internal`. + * + * @note This hands over ownership of the object pointed to by the `internal` pointer. + * + * @tparam ClassType The class implementing the JS interface (from C++). + * @param ctx JS context + * @param instance JS object which is handed ownership of the internal object being pointed to by `internal`. + * @param internal A pointer to an instance of a C++ object being wrapped. + */ template void ObjectWrap::set_internal(JSContextRef ctx, JSObjectRef& instance, typename ClassType::Internal* internal) From 87ab310937fda74f87196c5f5ce0517b2334a53a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Tue, 29 Mar 2022 11:30:27 +0200 Subject: [PATCH 16/22] Renamed realm_object_object --- src/js_realm_object.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/js_realm_object.hpp b/src/js_realm_object.hpp index 9100e9fbc0..f3f9dbddad 100644 --- a/src/js_realm_object.hpp +++ b/src/js_realm_object.hpp @@ -191,10 +191,10 @@ void RealmObjectClass::constructor(ContextType ctx, ObjectType this_object, A Arguments create_arguments{ctx, create_args.size(), create_args.data()}; ReturnValue result{ctx}; RealmClass::create(ctx, realm, create_arguments, result); - ObjectType realm_object_object = Value::validated_to_object(ctx, result); + ObjectType tmp_realm_object = Value::validated_to_object(ctx, result); // Copy the internal from the constructed object onto this_object - auto realm_object = get_internal>(ctx, realm_object_object); + auto realm_object = get_internal>(ctx, tmp_realm_object); // The finalizer on the ObjectWrap (applied inside of set_internal) will delete the `new_realm_object` auto new_realm_object = new realm::js::RealmObject(*realm_object); set_internal>(ctx, this_object, new_realm_object); From f4dedfb385e96d9038b852cb8b144cf683ffa61d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Tue, 29 Mar 2022 12:15:55 +0200 Subject: [PATCH 17/22] Made the constructor "values" required --- .../tests/src/tests/class-models.ts | 24 ++++--------------- types/index.d.ts | 2 +- 2 files changed, 5 insertions(+), 21 deletions(-) diff --git a/integration-tests/tests/src/tests/class-models.ts b/integration-tests/tests/src/tests/class-models.ts index ae417ab9ec..9c94d05cdb 100644 --- a/integration-tests/tests/src/tests/class-models.ts +++ b/integration-tests/tests/src/tests/class-models.ts @@ -75,7 +75,8 @@ describe("Class models", () => { }); describe("#constructor", () => { - class Person extends Realm.Object { + type UnmanagedPerson = Partial & Pick; + class Person extends Realm.Object { id!: Realm.BSON.ObjectId; name!: string; age!: number; @@ -88,10 +89,7 @@ describe("Class models", () => { type: "objectId", default: new Realm.BSON.ObjectId(), // TODO: Make this a function }, - name: { - type: "string", - default: "John Doe", - }, + name: "string", age: { type: "int", default: 32, @@ -103,21 +101,6 @@ describe("Class models", () => { openRealmBeforeEach({ schema: [Person] }); - it("creates objects without values", function (this: RealmContext) { - this.realm.write(() => { - // Expect no persons in the database - const persons = this.realm.objects("Person"); - expect(persons.length).equals(0); - - const person = new Person(this.realm); - expect(person).instanceOf(Person); - expect(person).instanceOf(Realm.Object); - // Expect the first element to be the object we just added - expect(persons.length).equals(1); - expect(persons[0]._objectId()).equals(person._objectId()); - }); - }); - it("creates objects with values", function (this: RealmContext) { this.realm.write(() => { // Expect no persons in the database @@ -128,6 +111,7 @@ describe("Class models", () => { expect(alice.name).equals("Alice"); // Expect the first element to be the object we just added expect(persons.length).equals(1); + expect(persons[0]._objectId()).equals(alice._objectId()); expect(persons[0].name).equals("Alice"); // Property value fallback to the default expect(persons[0].age).equals(32); diff --git a/types/index.d.ts b/types/index.d.ts index f41f7b1495..44520ceee5 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -214,7 +214,7 @@ declare namespace Realm { /** * Creates a new object in the database. */ - constructor(realm: Realm, values?: Partial>); + constructor(realm: Realm, values: RealmInsertionModel); /** * @returns An array of the names of the object's properties. From c08f63d3918e525957ba3fcc3422fc6fb0b340bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Tue, 29 Mar 2022 12:28:00 +0200 Subject: [PATCH 18/22] Renamed "RealmInsertionModel" to "Unmanged" --- types/index.d.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/types/index.d.ts b/types/index.d.ts index 44520ceee5..81a72b07cc 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -214,7 +214,7 @@ declare namespace Realm { /** * Creates a new object in the database. */ - constructor(realm: Realm, values: RealmInsertionModel); + constructor(realm: Realm, values: Unmanaged); /** * @returns An array of the names of the object's properties. @@ -926,10 +926,10 @@ type ExtractPropertyNamesOfType = { }[keyof T]; /** - * Exchanges properties defined as Realm.List with an optional Array>. + * Exchanges properties defined as Realm.List with an optional Array>. */ type RealmListsRemappedModelPart = { - [K in ExtractPropertyNamesOfType>]?: T[K] extends Realm.List ? Array> : never + [K in ExtractPropertyNamesOfType>]?: T[K] extends Realm.List ? Array> : never } /** @@ -956,7 +956,7 @@ type RemappedRealmTypes = * Joins T stripped of all keys which value extends Realm.Collection and all inherited from Realm.Object, * with only the keys which value extends Realm.List, remapped as Arrays. */ -type RealmInsertionModel = OmittedRealmTypes & RemappedRealmTypes; +type Unmanaged = OmittedRealmTypes & RemappedRealmTypes; declare class Realm { static defaultPath: string; @@ -1039,8 +1039,8 @@ declare class Realm { * @param {Realm.UpdateMode} mode? If not provided, `Realm.UpdateMode.Never` is used. * @returns T & Realm.Object */ - create(type: string, properties: RealmInsertionModel, mode?: Realm.UpdateMode.Never): T & Realm.Object; - create(type: string, properties: Partial | Partial>, mode: Realm.UpdateMode.All | Realm.UpdateMode.Modified): T & Realm.Object; + create(type: string, properties: Unmanaged, mode?: Realm.UpdateMode.Never): T & Realm.Object; + create(type: string, properties: Partial | Partial>, mode: Realm.UpdateMode.All | Realm.UpdateMode.Modified): T & Realm.Object; /** * @param {Class} type @@ -1048,8 +1048,8 @@ declare class Realm { * @param {Realm.UpdateMode} mode? If not provided, `Realm.UpdateMode.Never` is used. * @returns T */ - create(type: {new(...arg: any[]): T; }, properties: RealmInsertionModel, mode?: Realm.UpdateMode.Never): T; - create(type: {new(...arg: any[]): T; }, properties: Partial | Partial>, mode: Realm.UpdateMode.All | Realm.UpdateMode.Modified): T; + create(type: {new(...arg: any[]): T; }, properties: Unmanaged, mode?: Realm.UpdateMode.Never): T; + create(type: {new(...arg: any[]): T; }, properties: Partial | Partial>, mode: Realm.UpdateMode.All | Realm.UpdateMode.Modified): T; /** * @param {Realm.Object|Realm.Object[]|Realm.List|Realm.Results|any} object From 6f8cff204ef0374720a452eb626615c18ab32402 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Thu, 17 Mar 2022 16:00:03 +0100 Subject: [PATCH 19/22] Adding a note to the changelog --- CHANGELOG.md | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 32bb9cf21a..ab4c3c3e4f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,10 +2,31 @@ x.x.x Release notes (yyyy-MM-dd) ============================================================= ### Breaking change -* Model classes passed as schema to the `Realm` constructor must now extend `Realm.Object`. - -### Enhancements -* None. +* Model classes passed as schema to the `Realm` constructor must now extend `Realm.Object` and will no longer have their constructors called when pulling an object of that type from the database. Existing classes already extending `Realm.Object` now needs to call the `super` constructor passing two arguments: + - `realm`: The realm to create the object in. + - `values`: Values to pass to the `realm.create` call when creating the object in the database. +* Renamed the `RealmInsertionModel` type to `Unmanaged` to simplify and highlight its usage. + +### Enhancements +* Class-based models (i.e. user defined classes extending `Realm.Object` and passed through the `schema` when opening a Realm), will now create object when their constructor is called: + +```ts +class Person extends Realm.Object { + name!: string; + + static schema = { + name: "Person", + properties: { name: "string" }, + }; +} + +const realm = new Realm({ schema: [Person] }); +realm.write(() => { + const alice = new Person(realm, { name: "Alice" }); + // A Person { name: "Alice" } is now persisted in the database + console.log("Hello " + alice.name); +}); +``` ### Fixed * Fixed issue that could cause mangling of binary data on a roundtrip to/from the database ([#4278](https://github.com/realm/realm-js/issues/4278), since v10.1.4). From 0c1ce10fb801342e953594086cf490cc5a9dd021 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Wed, 30 Mar 2022 11:59:33 +0200 Subject: [PATCH 20/22] Apply suggestions to docstrings Co-authored-by: Kenneth Geisshirt --- CHANGELOG.md | 4 ++-- src/js_realm_object.hpp | 4 ++-- src/jsc/jsc_class.hpp | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab4c3c3e4f..bbcf5065eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,8 +2,8 @@ x.x.x Release notes (yyyy-MM-dd) ============================================================= ### Breaking change -* Model classes passed as schema to the `Realm` constructor must now extend `Realm.Object` and will no longer have their constructors called when pulling an object of that type from the database. Existing classes already extending `Realm.Object` now needs to call the `super` constructor passing two arguments: - - `realm`: The realm to create the object in. +* Model classes passed as schema to the `Realm` constructor must now extend `Realm.Object` and will no longer have their constructors called when pulling an object of that type from the database. Existing classes already extending `Realm.Object` now need to call the `super` constructor passing two arguments: + - `realm`: The Realm to create the object in. - `values`: Values to pass to the `realm.create` call when creating the object in the database. * Renamed the `RealmInsertionModel` type to `Unmanaged` to simplify and highlight its usage. diff --git a/src/js_realm_object.hpp b/src/js_realm_object.hpp index f3f9dbddad..5a52e8a075 100644 --- a/src/js_realm_object.hpp +++ b/src/js_realm_object.hpp @@ -168,12 +168,12 @@ typename T::Object RealmObjectClass::create_instance(ContextType ctx, realm:: * @brief Implements the constructor for a Realm.Object, calling the `Realm#create` instance method to create an * object in the database. * - * @note This differes from `RealmObjectClass::create_instance` as it is executed when end-users construct a `new + * @note This differs from `RealmObjectClass::create_instance` as it is executed when end-users construct a `new * Realm.Object()` (or another user-defined class extending `Realm.Object`), whereas `create_instance` is called when * reading objects from the database. * * @tparam T Engine specific types. - * @param ctx JS context + * @param ctx JS context. * @param this_object JS object being returned to the user once constructed. * @param args Arguments passed by the user when calling the constructor. */ diff --git a/src/jsc/jsc_class.hpp b/src/jsc/jsc_class.hpp index 405bfa849f..aa895d046d 100644 --- a/src/jsc/jsc_class.hpp +++ b/src/jsc/jsc_class.hpp @@ -622,7 +622,7 @@ typename ClassType::Internal* ObjectWrap::get_internal(JSContextRef c } /** - * @brief Stores data on the `instance` JS object, making it possible to retrieve the internal object insteance at a + * @brief Stores data on the `instance` JS object, making it possible to retrieve the internal object instance at a * later point, using `ObjectWrap::get_internal`. * * @note This hands over ownership of the object pointed to by the `internal` pointer. From bca48d9c60d0aae0a9b4b941dbe5208598d64d4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Wed, 30 Mar 2022 14:17:07 +0200 Subject: [PATCH 21/22] Add docstring of set_internal and get_internal --- src/js_types.hpp | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/js_types.hpp b/src/js_types.hpp index 002819ce74..3409c468c0 100644 --- a/src/js_types.hpp +++ b/src/js_types.hpp @@ -560,12 +560,33 @@ REALM_JS_INLINE typename T::Object create_instance_by_schema(typename T::Context return Object::template create_instance_by_schema(ctx, schema, internal); } +/** + * @brief Get the internal (C++) object backing a JS object. + * + * @tparam T Engine specific types. + * @tparam ClassType Class implementing the C++ interface backing the JS accessor object (passed as `object`). + * @param ctx JS context. + * @param object JS object with an internal object. + * @return Pointer to the internal object. + */ template REALM_JS_INLINE typename ClassType::Internal* get_internal(typename T::Context ctx, const typename T::Object& object) { return Object::template get_internal(ctx, object); } +/** + * @brief Set the internal (C++) object backing the JS object. + * + * @note Calling this transfer ownership of the object pointed to by `ptr` and links it to the lifetime of to the + * `object` passed as argument. + * + * @tparam T Engine specific types. + * @tparam ClassType Class implementing the C++ interface backing the JS accessor object (passed as `object`). + * @param ctx JS context. + * @param object JS object having its internal set. + * @param ptr A pointer to an internal object. + */ template REALM_JS_INLINE void set_internal(typename T::Context ctx, typename T::Object& object, typename ClassType::Internal* ptr) From 104ed35f4265f003e28e2b6ba3884a5b5458696a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Wed, 30 Mar 2022 14:17:33 +0200 Subject: [PATCH 22/22] Expect 2 arguments on the C++ code as well --- src/js_realm_object.hpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/js_realm_object.hpp b/src/js_realm_object.hpp index 5a52e8a075..6cc4097928 100644 --- a/src/js_realm_object.hpp +++ b/src/js_realm_object.hpp @@ -181,10 +181,10 @@ template void RealmObjectClass::constructor(ContextType ctx, ObjectType this_object, Arguments& args) { // Parse aguments - args.validate_between(1, 2); + args.validate_count(2); auto constructor = Object::validated_get_object(ctx, this_object, "constructor"); auto realm = Value::validated_to_object(ctx, args[0], "realm"); - auto values = Value::is_undefined(ctx, args[1]) ? Object::create_empty(ctx) : args[1]; + auto values = Value::validated_to_object(ctx, args[1], "values"); // Create an object std::vector create_args{constructor, values}; @@ -195,7 +195,9 @@ void RealmObjectClass::constructor(ContextType ctx, ObjectType this_object, A // Copy the internal from the constructed object onto this_object auto realm_object = get_internal>(ctx, tmp_realm_object); - // The finalizer on the ObjectWrap (applied inside of set_internal) will delete the `new_realm_object` + // The finalizer on the ObjectWrap (applied inside of set_internal) will delete the `new_realm_object` which is + // why we create a new instance to avoid a double free (the first of which will happen when the `tmp_realm_object` + // destructs). auto new_realm_object = new realm::js::RealmObject(*realm_object); set_internal>(ctx, this_object, new_realm_object); }