feat(codegen): commit generated client types to codebase #3492
Conversation
🦋 Changeset detectedLatest commit: e70545e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| FILE_CHANGES=$(git status --porcelain=v1 | grep 'packages/generated') | ||
| FILE_CHANGES_COUNT=$(git status --porcelain=v1 | grep 'packages/generated' | wc -l) |
There was a problem hiding this comment.
The strategy to verify if the codegen build has been commited
https://git-scm.com/docs/git-status#Documentation/git-status.txt---porcelainltversiongt
https://www.baeldung.com/linux/bash-count-lines-in-file#wc
| - name: Build Packages - Force | ||
| run: yarn build --force --no-cache |
There was a problem hiding this comment.
Force build
This reverts commit 238650f.
| /* tslint:disable */ | ||
| /* eslint-disable */ | ||
|
|
||
| export * from "./models" |
There was a problem hiding this comment.
question: Would it make sense in the end if we have multiple export from here, to namespace them like export * as Models so that when you import a type you now what it reference e.g Models.Address. It would avoid any collision as well if it happen that we attribute the same name in a different place. wdyt? This would be more of a convention like I am suggesting for medusa/types
There was a problem hiding this comment.
One of the tenants for this initiative was to be a drop-in replacement of the current type imports from @medusajs/medusa in client code. I would not impose a namespace on end users. import * as Models can be used if need be.
There was a problem hiding this comment.
If we export something else than the models later, the user will not be able to do as Models as it would also includes the rest. I agree that it is a drop in replacement so let’s not take this comment into account for now 👍
| /* eslint-disable */ | ||
| import { SetRelation, Merge } from "../core/ModelUtils" | ||
|
|
||
| export interface AdminDeleteDiscountsDiscountConditionsConditionParams { |
There was a problem hiding this comment.
question: All those interface shouldn't extends FindParams instead? Maybe not since it is regenerated if we change something. wdyt?
There was a problem hiding this comment.
Sadly, FindParams does not exist in our OAS realm. Parameter definition in OAS does not have the same composition capabilities as TS. That is why we have a lot of repetition of fields and expand in our OAS. As an example.
In order to achieve bundling of query Params into a type, we had to customize the codegen lib by introducing our own x-codegen.queryParams OAS extension. You can find the code here.
What
Commit generated client types to codebase.
Why
As a developer, it will provides better visibility on the impact of OAS changes to the generated type. Also allow to browse the types on GitHub.
How
/libfrom .gitignoreyarn build --force --no-cacheon GitHub. Caching was creating false positives.git statusand filter the output to target onlypackages/generateddirectory.Test
Proof of a failing check:
https:/medusajs/medusa/actions/runs/4432323763/jobs/7776235128
UPDATE: Failing check after updating branch with latest develop
https:/medusajs/medusa/actions/runs/4436707954/jobs/7785472045