-
Notifications
You must be signed in to change notification settings - Fork 64
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
feat: use a different associated field for references on native #818
Conversation
f4c7525
to
52678cc
Compare
245be17
to
b668fd2
Compare
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.
naming nit, otherwise LGTM
// native uses the connected field instead of associatedWithFields | ||
// when using references associatedWithFields and associatedWithNative are not the same | ||
// getConnectedFieldV2 also ensures there is a matching belongsTo field with references | ||
const associatedWithNativeReferences = getConnectedFieldV2(field, model, otherSide, connectionDirective.name, shouldUseModelNameFieldInHasManyAndBelongsTo) |
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.
naming nit: can we come up with something more descriptive than V2
? :) Maybe getConnectedFieldForNativeReferences
or something?
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.
Looks like the getConnectedFieldV2
API was around for a while and used in multiple places. We can maybe refactor this outside of this PR?
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 moved the refactor to #822
50cd346
to
14ee319
Compare
Description of changes
respectReferences
and enable references for all platformsExample:
The
.hasMany
on the generated model for native platforms will useassociatedWith: primary
instead ofassociatedWith: ["primaryTenantId", "primaryInstanceId", "primaryRecordId"]
. This is how composite primary keys work for native currently.Codegen Paramaters Changed or Added
N/A
Issue #, if available
N/A
Description of how you validated changes
Checklist
yarn test
passesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.