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

feat: add aeria #1218

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

feat: add aeria #1218

wants to merge 10 commits into from

Conversation

minenwerfer
Copy link

This PR adds the core validation module of Aeria, a TypeScript web framework and source of truth.
It validates objects against JSON schemas and it complains with benchmark requirements.

Repo: https://github.com/aeria-org/aeria

package.json Outdated Show resolved Hide resolved
docs/results/node-20.json Outdated Show resolved Hide resolved
cases/deepkit/build/index.d.ts Outdated Show resolved Hide resolved
@moltar
Copy link
Owner

moltar commented Mar 5, 2024

Please run npm run lint

@minenwerfer
Copy link
Author

Sorry I still have to work on this PR.
I didn't notice the lib wasn't passing some tests.

@minenwerfer
Copy link
Author

minenwerfer commented Mar 5, 2024

@moltar could you review my PR now?
Deepkit files were affected by some script, I'm not sure which one. I reverted the changes and I believe it's all okay now.

docs/results/node-14.json Outdated Show resolved Hide resolved
@moltar
Copy link
Owner

moltar commented Mar 6, 2024

Please add the library to the README as well.

And it looks like the lint has failed.

Warning:   69:25  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any

/home/runner/work/typescript-runtime-type-benchmarks/typescript-runtime-type-benchmarks/cases/aeria.ts
Warning:    55:10  warning  '_' is assigned a value but never used                 @typescript-eslint/no-unused-vars
Warning:    61:17  warning  Unexpected any. Specify a different type               @typescript-eslint/no-explicit-any
Error:    63:7   error    Replace `(·!result·` with `·(!result`                  prettier/prettier
Error:    64:24  error    Insert `;`                                             prettier/prettier
Error:    67:18  error    Insert `;`                                             prettier/prettier
Warning:    72:10  warning  '_' is assigned a value but never used                 @typescript-eslint/no-unused-vars
Warning:    76:17  warning  Unexpected any. Specify a different type               @typescript-eslint/no-explicit-any
Error:    78:7   error    Replace `(·!result·` with `·(!result`                  prettier/prettier
Error:    79:24  error    Insert `;`                                             prettier/prettier
Error:    82:18  error    Insert `;`                                             prettier/prettier
Warning:    87:10  warning  '_' is assigned a value but never used                 @typescript-eslint/no-unused-vars
Warning:    91:17  warning  Unexpected any. Specify a different type               @typescript-eslint/no-explicit-any
Error:    92:7   error    Replace `(·!validate(data)·` with `·(!validate(data)`  prettier/prettier
Error:    93:24  error    Insert `;`                                             prettier/prettier
Error:    96:16  error    Insert `;`                                             prettier/prettier
Warning:   101:10  warning  '_' is assigned a value but never used                 @typescript-eslint/no-unused-vars
Warning:   103:17  warning  Unexpected any. Specify a different type               @typescript-eslint/no-explicit-any
Error:   104:7   error    Replace `(·!validate(data)·` with `·(!validate(data)`  prettier/prettier
Error:   105:24  error    Insert `;`                                             prettier/prettier
Error:   108:16  error    Insert `;`                                             prettier/prettier

@moltar moltar requested a review from hoeck March 6, 2024 15:02
@minenwerfer
Copy link
Author

Please add the library to the README as well.

And it looks like the lint has failed.

Warning:   69:25  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any

/home/runner/work/typescript-runtime-type-benchmarks/typescript-runtime-type-benchmarks/cases/aeria.ts
Warning:    55:10  warning  '_' is assigned a value but never used                 @typescript-eslint/no-unused-vars
Warning:    61:17  warning  Unexpected any. Specify a different type               @typescript-eslint/no-explicit-any
Error:    63:7   error    Replace `(·!result·` with `·(!result`                  prettier/prettier
Error:    64:24  error    Insert `;`                                             prettier/prettier
Error:    67:18  error    Insert `;`                                             prettier/prettier
Warning:    72:10  warning  '_' is assigned a value but never used                 @typescript-eslint/no-unused-vars
Warning:    76:17  warning  Unexpected any. Specify a different type               @typescript-eslint/no-explicit-any
Error:    78:7   error    Replace `(·!result·` with `·(!result`                  prettier/prettier
Error:    79:24  error    Insert `;`                                             prettier/prettier
Error:    82:18  error    Insert `;`                                             prettier/prettier
Warning:    87:10  warning  '_' is assigned a value but never used                 @typescript-eslint/no-unused-vars
Warning:    91:17  warning  Unexpected any. Specify a different type               @typescript-eslint/no-explicit-any
Error:    92:7   error    Replace `(·!validate(data)·` with `·(!validate(data)`  prettier/prettier
Error:    93:24  error    Insert `;`                                             prettier/prettier
Error:    96:16  error    Insert `;`                                             prettier/prettier
Warning:   101:10  warning  '_' is assigned a value but never used                 @typescript-eslint/no-unused-vars
Warning:   103:17  warning  Unexpected any. Specify a different type               @typescript-eslint/no-explicit-any
Error:   104:7   error    Replace `(·!validate(data)·` with `·(!validate(data)`  prettier/prettier
Error:   105:24  error    Insert `;`                                             prettier/prettier
Error:   108:16  error    Insert `;`                                             prettier/prettier

I just fixed those problems in my last push.

@minenwerfer
Copy link
Author

The CI build has failed, I'm on it now.

@minenwerfer
Copy link
Author

I had to switch to a dynamic require() because tsc --noEmit was reaching node_modules/@aeriajs/**/*.d.ts and failing probably because the lib uses a newer Typescript version. Alternatively skipLibCheck could be used to ignore files under node_modules.
Lint, build, and test are all okay now.

@minenwerfer minenwerfer requested a review from moltar March 6, 2024 17:27
@moltar
Copy link
Owner

moltar commented Mar 6, 2024

@hoeck Wanna take a look here?

@hoeck
Copy link
Collaborator

hoeck commented Mar 7, 2024

@hoeck Wanna take a look here?

Yeah will do, just need some time. Tomorrow is a developer conference in my hometown and I need to prepare a lightning talk about runtypes and the benchmark 😁

@moltar
Copy link
Owner

moltar commented Mar 7, 2024

@hoeck Good luck on your talk!

Please share your talk, if you feel comfortable. Maybe even link it in the README in some resource section? Would love to see it!

@minenwerfer
Copy link
Author

minenwerfer commented Mar 9, 2024

Hey! In this meantime I updated the dependency version because the old one was by mistake polluting the dependency tree with unnecessary subdependencies, making CI slower.

@hoeck
Copy link
Collaborator

hoeck commented Mar 10, 2024

The lightning talk (8mins) went great 😅 even though I was nervous as hell. From the feedback, seemed I was able to entertain some people. Slides with notes are here: https://hoeck.github.io/lightning-talk-runtypes/presenter/1 - please let me know @moltar if those are useful enough to link to the readme.

I had to switch to a dynamic require() because tsc --noEmit was reaching node_modules/@aeriajs/**/*.d.ts and failing probably because the lib uses a newer Typescript version. Alternatively skipLibCheck could be used to ignore files under node_modules.

I'd say we should enable skipLibCheck: true to get aeria working right now. That seems a better tradeoff that using require instead of import to skip the type checking for this one library.

@moltar The reason the repo is stuck at Typescript 5.1 seems to be the following to me:

  1. dependabot tries to update to the latest TS version (e.g. v5.4.2)
  2. some libs (e.g. typia, deepkit) do not yet support that new version, upgrade fails
  3. later those libs are upgraded and now do support the new TS version, but dependabot does not try to upgrade the same TS again, starting again at 1. for a new TS version

From my perspective, we could:

  1. manually upgrade TS to newer (not the very latest) versions
  2. somehow isolate each package to have its own package.json and node_modules so we are able to run each package with its own dependencies and typescript-version

Wasn't there an issue for 2. already? Couldn't find it though but I'm sure you mentioned some kind of isolation some time ago.
Found it: #864

@minenwerfer Which version of TS do you need for aeria (couldn't find anything declared in your package.json)? Could you try to upgrade the benchmark to use e.g. TS 5.4 and check if everything works (without skiplibcheck) as part of this PR? Thanks.

@minenwerfer
Copy link
Author

minenwerfer commented Mar 10, 2024

The lightning talk (8mins) went great 😅 even though I was nervous as hell. From the feedback, seemed I was able to entertain some people. Slides with notes are here: https://hoeck.github.io/lightning-talk-runtypes/presenter/1 - please let me know @moltar if those are useful enough to link to the readme.

I had to switch to a dynamic require() because tsc --noEmit was reaching node_modules/@aeriajs/**/*.d.ts and failing probably because the lib uses a newer Typescript version. Alternatively skipLibCheck could be used to ignore files under node_modules.

I'd say we should enable skipLibCheck: true to get aeria working right now. That seems a better tradeoff that using require instead of import to skip the type checking for this one library.

@moltar The reason the repo is stuck at Typescript 5.1 seems to be the following to me:

  1. dependabot tries to update to the latest TS version (e.g. v5.4.2)
  2. some libs (e.g. typia, deepkit) do not yet support that new version, upgrade fails
  3. later those libs are upgraded and now do support the new TS version, but dependabot does not try to upgrade the same TS again, starting again at 1. for a new TS version

From my perspective, we could:

  1. manually upgrade TS to newer (not the very latest) versions
  2. somehow isolate each package to have its own package.json and node_modules so we are able to run each package with its own dependencies and typescript-version

Wasn't there an issue for 2. already? Couldn't find it though but I'm sure you mentioned some kind of isolation some time ago. Found it: #864

@minenwerfer Which version of TS do you need for aeria (couldn't find anything declared in your package.json)? Could you try to upgrade the benchmark to use e.g. TS 5.4 and check if everything works (without skiplibcheck) as part of this PR? Thanks.

I also had to bump the typia package because it uses TS in the runtime and the older version didn't support version 5.4. I also had to change the extends in tsconfig.json to an absolute path for some reason, but then everything worked fine, including aeria. So I think we have 3 options:

  1. Bump Typescript to 5.4 (and typia to its latest version)
  2. Incrementally create separated workspaces for cases, starting with aeria
  3. Assume dependencies were compiled right and leave skipLibCheck in the tsconfig.json

Please let me know if you want me to commit anything.
PS: great presentation!

@minenwerfer
Copy link
Author

@moltar @hoeck please check if those changes are acceptable.

@@ -1,5 +1,5 @@
{
"extends": "tsconfigs/nodejs-module",
"extends": "./node_modules/tsconfigs/nodejs-module.json",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just curious, what does this change? did the old stop working in the new ts version?

Copy link
Author

Choose a reason for hiding this comment

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

Yes. tsc would still run fine but npm run test would fail with:

TSError: ⨯ Unable to compile TypeScript:
error TS6053: File 'tsconfigs/nodejs-module' not found.

A bit weird indeed.

@hoeck
Copy link
Collaborator

hoeck commented Mar 14, 2024

Looks good to me @moltar what do you think?

@minenwerfer unfortunately the linter does not work any more, could you please check that or maybe update the linter too? Thx or let me know if you need help / have no time.

@moltar
Copy link
Owner

moltar commented Mar 14, 2024

I updated gts (linter) in #1221, please see if this fixes the problem.

@minenwerfer
Copy link
Author

minenwerfer commented Mar 14, 2024

@moltar I fixed the build errors concerning Aeria, but how should we proceed about the errors in the other 2 libs?

node_modules/@sinclair/typebox/typebox.d.ts:203:137 - error TS2589: Type instantiation is excessively deep and possibly infinite.

203 export type TIndex<T extends TSchema, K extends TPropertyKey[]> = T extends TRecursive<infer S> ? TIndex<S, K> : T extends TIntersect ? UnionType<Flat<TIndexRestMany<T, K>>> : T extends TUnion ? UnionType<Flat<TIndexRestMany<T, K>>> : T extends TObject ? UnionType<Flat<TIndexRestMany<T, K>>> : T extends TTuple ? UnionType<Flat<TIndexRestMany<T, K>>> : TNever;
                                                                                                                                            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

node_modules/@sinclair/typebox/typebox.d.ts:203:196 - error TS2589: Type instantiation is excessively deep and possibly infinite.

203 export type TIndex<T extends TSchema, K extends TPropertyKey[]> = T extends TRecursive<infer S> ? TIndex<S, K> : T extends TIntersect ? UnionType<Flat<TIndexRestMany<T, K>>> : T extends TUnion ? UnionType<Flat<TIndexRestMany<T, K>>> : T extends TObject ? UnionType<Flat<TIndexRestMany<T, K>>> : T extends TTuple ? UnionType<Flat<TIndexRestMany<T, K>>> : TNever;
                                                                                                                                                                                                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

node_modules/@sinclair/typebox/typebox.d.ts:203:256 - error TS2589: Type instantiation is excessively deep and possibly infinite.

203 export type TIndex<T extends TSchema, K extends TPropertyKey[]> = T extends TRecursive<infer S> ? TIndex<S, K> : T extends TIntersect ? UnionType<Flat<TIndexRestMany<T, K>>> : T extends TUnion ? UnionType<Flat<TIndexRestMany<T, K>>> : T extends TObject ? UnionType<Flat<TIndexRestMany<T, K>>> : T extends TTuple ? UnionType<Flat<TIndexRestMany<T, K>>> : TNever;
                                                                                                                                                                                                                                                                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

node_modules/@sinclair/typebox/typebox.d.ts:203:315 - error TS2589: Type instantiation is excessively deep and possibly infinite.

203 export type TIndex<T extends TSchema, K extends TPropertyKey[]> = T extends TRecursive<infer S> ? TIndex<S, K> : T extends TIntersect ? UnionType<Flat<TIndexRestMany<T, K>>> : T extends TUnion ? UnionType<Flat<TIndexRestMany<T, K>>> : T extends TObject ? UnionType<Flat<TIndexRestMany<T, K>>> : T extends TTuple ? UnionType<Flat<TIndexRestMany<T, K>>> : TNever;
                                                                                                                                                                                                                                                                                                                              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

node_modules/@sinclair/typebox/typebox.d.ts:205:235 - error TS2344: Type 'AssertRest<R, TSchema[]>' does not satisfy the constraint 'TTemplateLiteralKind[]'.
  Type '[] | (TSchema[] & unknown[])' is not assignable to type 'TTemplateLiteralKind[]'.
    Type 'TSchema[] & unknown[]' is not assignable to type 'TTemplateLiteralKind[]'.
      The types returned by 'pop()' are incompatible between these types.
        Type 'TSchema | undefined' is not assignable to type 'TTemplateLiteralKind | undefined'.

205 export type TIntrinsicTemplateLiteral<T extends TTemplateLiteralKind[], M extends TIntrinsicMode> = M extends ('Lowercase' | 'Uppercase') ? T extends [infer L, ...infer R] ? [TIntrinsic<AssertType<L>, M>, ...TIntrinsicTemplateLiteral<AssertRest<R>, M>] : T : M extends ('Capitalize' | 'Uncapitalize') ? T extends [infer L, ...infer R] ? [TIntrinsic<AssertType<L>, M>, ...R] : T : T;
                                                                                                                                                                                                                                              ~~~~~~~~~~~~~

node_modules/myzod/libs/types.d.ts:49:146 - error TS2589: Type instantiation is excessively deep and possibly infinite.

49 type ArrayIntersection<A1 extends ArrayType<any>, A2 extends ArrayType<any>> = A1 extends ArrayType<infer S1> ? A2 extends ArrayType<infer S2> ? ArrayType<IntersectionResult<S1, S2>> : never : never;
                                                                                                                                                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Found 6 errors in 2 files.

Errors  Files
     5  node_modules/@sinclair/typebox/typebox.d.ts:203
       1  node_modules/myzod/libs/types.d.ts

This must have been introduced by TS 5.4 and I didn't notice it, sorry.

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