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

Typescript definitions for operations #224

Conversation

oleksandrpravosudko-okta
Copy link
Contributor

@oleksandrpravosudko-okta oleksandrpravosudko-okta commented Feb 16, 2021

  • add handlebars templates for:
    • generated client
    • models
  • register TypeScript signature builders/formatters helpers
  • import all model types referenced by method in generated client and models types
  • add minimal handling of property names collisions and escaping property names

Generated model types are extracted to a separate branch: #225
TS lint setup is to follow in a subsequent PR.

Resolves: OKTA-291118

@oleksandrpravosudko-okta oleksandrpravosudko-okta force-pushed the op-okta-291118-operations-typescript-definitions branch from 59c916f to cb3980a Compare February 16, 2021 15:26
@oleksandrpravosudko-okta oleksandrpravosudko-okta marked this pull request as ready for review February 18, 2021 13:41
const getBodyModelNameInCamelCase = operation => {
const MODELS_SHOULD_NOT_PROCESS = ['object', 'string', 'undefined'];

// BEGIN work around spec mismatches and upstream parsing incosistencies
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: should be "inconsistencies", though I appreciate the irony

}
}
return typedArgs.join(', ');
}
Copy link
Contributor

@swiftone swiftone Feb 18, 2021

Choose a reason for hiding this comment

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

nit: missing semicolon

Several of the const below as well

const MODELS_SHOULD_NOT_PROCESS = ['object', 'string', 'undefined'];

// BEGIN work around spec mismatches and upstream parsing incosistencies
const RESTRICTED_MODEL_PROPERTY_OVERRIDES = {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is good work dealing with the exceptions, but I'm worried the next person to touch this may not follow, particularly since the templates say to set the property UNLESS it is an override (the opposite of what I'd expect an override to do).

Could you drop in a 1-2 line comment clarifying the logic here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure - I was hoping that could be read as 'add a property unless it's an illegal override', but that didn't work out so well 😅

@oleksandrpravosudko-okta oleksandrpravosudko-okta merged commit eba75e9 into op-ts-types-support Feb 24, 2021
@oleksandrpravosudko-okta oleksandrpravosudko-okta deleted the op-okta-291118-operations-typescript-definitions branch February 24, 2021 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants