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

feat(codegen,types): SetRelation on expanded types #3477

Merged
merged 7 commits into from
Mar 15, 2023

Conversation

patrick-medusajs
Copy link
Contributor

What

On expanded relations, mutate type using SetRelation, an alias for SetRequired with SetNonNullable.

Why

Simplifies implementation in client code.

How

  • Export SetRelation type util
  • Update codegen template

Extra scope:

  • Improve codegen error handling when processing x-expanded-relations
  • Add eager sub-categorization to x-expanded-relations to highlight relations that are loaded using TypeORM eager.

@patrick-medusajs patrick-medusajs requested a review from a team as a code owner March 14, 2023 21:35
@vercel
Copy link

vercel bot commented Mar 14, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
medusa-docs ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Mar 15, 2023 at 11:36AM (UTC)

@changeset-bot
Copy link

changeset-bot bot commented Mar 14, 2023

🦋 Changeset detected

Latest commit: 2d4c252

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@medusajs/openapi-typescript-codegen Minor
@medusajs/client-types Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

? findPropInAllOf(nestedRelation.field, model, allModels)
: getPropertyByName(nestedRelation.field, model)
const prop = ["all-of", "any-of", "all-of"].includes(model.export)
? findPropInAllOf(nestedRelation.field, model, allModels)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: this method might need a name change. Got nothing to do with AllOf alone anymore

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, all-of is duplicated

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch 🎣. Fixed in 1759dbf

? findPropInAllOf(nestedRelation.field, model, allModels)
: getPropertyByName(nestedRelation.field, model)
const prop = ["all-of", "any-of", "all-of"].includes(model.export)
? findPropInAllOf(nestedRelation.field, model, allModels)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, all-of is duplicated

NestedRelation: ${JSON.stringify(nestedRelation, null, 2)}
Model: ${JSON.stringify(model.spec, null, 2)}`)
}
if (["all-of", "any-of", "all-of"].includes(prop.export)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

todo: all-of duplicated

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch 🎣. Fixed in 1759dbf

@kodiakhq kodiakhq bot merged commit 826d4be into develop Mar 15, 2023
@kodiakhq kodiakhq bot deleted the feat/codegen-types-set-relation branch March 15, 2023 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants