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): x-expanded-relations #3442

Merged
merged 8 commits into from
Mar 13, 2023

Conversation

patrick-medusajs
Copy link
Contributor

@patrick-medusajs patrick-medusajs commented Mar 10, 2023

What

Alter generated types base on x-expanded-relations OAS extension declared on schemaObjects.

Why

Often, API endpoints will automatically expand a model relations by default. They can also decorate a model with calculated totals. In order to more accurately represent the API, we wish to alter the generated types based on the expanded relations information.

How

  • Follow the relation declaration signature as the backend controllers and the expand query param, i.e.: items.variant.product.
  • Introduce a custom x-expended-relations OAS extension.
  • Allow for organizing declared relations to help their maintenance.
  • Use traversal algorithms in codegen to support deeply nested relationships.
  • Use type-fest's Merge and SetRequired to efficiently alter the types while enabling great intellisense for IDEs.

Extra scope:

  • Added convenience yarn script to interact with the medisa-oas CLI within the monorepo.

Test

Include in the PR are two implementations of the x-expanded-relations on OAS schema, a simple and a complex one.

Step 1

  • Run yarn install
  • Run yarn build
  • Run yarn medusa-oas oas --type combined --out-dir ~/tmp/oas
  • Run yarn medusa-oas client --type combined --component types --src-file ~/tmp/oas/combined.osa.json --out-dir ~/tmp/types
  • Open ~/tmp/types/models/StoreRegionsRes
  • Expect relations to be declared as required

Step 2

  • Open ~/tmp/types/models/StoreCartsRes
  • Expect relations to be declared as required
  • Expect nested relations to have relations as required.

Step 3 (optional)

  • Open ~/tmp/types in an intellisense capable IDE
  • Within the index.ts file, attempt to declare a const storeRegionRes: StoreRegionRes = {}
  • Expect IDE to highlight that countries is a required field of StoreRegionRes

@changeset-bot
Copy link

changeset-bot bot commented Mar 10, 2023

🦋 Changeset detected

Latest commit: 708eefa

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

This PR includes changesets to release 1 package
Name Type
@medusajs/openapi-typescript-codegen 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

@vercel
Copy link

vercel bot commented Mar 10, 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 13, 2023 at 2:18PM (UTC)

@patrick-medusajs patrick-medusajs force-pushed the feat/codegen-x-extended-relations branch 5 times, most recently from 8738a6c to 5503c91 Compare March 10, 2023 19:25
@patrick-medusajs patrick-medusajs force-pushed the feat/codegen-x-extended-relations branch from 1eb2469 to a928ed2 Compare March 10, 2023 20:15
@patrick-medusajs patrick-medusajs self-assigned this Mar 10, 2023
@patrick-medusajs patrick-medusajs marked this pull request as ready for review March 10, 2023 20:51
@patrick-medusajs patrick-medusajs requested a review from a team as a code owner March 10, 2023 20:51
Copy link
Contributor

@olivermrbl olivermrbl left a comment

Choose a reason for hiding this comment

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

Great job @patrick-medusajs - it works as expected!

What do you think are good next steps?

It seems like adding this to all responses is a lot of work, but it might be best to get through it now?

@patrick-medusajs
Copy link
Contributor Author

What do you think are good next steps?

I would merge this feature as is. I would even argue that we could remove the two examples included with it.

Then, I would refactor the OAS for Store responses in one PR and do another PR for Admin responses.

It seems like adding this to all responses is a lot of work, but it might be best to get through it now?

It's not worst than editing content on a website. It might feel like tedious work but "this is the way". And yes, I would recommend tackling it now.

@olivermrbl
Copy link
Contributor

@patrick-medusajs Sounds like a solid plan 💪

@patrick-medusajs
Copy link
Contributor Author

@olivermrbl, anything missing before merging this one?

@kodiakhq kodiakhq bot merged commit 7b57695 into develop Mar 13, 2023
@kodiakhq kodiakhq bot deleted the feat/codegen-x-extended-relations branch March 13, 2023 14:22
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.

2 participants