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

fix: resolving typing issues in JS codegen #593

Merged
merged 4 commits into from
Dec 9, 2022
Merged

Conversation

erunion
Copy link
Member

@erunion erunion commented Dec 9, 2022

🚥 Fixes #588

🧰 Changes

  • Update codegen for CJS exports to have the main createSDK export set as export = in the .d.ts file. Having it as export default createSDK was causing issues where the typing was expecting you to to do sdk.default.someOperation even though the way you load the SDK doesn't require you to have to destructure it off default.
  • This also updates JS type generation to no longer export types in the main SDK entry point as it's causing issues with the affformentioned default export issue and also JS users can't import these types anyways. They'll still have access to them by way of TS intellisense but if you want to import these types you'll obviously need to use TS codegen or load them from types.d.ts. And yes this does work even though the file has a .d.ts extension:

Screen Shot 2022-12-09 at 1 01 46 PM

🧬 QA & Testing

This is what using a CJS SDK looks like now:

Screen Shot 2022-12-09 at 1 03 55 PM

What it looked like before (with export default createSDK in the index.d.ts file):

Screen Shot 2022-12-09 at 1 04 33 PM

@erunion erunion added bug Something isn't working area:core Issues related to `core`, which is the package that powers the SDKs at runtime labels Dec 9, 2022
@erunion erunion changed the title fix: stop exporting types in the main entrypoint for JS projects fix: resolving typing issues in JS codegen Dec 9, 2022
@erunion erunion marked this pull request as ready for review December 9, 2022 21:06
@erunion
Copy link
Member Author

erunion commented Dec 9, 2022

Ignore the smoketest failures, the dataset from https://api.apis.guru seems to delete specs when they get version bumps and we run with a local copy of https://api.apis.guru/v2/list.json

Copy link
Member

@kanadgupta kanadgupta left a comment

Choose a reason for hiding this comment

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

other than the small comment suggestions, LGTM!

packages/api/src/cli/codegen/languages/typescript.ts Outdated Show resolved Hide resolved
Co-authored-by: Kanad Gupta <8854718+kanadgupta@users.noreply.github.com>
@erunion erunion merged commit ab97a70 into main Dec 9, 2022
@erunion erunion deleted the fix/cjs-typing-issues branch December 9, 2022 21:42
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 bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CJS/JS intellisense suggestions doesn't align with actual types
2 participants