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

[WIP] Replace with json-schema-to-ts package #4

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lcsmuller
Copy link
Member

@lcsmuller lcsmuller commented May 5, 2023

Replace '@profusion/json-schema-to-typescript-definitions' with 'json-schema-to-ts' package

Description

  • Replaces @profusion/json-schema-to-typescript package with json-schema-to-ts

Related Issues

Closes #2

Progress

  • Fix slow recursive typing
  • Replace package

Pull request checklist

  • Tests: This PR includes tests for covering the features or bug fixes (if applicable).
  • Docs: This PR updates/creates the necessary documentation.
  • CI: Make sure your Pull Request passes all CI checks. If not, clarify the motif behind that and the action plan to solve it (may reference a ticket)

How to test it

Visual reference

@lcsmuller lcsmuller self-assigned this May 5, 2023
@lcsmuller lcsmuller force-pushed the refactor/replace-json-schema-lib branch 2 times, most recently from 3a3b6f9 to 2d62d92 Compare May 5, 2023 19:16
@lcsmuller lcsmuller force-pushed the chore/update-deps branch 2 times, most recently from cb0bb34 to 902b01c Compare May 8, 2023 18:53
@lcsmuller lcsmuller force-pushed the refactor/replace-json-schema-lib branch 4 times, most recently from 42ed0cd to c3c92d1 Compare May 9, 2023 12:33
@lcsmuller lcsmuller force-pushed the chore/update-deps branch from 902b01c to bc03b21 Compare May 10, 2023 12:25
@lcsmuller lcsmuller force-pushed the refactor/replace-json-schema-lib branch 6 times, most recently from b5fad7c to a3d0ea9 Compare May 16, 2023 19:26
lib/types.ts Outdated
*
* There is no need to coerce types as Ajv.validate() will do that
* for you.
*/
export type EnvSchemaParserFn<
S extends BaseEnvSchema,
K extends keyof S['properties'],
V extends BaseEnvParsed<S> = TypeFromJSONSchema<S>,

Choose a reason for hiding this comment

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

here for example, if we always use TypeFromJSONSchema<S> anyway, we could simply

export type EnvSchemaParserFn<
  S extends BaseEnvSchema,
> = (
  str: string,
  propertySchema: Readonly<S['properties'][KeyOf<S['properties']>]>,
  key: KeyOf<S['properties']>,
  schema: Readonly<S>,
) => EnvSchemaPropertyValue<S, TypeFromJSONSchema<S>>;

But if you do this on EnvSchemaPropertyValue, you won't even need to do this

lib/types.ts Outdated
Comment on lines 41 to 45
export type EnvSchemaPropertyValue<
S extends BaseEnvSchema,
K extends keyof S['properties'],
> = K extends keyof TypeFromJSONSchema<S> ? TypeFromJSONSchema<S>[K] : never;
V extends BaseEnvParsed<S>,
> = V[KeyOf<V>];

Choose a reason for hiding this comment

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

Ultimately, you would simply do

export type EnvSchemaPropertyValue<
  S extends BaseEnvSchema,
> = TypeFromJSONSchema<S>[KeyOf<TypeFromJSONSchema<S>>];

If that's possible, or makes sense. Well, if in fact I miss something and there is some case where you don't use the default value =TypeFromJSONSchema<S>, then this is necessary, but I haven't found

Copy link
Member Author

Choose a reason for hiding this comment

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

What I'm trying to avoid here is the previous behavior of repeatedly calling TypeFromJSONSchema<>, processing the same conversion over and over. Which is what made this codebase so slow to work with..

@lcsmuller lcsmuller force-pushed the chore/update-deps branch 4 times, most recently from 2829f8f to e25930f Compare May 18, 2023 19:14
@lcsmuller lcsmuller force-pushed the refactor/replace-json-schema-lib branch 2 times, most recently from 1d86412 to 6ec7263 Compare May 18, 2023 19:23
@lcsmuller lcsmuller force-pushed the chore/update-deps branch 2 times, most recently from 199ce0d to a9f3ffa Compare May 22, 2023 19:27
@lcsmuller lcsmuller force-pushed the refactor/replace-json-schema-lib branch 2 times, most recently from 784ca2d to b304594 Compare May 22, 2023 21:45
@lcsmuller lcsmuller force-pushed the chore/update-deps branch from a9f3ffa to f8aa8ae Compare May 23, 2023 11:04
@lcsmuller lcsmuller force-pushed the refactor/replace-json-schema-lib branch from b304594 to c4690dd Compare May 23, 2023 11:05
@lcsmuller lcsmuller force-pushed the chore/update-deps branch from f8aa8ae to acef2f8 Compare May 24, 2023 17:30
@lcsmuller lcsmuller force-pushed the refactor/replace-json-schema-lib branch from c4690dd to 4a934a2 Compare May 24, 2023 17:42
@lcsmuller lcsmuller mentioned this pull request May 24, 2023
Base automatically changed from chore/update-deps to master May 25, 2023 11:21
@lcsmuller lcsmuller force-pushed the refactor/replace-json-schema-lib branch 4 times, most recently from 26427da to fb0f884 Compare May 25, 2023 21:38
- Remove redundant type recursions
- Improve typing readability
- Avoid casting

Part of #2
@lcsmuller lcsmuller force-pushed the refactor/replace-json-schema-lib branch from fb0f884 to 46da610 Compare May 25, 2023 21:42
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.

Replace @profusion/json-schema-to-typescript-definitions with json-schema-to-ts package
2 participants