-
Notifications
You must be signed in to change notification settings - Fork 28
refactor(lockfile): consolidate TS and JSON schema type definitions #777
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
Conversation
Co-authored-by: Kanad Gupta <8854718+kanadgupta@users.noreply.github.com>
Co-authored-by: Kanad Gupta <8854718+kanadgupta@users.noreply.github.com>
Co-authored-by: Kanad Gupta <8854718+kanadgupta@users.noreply.github.com>
| } | ||
| } | ||
| }, | ||
| "additionalProperties": false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this caught my attention thanks to json-schema-to-ts 👑
| "$schema": { "type": "string" }, | ||
| "apis": { "type": "array", "description": "The list of installed APIs", "items": { "$ref": "#/definitions/api" } } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prettier 🤷🏽♂️
| "scripts": { | ||
| "attw": "attw --pack --format table-flipped", | ||
| "build": "tsc", | ||
| "build:versionedfiles": "NODE_OPTIONS=--no-warnings node --loader ts-node/esm bin/buildVersionedFiles.ts", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't love this file name nor its corresponding package.json script name, open to ideas here 😬
| export enum SupportedLanguages { | ||
| JS = 'js', | ||
| } | ||
| export const SupportedLanguages = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can't use TS enums here anymore since we're deriving our supported languages from the JSON schema
erunion
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some very small changes but otherwise this looks great.
Co-authored-by: Jon Ursenbach <erunion@users.noreply.github.com>
Co-Authored-By: Jon Ursenbach <jon@ursenba.ch>
Co-Authored-By: Jon Ursenbach <jon@ursenba.ch>
packages/api/src/commands/install.ts
Outdated
| new Option('-l, --lang <language>', 'SDK language').default(SupportedLanguages.JS).choices([SupportedLanguages.JS]), | ||
| ) | ||
| .addOption(new Option('-y, --yes', 'Automatically answer "yes" to any prompts printed')) | ||
| .action(async (uri: string, options: { identifier?: string; lang: string; yes?: boolean }) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| .action(async (uri: string, options: { identifier?: string; lang: string; yes?: boolean }) => { | |
| .action(async (uri: string, options: { identifier?: string; lang: SupportedLanguage; yes?: boolean }) => { |
packages/api/src/commands/install.ts
Outdated
| let language: SupportedLanguage; | ||
| if (options.lang) { | ||
| language = options.lang as SupportedLanguages; | ||
| language = options.lang as SupportedLanguage; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| language = options.lang as SupportedLanguage; | |
| language = options.lang; |
Co-Authored-By: Jon Ursenbach <jon@ursenba.ch>
🧰 Changes
This does a bit of quality-of-life work to consolidate our TS types and our JSON Schema definition for the
apilockfile schema. We now rebuild theschema.jsonfile every time we runnpm version, similar to how we do forpackageInfo.ts.🧬 QA & Testing
Does the
buildVersionedFiles.tswork as expected when you run it locally? If the TS build and all the tests pass, we should be in good shape.