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): required devDeps are now listed in package.json #761

Merged
merged 3 commits into from
Oct 16, 2023

Conversation

erunion
Copy link
Member

@erunion erunion commented Oct 16, 2023

🧰 Changes

Per feedback in #754 this slightly refactors the package.json file we generate for SDKs to now list the dev-only dependencies that are required to build an SDK.

I've also added in a prepare script to these for rebuilding the compiled SDK, as well as a files array for when/if a codegen'd SDK is published to NPM.

@erunion erunion added enhancement New feature or request area:core Issues related to `core`, which is the package that powers the SDKs at runtime labels Oct 16, 2023
@erunion erunion changed the title refactor(codegen): required devDeps are now listed in package.json fear(codegen): required devDeps are now listed in package.json Oct 16, 2023
@erunion erunion changed the title fear(codegen): required devDeps are now listed in package.json refactor(codegen): required devDeps are now listed in package.json Oct 16, 2023
@erunion erunion requested a review from kanadgupta October 16, 2023 22:08
@erunion erunion marked this pull request as ready for review October 16, 2023 22:08
@erunion erunion changed the title refactor(codegen): required devDeps are now listed in package.json feat(codegen): required devDeps are now listed in package.json Oct 16, 2023
@erunion erunion force-pushed the refactor/codegen-dependencies branch from 5971aa0 to 540d869 Compare October 16, 2023 22:08
reason: string;
url: string;
url?: string;
Copy link
Member Author

Choose a reason for hiding this comment

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

Made url optional because it feels silly to write out https://typescriptlang.org/ as the URL for TS.

@@ -28,8 +28,9 @@ export default abstract class CodeGenerator {
requiredPackages!: Record<
string,
{
dependencyType: 'production' | 'development';
Copy link
Member Author

@erunion erunion Oct 16, 2023

Choose a reason for hiding this comment

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

Made this as dependencyType instead of devDependency: boolean so it's not specific to NPM and Node.

Copy link
Member

Choose a reason for hiding this comment

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

yeah that's a great call

@@ -28,8 +28,9 @@ export default abstract class CodeGenerator {
requiredPackages!: Record<
string,
{
dependencyType: 'production' | 'development';
Copy link
Member

Choose a reason for hiding this comment

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

yeah that's a great call

typescript: {
dependencyType: 'development',
reason: 'Required for `tsup`.',
version: '^5.2.2',
Copy link
Member

Choose a reason for hiding this comment

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

if and when we improve the codegen for our test fixtures (either via a build script or something), it'd be cool if we can dynamically pull in the versions from our dependencies

Copy link
Member Author

Choose a reason for hiding this comment

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

100%

@erunion erunion added this to the v7 milestone Oct 16, 2023
Base automatically changed from refactor/codegen-oas-dependency to main October 16, 2023 22:24
@erunion erunion merged commit a217ac5 into main Oct 16, 2023
5 checks passed
@erunion erunion deleted the refactor/codegen-dependencies branch October 16, 2023 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:core Issues related to `core`, which is the package that powers the SDKs at runtime enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants