Skip to content

Conversation

@erunion
Copy link
Member

@erunion erunion commented Oct 19, 2023

🧰 Changes

This is a big changeset but the only changes that happened here are the following:

  • Updated the TS codegen test suite to allow us to ad-hoc regenerate failing SDK fixtures by running vitest with a UPDATE_FIXTURES=1 environment variable.
  • Fixed a quirk where in a couple cases a line-terminating semicolon would be annoyingly be placed on the following line.
  • Generated JSON Schemas now look a lot nicer thanks to ts-morph's formatting API.1
    • I have not noticed any adverse impacts to codegen performance by enabling this.

Footnotes

  1. https://ts-morph.com/manipulation/formatting

@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 19, 2023
Comment on lines -238 to -246
// Because we're returning the raw source files for TS generation we also need to separately
// emit out our declaration files so we can put those into a separate file in the installed
// SDK directory.
...this.project
.emitToMemory({ emitOnlyDtsFiles: true })
.getFiles()
.map(sourceFile => ({
[path.basename(sourceFile.filePath)]: sourceFile.text,
})),
Copy link
Member Author

Choose a reason for hiding this comment

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

This is no longer used and is safe to be removed. It was a remaining artifact of the previous CJS and (attempted) ESM export work where we were generating .d.ts files ourselves.

initializer: writer => {
// `ts-morph` doesn't have any way to cleanly create an IIFE.
writer.writeLine('(() => { return new SDK(); })()');
writer.write('(() => { return new SDK(); })()');
Copy link
Member Author

@erunion erunion Oct 19, 2023

Choose a reason for hiding this comment

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

Changing this from .writeLine() to .write() fixes a quirk where the ending ; would get placed on the following line because .writeLine() terminates the thing we're writing with a \n.

const createSDK = (() => { return new SDK(); })()
;

and now

const createSDK = (() => { return new SDK(); })();

it's the little things

str = str.replace(REF_PLACEHOLDER_REGEX, '$1');

writer.writeLine(`${str} as const`);
writer.write(`${str} as const`);
Copy link
Member Author

@erunion erunion Oct 19, 2023

Choose a reason for hiding this comment

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

Same as the other semicolon fix. These schemas were getting generated like the following:

const schema = {"type": "string"} as const
;

They're now this:

const schema = {"type": "string"} as const;

@erunion erunion marked this pull request as ready for review October 19, 2023 07:14
@erunion erunion requested a review from kanadgupta October 19, 2023 07:14
@erunion erunion added this to the v7 milestone Oct 19, 2023
*
* @see {@link https://ts-morph.com/manipulation/formatting}
*/
sourceFile.formatText();
Copy link
Contributor

Choose a reason for hiding this comment

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

nice, TIL! kinda curious about the performance of this vs prettier vs others (but don't think we need to investigate that any time soon)

Co-authored-by: Kanad Gupta <8854718+kanadgupta@users.noreply.github.com>
@erunion erunion merged commit 046b029 into main Oct 19, 2023
@erunion erunion deleted the feat/cleaner-sdk-schemas branch October 19, 2023 14:40
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.

3 participants