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

Add typescript schema export utility #119

Merged
merged 4 commits into from
Jul 7, 2023
Merged

Conversation

foxymiles
Copy link
Contributor

Public-Facing Changes

None

Description

Add typescript schema export utility. This will be used in studio to import schema definitions in order to make them available for use in user scripts.

for (const schema of allSchemaNames) {
indexTS += `export * from "./${schema.name}";\n`;
}
schemas["index"] = indexTS;
Copy link
Member

Choose a reason for hiding this comment

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

I think this also needs to include TIME_TS and DURATION_TS.

Could we also use this new function here to reduce duplication?

@foxymiles foxymiles requested a review from jtbandes July 7, 2023 17:06
Copy link
Member

@jtbandes jtbandes left a comment

Choose a reason for hiding this comment

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

Left some final nits and suggestions but the main logic looks good to me. Thanks!

Comment on lines 4 to 9
/**
* Export typescript schema as source, keyed by the schema name.
*
* @returns a record of schema name => schema source.
*/
export function exportTypescriptSchema(): Record<string, string> {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/**
* Export typescript schema as source, keyed by the schema name.
*
* @returns a record of schema name => schema source.
*/
export function exportTypescriptSchema(): Record<string, string> {
/**
* Export typescript schemas as source, keyed by the file name.
*
* @returns a record of schema name => schema source.
*/
export function exportTypeScriptSchemas(): Record<string, string> {

I would go with "file name" because index is not really a "schema name". This also makes me wonder if we should add ".ts" suffix to the keys as well?

Maybe use a Map?

Copy link
Member

Choose a reason for hiding this comment

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

  • I think you'll need to export this from internal/index.ts
  • also would rename file to exportTypeScriptSchemas.ts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • I think you'll need to export this from internal/index.ts

👍

  • also would rename file to exportTypeScriptSchemas.ts

The other files in this directory use the singular so I stuck with that.

Copy link
Member

Choose a reason for hiding this comment

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

The other functions also tend to operate on just 1 schema at a time

@foxymiles foxymiles merged commit 5a6ab57 into main Jul 7, 2023
8 checks passed
@foxymiles foxymiles deleted the miles/export-ts-schemas branch July 7, 2023 18:32
@jtbandes
Copy link
Member

jtbandes commented Jul 7, 2023

@foxymiles "schemata" is a cute spelling...but the name of this repo is foxglove/schemas, so I think we should probably stick with that plural form 😬

image

jtbandes added a commit that referenced this pull request Jul 7, 2023
Follow-up from #119 

- Replace "schemata" with "schemas"
- Use "TypeScript" capitalization
- Export generateTypeScript stuff in case we need it later
- Return a Map instead of an object
- Clarified documentation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants