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

Allow specifying required fields on Realm.Object #5000

Merged
merged 5 commits into from
Oct 18, 2022

Conversation

tomduncalf
Copy link
Contributor

What, How & Why?

The current types for Realm.Object (post-class based models) make it so that when you construct an object with new MyObject(realm: Realm, objectValues: Record<string, whatever>), you are required to specify a value for every property of the schema in objectValues.

In the case of schemas with default values specified, this means you would need to work around type errors if you do not want to specify an explicit value for the properties with defaults – e.g. you could specify fieldName: undefined which would result in the default being used, which works but is a little clumsy.

This is likely to become a more common use case post-Typescript models, as we are making it very natural to specify schema defaults.

This change makes it so that all fields of the schema are optional, unless explicitly specified as part of a union type for the second type parameter: class MyObject extends Realm.Object<MyObject, 'requiredField1' | 'requiredField2'> – so users should specify any required fields (i.e. ones which do not have a default value) in this type parameter.

If required fields are not specified in the type parameter, TypeScript will treat them as optional, which could lead to errors where required fields are not being passed in. On the other hand, this will not break existing code as we are by default relaxing the strictness of the type.

The reason behind making all fields optional by default rather than required is that core has an internal default value for all types, so this will not lead to runtime errors. It would of course be much nicer to be able to infer which fields have a default value being assigned but I don't think there's any way to express this in TypeScript.

☑️ ToDos

  • 📝 Changelog entry
  • 📝 Compatibility label is updated or copied from previous entry
  • 📝 Update COMPATIBILITY.md
  • 🚦 Tests
  • 🔀 Executed flexible sync tests locally if modifying flexible sync
  • 📦 Updated internal package version in consuming package.jsons (if updating internal packages)
  • 📱 Check the React Native/other sample apps work if necessary
  • 📝 Public documentation PR created or is not necessary
  • 💥 Breaking label has been applied or is not necessary

If this PR adds or changes public API's:

  • typescript definitions file is updated
  • jsdoc files updated
  • Chrome debug API is updated if API is available on React Native

@cla-bot cla-bot bot added the cla: yes label Oct 11, 2022
@tomduncalf tomduncalf mentioned this pull request Oct 11, 2022
12 tasks
@kraenhansen kraenhansen force-pushed the v11 branch 2 times, most recently from 7674954 to 916ea33 Compare October 18, 2022 08:30
@kraenhansen kraenhansen force-pushed the td/required-fields-change branch from 92336a9 to ff3ba57 Compare October 18, 2022 12:12
@kraenhansen kraenhansen changed the base branch from v11 to master October 18, 2022 12:12
@kraenhansen kraenhansen force-pushed the td/required-fields-change branch from ff3ba57 to 2d9293d Compare October 18, 2022 12:27
@kraenhansen kraenhansen merged commit 47555c4 into master Oct 18, 2022
@kraenhansen kraenhansen deleted the td/required-fields-change branch October 18, 2022 12:31
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants