-
Notifications
You must be signed in to change notification settings - Fork 369
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
New schema docs #3290
base: develop
Are you sure you want to change the base?
New schema docs #3290
Conversation
🚀 Deployed on https://deploy-preview-3290--moor.netlify.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.
LGTM apart from the remaining comments on other pages. I can take care of them too.
https://drift.simonbinder.eu/custom_row_classes/ /dart_api/dataclass/ | ||
https://drift.simonbinder.eu/type_converters/ /dart_api/tables/#custom-types |
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.
Shouldn't these be without the domain like the others above?
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.
Yes. Remove the leading url
|
||
{{ load_snippet('data-class-name','lib/snippets/dart_api/dataclass.dart.excerpt.json') }} | ||
|
||
## Json serialization |
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 this should not be this far up - and there should be a little warning saying that using custom row classes is the preferred approach for JSON serialization.
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.
Why?
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.
Drift is a relational persistence library, and ideally shouldn't have to do anything to do with serialization. It's a historical artifact that I regret, but it's too late to fix this now.
Our serialization capabilities are incredibly restricted compared to packages that actually intend to do serialziation, which is why I recommend using custom row classes to combine drift with other libraries where necessary.
@@ -1,23 +1,112 @@ | |||
--- | |||
|
|||
title: Custom row classes | |||
description: Use your own classes as data classes for drift tables | |||
title: Dataclass |
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 still not too happy with the generic Dataclass
name, does "Generated classes" with a description like "Overview of classes generated by drift for reading and writing to the database" work?
|
||
<div class="annotate" markdown> | ||
|
||
{{ load_snippet('generated-companion','lib/snippets/dart_api/dataclass.dart.excerpt.json') }} |
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 we should use a tabbed view here with Manager vs. the Core query builder instead of having it in a single snippet.
### Value object | ||
When using the companion object to update a row, optional fields must be wrapped in a `Value` object. This is used by Drift to distinguish between `null` and not present values. | ||
|
||
{{ load_snippet('generated-value','lib/snippets/dart_api/dataclass.dart.excerpt.json') }} |
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 example really isn't too helpful. Values were also used in the example above for updates, perhaps we could reference that here.
This is used by Drift to distinguish between
null
and not present values.
A better explanation would be to mention what drift actually does with that: For updates, there's a distinction between setting the value of a column to null
vs. not touching it at all. We can't represent without introducing some kind of optional structure, which is what Value
is.
|
||
[^1]: Like bigger than 4,503,599,627,370,496! | ||
|
||
Using `dartCast<BigInt>()` will ensure that the result is interpreted as a `BigInt` by drift. |
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 also note that this doesn't change the behavior in SQL at all.
@@ -76,6 +76,20 @@ bitwise operations: | |||
|
|||
{{ load_snippet('bitwise','lib/snippets/dart_api/expressions.dart.excerpt.json') }} | |||
|
|||
### BigInt | |||
|
|||
You may want to cast an expression to a `BigInt` if: |
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 may want to cast an expression to a `BigInt` if: | |
You may want to cast an expression to a `BigInt` if both of these apply to your app: |
|
||
You may want to cast an expression to a `BigInt` if: | ||
|
||
- The result of an arithmetic operation will be extremely large[^1]. |
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's the point of being imprecise here? We can just write
- The result of an arithmetic operation will be extremely large[^1]. | |
- The result of an arithmetic operation is not representable by a `double` (i.e., it is larger than `2^52`). |
@@ -1,17 +1,11 @@ | |||
--- | |||
|
|||
title: Manager | |||
title: Queries |
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.
IMO we should leave this as-is before the restructuring is more complete. At the moment we'd then effectively have multiple pages on queries which is more confusing than before.
We can name it "Queries with Manager" or something and also point out in the introduction/getting started guide that there are two ways to write these queries (like we already do on the new table page).
Co-authored-by: Simon Binder <[email protected]>
This PR: