-
Notifications
You must be signed in to change notification settings - Fork 88
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
(DOCSP-39502): Consolidate Define an Object Model page #3314
(DOCSP-39502): Consolidate Define an Object Model page #3314
Conversation
Readability for Commit Hash: 1e30fb0 You can see any previous Readability scores (if they exist) by looking Readability scores for changed documents:
For Grade Level, aim for 8 or below. For Reading Ease scores, aim for 60 or above:
For help improving readability, try Hemingway App. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some suggestions for the JS object model info.
...api-details/javascript/model-data/object-models-define-embedded-object-js-ts-description.rst
Outdated
Show resolved
Hide resolved
(like most of the examples on this page). Or you can define models as | ||
JavaScript objects. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After talking with the JS SDK team, it makes sense to simplify the approach to only using classes. With the caveat that we never use new
to construct a new instance. Instead, we define a model with a class, then create objects use an object literal. Like this:
class Book extends Realm.Object {
// Don't forget to declare these.
title!: string;
price!: number;
static schema: ObjectSchema = {
name: "Book",
properties: {
title: "string",
price: "double",
},
};
}
const book = realm.write(() => {
return realm.create(Book, { title: "My Title", price: 20.25 });
});
// TS knows about these fields from the declarations in the model.
book.title;
book.price;
Passing the class into realm.create
, followed by an object literal, gives us a better dev experience in TypeScript. It doesn't provide any benefits in JavaScript, but we should use the same approach in both for consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should note that you shouldn't use new
with class-based models for a few reasons:
- It calls
realm.create()
, so must always be used in a write transaction - It doesn't play well with embedded objects
It really comes down to when objects are just objects and when they are managed objects. Calling new
automatically creates a managed object.
Not calling new
lets you create unmanaged objects, then pass them and the class model to realm.create()
when it makes sense to do so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it! Thanks for clarifying these details with the JS team.
I've updated the wording here and in the TS description. Can you take a look and make sure this accurately communicates the important points?
I do worry that we're now saying something in the docs that is not consistent with all of our code examples. But I guess that's a problem to address in the consolidation gaps epic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We seem to mostly adhere to the guidance - at least on this page. The Node.js examples definitely need work. They haven't been modernized/standardized in a long time (since before I started at MongoDB).
If you do define a model that does not extend ``Realm.Object``, you cannot pass | ||
the model directly to the database. Instead, you pass only the schema to the | ||
database that manages the object. | ||
|
||
.. TODO: Provide more info about why you would choose one of these approaches over the other |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on my comment above, we don't recommend this and should probably avoid talking about it.
When the SDK model does *not* extend ``Realm.Object``, as when it is an | ||
embedded object, you pass only the object's schema to the database ``schema`` | ||
list. You cannot pass the object itself directly to the database. | ||
|
||
.. TODO: Verify that this is accurate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's leave this approach out of the docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Requesting changes mainly for some remaining "realm" stuff. I had a question in particular about "Realm object" that will affect a good number of my suggestions.
@dacharyc, thank you for doing this difficult and delicate work! Having everything together like this makes it much easier to understand how to work with objects. My brain is spaghetti after reading it. It must have been such a bear to put together.
A higher-level question: do we want to introduce a generic managed vs not managed objects section that talks about the different between language objects and realm objects?
.. tab:: | ||
:tabid: typescript | ||
|
||
.. include:: /includes/api-details/javascript/model-data/object-models-object-schema-js-ts-description.rst | ||
|
||
.. _sdks-define-objects: | ||
|
||
Define an SDK Object Model |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This section feels odd because it's the heading is exact same as the page title, but it's kind of just there without any words to introduce the section or explain why there's a code example here.
Feels like this was meant to be integrated into another section?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hah. Good catch. It only looks like that because I forgot to go back and add descriptions for JavaScrpt/TypeScript after our convos with engineering - all of the other languages have text here. This is "How to Define an Object Model," but Cory and I talked about it, and we don't really use "How to" in our heading titles.
I will add in the JavaScript/TypeScript descriptions, and will experiment with tweaking the headings so they're not exactly the same.
#. At a minimum, add the two fields required by the GeoJSON spec: | ||
|
||
- A field of type ``IList<double>`` that maps to a "coordinates" (case sensitive) | ||
property in the realm schema. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"realm" should probably be changed to "database", right?
Use the Embedded Class | ||
`````````````````````` | ||
|
||
You then use the custom GeoPoint class in your Realm object model, as shown in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You then use the custom GeoPoint class in your Realm object model, as shown in | |
You then use the custom GeoPoint class in your SDK object model, as shown in |
@@ -0,0 +1,8 @@ | |||
In C#, to define a Realm object, inherit from the the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In C#, to define a Realm object, inherit from the the | |
In C#, to define an SDK object, inherit from the the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in this case, we are specifically talking about Realm object versus the other object types (Asymmetric, Embedded) so this is an appropriate Realm usage.
|
||
Then, use the generated ``RealmObject`` model in your application code. | ||
|
||
The following example shows how to model an embedded object in a Realm schema. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Realm schema" to "database schema"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. This one is tricky - I'm going to go with "Realm object schema" because we're specifically talking about the schema for a Realm object.
@@ -0,0 +1,2 @@ | |||
In Swift, to define a Realm object, create a class that inherits from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Swift, to define a Realm object, create a class that inherits from | |
In Swift, to define an SDK object, create a class that inherits from |
#. At a minimum, add the two fields required by the GeoJSON spec: | ||
|
||
- A field of type ``double[]`` that maps to a "coordinates" (case sensitive) | ||
property in the realm schema. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
property in the realm schema. | |
property in the database schema. |
The SDK provides three specific object types, plus support for using an object | ||
with geospatial data: | ||
|
||
- **Realm object**: the base object type used by the SDK. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sticking with Realm for this use case? Could we use "managed object" instead? The decision here would impact my suggestions in other places to replace "realm object".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's important to stick with Realm
for this use case, since literally all the APIs and API docs refer to it that way. As it's just the name of a specific object type in this context, I don't think this is inconsistent with Realm database rebranding.
.. _sdks-define-special-property-types: | ||
|
||
Define Special Property Types | ||
----------------------------- | ||
Define Special Property Behaviors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know it's from the old content, but are these really "special"? I think "Define Property Behaviors" would work better.
@@ -0,0 +1,3 @@ | |||
In the Kotlin SDK, to define an asymmetric object type, create a Kotlin class | |||
that implements the :kotlin-sdk:`AsymmetricRealmObject | |||
<io.realm.kotlin.types/-asymmetric-realm-object/index.html>` interface: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm getting a 403 on this link. However, I can navigate to the right page and copy-pasting from there works. It's the exact same URL, so this could absolutely be a me thing.
Resolved URL that works for me: https://www.mongodb.com/docs/realm-sdks/kotlin/latest/library-sync/io.realm.kotlin.types/-asymmetric-realm-object/index.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! This was my error - this link should use the :kotlin-sync-sdk:
role, not the :kotlin-sdk:
role. Updating!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent work, @dacharyc! Thank you for putting so much thought and effort into this.
ebc8e9d
into
mongodb:feature-consolidated-sdk-docs
✨ Staging URL: https://preview-mongodbmongodb.gatsbyjs.io/realm/feature-consolidated-sdk-docs/ 🪵 Logs |
Pull Request Info - SDK Docs Consolidation
Jira ticket: https://jira.mongodb.org/browse/DOCSP-39502
Staged Page
Page Source
Add links to every SDK's pages where you got the SDK-specific information:
PR Author Checklist
Before requesting a review for your PR, please check these items:
feature-consolidated-sdk-docs
branch instead ofmaster
Naming
.rst
files comply with the naming guidelinesLinks and Refs
Content
Reviewer Checklist
As a reviewer, please check these items: