-
Notifications
You must be signed in to change notification settings - Fork 41
feature: Type relationships as TExpand
#135
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
base: main
Are you sure you want to change the base?
Conversation
|
Thanks for the contribution and putting the work into this. It's been attempted a few times before but was ultimately too complicated. Does it handle nested fields? They can be up to 6 levels deep. |
I'm afraid not @patmood - as it types it as |
|
Hey @patmood, so I'm testing that IRL, and I changed it to expand with
Of course, as we don't know what the user queried for, it shows all possible outcomes. I tried to contribute to SDK with that feature, but it wasn't approved. But, in theory we could try to implement this at generated types level. |
|
@patmood any thoughts? |
|
Sorry I haven’t had a chance to dive into this yet.
It’s definitely impressive but a trade off between complexity and
usefulness.
If it shows all possible results then you’ll still need to validate in
code. Wouldn’t it be better to provide the expected type?
I will do a proper review but I need to find enough time to understand it.
…On Fri, 8 Aug 2025 at 4:36 am, Wojciech Grzebieniowski < ***@***.***> wrote:
*g12i* left a comment (patmood/pocketbase-typegen#135)
<#135 (comment)>
@patmood <https://github.com/patmood> any thoughts?
—
Reply to this email directly, view it on GitHub
<#135 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA6YGPFCQXERUTPBFE64DXD3MOMBHAVCNFSM6AAAAACCD7IQVKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTCNRVGMZDGNJUHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
I haven't forgotten about this. I think the pb_schema in the repo must be outdated and is missing some relation metadata, which is why the output is unchanged for the example types. I will update it and finish reviewing. |
patmood
left a comment
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.
Sorry for the slow review @g12i - This is really good stuff though.
To get this merged I this merged I think we need to:
- get to the bottom of collectionId in the schema (were you using an old PB version?)
- after fixing that, generate new example types
- fix merge conflicts with latest changes
Are you still interested in getting this merged?
| system: boolean | ||
| required: boolean | ||
| unique: boolean | ||
| options?: { collectionId?: string } |
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 does not exist in the latest version of pocketbase. collectionId is just part of the FieldSchema. So you know where you go this from? Was it a change in a recent version of pocketbase?
| options?: { collectionId?: string } | |
| // older pocketbase versions nested data under an `options` field | |
| options?: { collectionId?: string } | |
| collectionId?: string |
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.
Hey @patmood, I need to check. But it might take a while, as I've been doing this on another machine. Will come back to you!
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.
So I was using 0.22.22 and in my schema it's in options, pic rel.
From what I see it was changed in pocketbase/pocketbase@844f18c#diff-461ee4ec16043fa76300b50766bb2085d3277bbc5c779499d222c369052359eb
Use to be in RelationOptions
Now it's in RelationField
If I understand Go correctly
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.
Cool I think we can support both with a comment that the old structure is only up X version. That way it's backwards compatible.
| relationNodes.forEach((childNode) => { | ||
| childNode.fields.forEach((field) => { | ||
| const parentId = | ||
| field.type === "relation" ? field.options?.collectionId : undefined |
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 was why I couldn't get it working....
| field.type === "relation" ? field.options?.collectionId : undefined | |
| field.options?.collectionId || field.collectionId |
|
Hey @g12i - Did you want to make those updates? If not, I have some time and can merge this into a different branch and take it over. |
|
Hey @patmood, sorry I was on holiday. Now I'm back and kind of busy with day-to-day, so feel free to take it over |

This PR adds support for mapping
TExpandfield with actual relationships between types.For example, given 3 collections:
Author -> (has many) -> Courses -> (has many) Chapters -> (has many) (sub) Chapters
It would generate following schema: