PhilanthropyDataCommons / sdk

Instructions and supporting files for SDK generation
GNU Affero General Public License v3.0
1 stars 0 forks source link

Create the Typescript templates #7

Closed slifty closed 9 months ago

slifty commented 10 months ago

This PR implements an initial TypeScript sdk generator which provides type definitions based on a swagger specification.

The PR does NOT do the following:

  1. Allow package versioning in the resulting SDK.
  2. Allow other templated fields (e.g. project name, package name) in the resulting SDK.
  3. Provide type guards for the type definitions.
  4. Provide any sort of automated testing to ensure the generated code actually functions.
  5. Provide any sort of automated testing to ensure the generator actually works.

I would like to add these enhancements in future PRs instead of trying to do it all here.


To test, you will need a local java runtime environment and local node / nvm. From there you can:

  1. Follow the README instructions to set up / invoke the generation
  2. Once the generation is complete, you should see a typescript project in build/typescript. CD to there.
  3. Invoke the build (npm run build)
  4. Once the build is complete you should have a working compiled project in build/typescript/dist which you could in theory place in a node_modules and use as type definitions!

Resolves #2

reefdog commented 9 months ago

@slifty When I try to follow the instructions, I get all the way to npm run build (note: I needed to npm i before that), then I get an error about a missing tsconfig.json:

➜  typescript git:(2-create-typescript-templates) npm run build

> @pdc/sdk@0.0.0 build
> npm run cleanup && npm run build:node

> @pdc/sdk@0.0.0 cleanup
> rimraf dist

> @pdc/sdk@0.0.0 build:node
> tsc -p tsconfig.json

error TS5058: The specified path does not exist: 'tsconfig.json'.

And the dist/ directory is not created.

Is this known/expected? How should I proceed?

slifty commented 9 months ago

@reefdog ah HA -- that file was missed -- hooray for code reviews!

I'll fix and re-run it all tomorrow morning and ping you when ready for a new review

slifty commented 9 months ago

@reefdog fixed up and ready for re-review!

reefdog commented 9 months ago

Okay, after ./gradlew clean and regenerating, we got farther! But now I'm seeing:

➜  typescript git:(2-create-typescript-templates) npm run build

> @pdc/sdk@0.0.0 build
> npm run cleanup && npm run build:node

> @pdc/sdk@0.0.0 cleanup
> rimraf dist

> @pdc/sdk@0.0.0 build:node
> tsc -p tsconfig.json

src/types/BulkUpload.ts:8:20 - error TS2702: 'BulkUpload' only refers to a type, but is being used as a namespace here.

8  readonly status?: BulkUpload.StatusEnum;
                     ~~~~~~~~~~

src/types/BulkUpload.ts:8:20 - error TS4033: Property 'status' of exported interface has or is using private name 'BulkUpload'.

8  readonly status?: BulkUpload.StatusEnum;
                     ~~~~~~~~~~

Found 2 errors in the same file, starting at: src/types/BulkUpload.ts:8
slifty commented 9 months ago

So that's fun!

I'm using an older swagger spec for our API that didn't have enums; the generators handle enums differently somehow, so I'll need to improve the templates.

This raises a meta concept though: as we update our swagger spec we may well accidentally break the generators. Ideally there'd be a way to test them / test the resulting code / etc.

slifty commented 9 months ago

Tapped into the enum support.

The generated enum is not exactly how I would want to create it, but the effort in changing their naming convention is... large and definitely not worth it:


export namespace BulkUpload {
    export enum StatusEnum {
    Pending = 'pending',
    InProgress = 'in_progress',
    Completed = 'completed',
    Failed = 'failed',
    Canceled = 'canceled'
  }
}

So accessing these enums would involve BulkUpload.StatusEnum.Pending

Had we defined this manually I'd have preferred something like Statuses.PENDING

Anyway, @reefdog I think this is ready for you once more!

reefdog commented 9 months ago

@slifty I don't see any new commits (or force-pushes) since the last review, and I'm getting the same results as before. Did you forget to push some changes, or do I need to do something new in my testing, or…?

slifty commented 9 months ago

@reefdog ugh I guess I should push it so you can actually view it (sorry about that)

slifty commented 9 months ago

@reefdog I hear you, though to some extent I don't know if that's possible to fit in a review (the templates are a UX and we'll iterate on them, but we won't be able to really know what enhancements are needed until after getting user feedback, including our own user feedback).

The best we can do is review the proposed interfaces that are generated!