-
Notifications
You must be signed in to change notification settings - Fork 88
(DOCSP-26984): @realm/reactify: Mixed - React Native SDK #2411
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-26984): @realm/reactify: Mixed - React Native SDK #2411
Conversation
eb2ab50
to
8f60cd1
Compare
Readability for Commit Hash: 3c5f807 You can see any previous Readability scores (if they exist) by looking Flesch Reading Ease scores for changed documents:
The following table can be helpful in assessing the readability score of a document.
|
examples/react-native/__tests__/js/realm-database/schemas/mixed-tests.jsx
Show resolved
Hide resolved
examples/react-native/__tests__/js/realm-database/schemas/mixed-tests.jsx
Show resolved
Hide resolved
examples/react-native/__tests__/js/realm-database/schemas/mixed-tests.jsx
Outdated
Show resolved
Hide resolved
examples/react-native/__tests__/js/realm-database/schemas/mixed-tests.jsx
Show resolved
Hide resolved
@@ -0,0 +1,145 @@ | |||
import React, {useEffect} from 'react'; |
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.
all comments/suggestions on examples/react-native/__tests__/js/realm-database/schemas/mixed-tests.jsx
also apply to this file
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.
still applies for remaining nit https://github.com/mongodb/docs-realm/pull/2411/files#r1061575928
type except a collection. You can create collections (lists, sets, and | ||
dictionaries) of type ``mixed``, but a ``mixed`` type itself cannot be a | ||
collection. Properties using the mixed data type can also hold null values. |
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.
outside of scope of this PR, but would be good to have example of using mixed type with a collection.
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.
Interesting idea, I'm not opposed to it, but don't necessarily want to bulk up this page. Maybe a note saying for additional examples, see ?
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.
what would that note point to?
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.
Possibly an "Additional Examples" page like we used to have for C#?
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.
nice work! i like the way you handled the @realm/react-ification of the examples.
couple small suggestions/nits throughout.
and then some larger comments about how the data type is described that fall outside of the scope of this PR, but should probably be addressed. could be good work for @krollins-mdb to learn more about the SDK.
|
||
useEffect(() => { | ||
// Add data to the Realm when the component mounts |
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.
useEffect(() => { | |
// Add data to the Realm when the component mounts | |
// Add data to the Realm when the component mounts | |
useEffect(() => { |
nit: think it'd be clearer if this comment before the hook.
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 like it where it is because the comment is directly above the write transaction where you add the data
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.
lgtm.
couple little things to fix and 1 open question, but nothing blocking.
Co-authored-by: Ben Perlmutter <[email protected]>
## Pull Request Info ### Jira - https://jira.mongodb.org/browse/DOCSP-26984 ### Staged Changes - [Mixed - React Native SDK](https://docs-mongodbcom-staging.corp.mongodb.com/realm/docsworker-xlarge/mixed-object/sdk/react-native/realm-database/schemas/mixed/) ### Reminder Checklist If your PR modifies the docs, you might need to also update some corresponding pages. Check if completed or N/A. - [N/A] Create Jira ticket for corresponding docs-app-services update(s), if any - [N/A] Checked/updated Admin API - [N/A] Checked/updated CLI reference ### Review Guidelines [REVIEWING.md](https://github.com/mongodb/docs-realm/blob/master/REVIEWING.md) Co-authored-by: Mohammad Hunan Chughtai <[email protected]> Co-authored-by: Ben Perlmutter <[email protected]>
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.
What we are missing here is a nice method called Realm.Object.getPropertyType(propertyName:string)
. This will return the underlying property type as a string. This is useful when trying to do some type checking on a mixed
property.
The cat example could use this to logically depict how it should interpret the given birthday
type.
## Pull Request Info ### Jira - https://jira.mongodb.org/browse/DOCSP-26984 ### Staged Changes - [Mixed - React Native SDK](https://docs-mongodbcom-staging.corp.mongodb.com/realm/docsworker-xlarge/mixed-object/sdk/react-native/realm-database/schemas/mixed/) ### Reminder Checklist If your PR modifies the docs, you might need to also update some corresponding pages. Check if completed or N/A. - [N/A] Create Jira ticket for corresponding docs-app-services update(s), if any - [N/A] Checked/updated Admin API - [N/A] Checked/updated CLI reference ### Review Guidelines [REVIEWING.md](https://github.com/mongodb/docs-realm/blob/master/REVIEWING.md) Co-authored-by: Mohammad Hunan Chughtai <[email protected]> Co-authored-by: Ben Perlmutter <[email protected]>
Pull Request Info
Jira
Staged Changes
Reminder Checklist
If your PR modifies the docs, you might need to also update some corresponding
pages. Check if completed or N/A.
Review Guidelines
REVIEWING.md