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

added spectypes #856

Merged
merged 5 commits into from Apr 21, 2022
Merged

added spectypes #856

merged 5 commits into from Apr 21, 2022

Conversation

iyegoroff
Copy link
Contributor

@iyegoroff iyegoroff commented Apr 21, 2022

Hi @moltar!

Added spectypes

I have to update package-lock.json, because I was unable to access freajs.org for some reason.

Copy link
Owner

@moltar moltar left a comment

Choose a reason for hiding this comment

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

Clever approach! 👍🏼

Worried about adding the build steps. Not sure if this is far 😁

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@moltar moltar requested a review from hoeck April 21, 2022 11:52
@iyegoroff
Copy link
Contributor Author

Worried about adding the build steps. Not sure if this is far 😁

I decided to use additional build step here, because in future someone can create similar compiled validator library as a plugin for another compiler and this may result in a conflict when the same code will be processed by different compilers.

Copy link
Collaborator

@hoeck hoeck left a comment

Choose a reason for hiding this comment

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

Great Work!

I'm not a friend of babel and codegen (the only 'codegen' that I like are Lisp macros 😁) as I've often been bitten by ultra complex build system setups.

But the performance and code-size is indeed hard to beat with plain functional approaches. And its safe too (you do the extra key checks and pass all strict tests). Also the generated code looks sufficiently readable.

cases/spectypes.ts Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@iyegoroff
Copy link
Contributor Author

Great Work!

I'm not a friend of babel and codegen (the only 'codegen' that I like are Lisp macros 😁) as I've often been bitten by ultra complex build system setups.

But the performance and code-size is indeed hard to beat with plain functional approaches. And its safe too (you do the extra key checks and pass all strict tests). Also the generated code looks sufficiently readable.

Hi @hoeck ! Thanks for the feedback!

@moltar
Copy link
Owner

moltar commented Apr 21, 2022

Build failing

@moltar moltar closed this Apr 21, 2022
@moltar
Copy link
Owner

moltar commented Apr 21, 2022

> typescript-runtime-type-benchmarks@1.0.0 test:build /home/runner/work/typescript-runtime-type-benchmarks/typescript-runtime-type-benchmarks
> tsc --noEmit

Error: cases/spectypes/index.ts(1,53): error TS2307: Cannot find module './build' or its corresponding type declarations.
npm ERR! code ELIFECYCLE
npm ERR! errno 2
npm ERR! typescript-runtime-type-benchmarks@1.0.0 test:build: `tsc --noEmit`
npm ERR! Exit status 2
npm ERR! 
npm ERR! Failed at the typescript-runtime-type-benchmarks@1.0.0 test:build script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

@moltar moltar reopened this Apr 21, 2022
@moltar
Copy link
Owner

moltar commented Apr 21, 2022

oopsy, closed by accident

@iyegoroff
Copy link
Contributor Author

Should be fixed now

package.json Outdated Show resolved Hide resolved
@moltar moltar merged commit 9910372 into moltar:master Apr 21, 2022
@moltar
Copy link
Owner

moltar commented Apr 21, 2022

@iyegoroff impressive performance! A new record! 🥇

But what we did forget in this PR is the README mention, under "Packages Compared" heading.

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