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

Compile Time check for json decorator #93

Open
anyon17 opened this issue Oct 14, 2024 · 5 comments
Open

Compile Time check for json decorator #93

anyon17 opened this issue Oct 14, 2024 · 5 comments

Comments

@anyon17
Copy link

anyon17 commented Oct 14, 2024

Right now a class decorated with @json containing fields which might not have types whose declaration have @json decorator panics at the runtime. Is there any way to have this check at compile time so that compiler can exactly point to the field which does not have @json decorator in it's declaration.

@JairusSW
Copy link
Owner

JairusSW commented Oct 14, 2024

Here's an example for context
A class decorated with @json, but not all fields implement the JSON Type

@json
class Foo {
  public bar: string = "this is ok"
  public baz: i32 | null = 0
  // ^ this is not okay
}
Threw "unreachable" at runtime!
 at 0:30 ...

This will fail miserably and throw something like "index out of range" or "unreachable" which are highly non-descriptive. Rather:

@json
class Foo {
  public bar: string = "this is ok"
  public baz: i32 | null = 0
  // ^ this is not okay
}

ERROR: Property Foo.baz in <./src/index.ts> does not implement a JSON-compatible type!
Either decorate it with the `@omit` decorator or fix it!

Instead, we actually throw at COMPILE TIME to ensure that the user has fault tolerant code

@anyon17
Copy link
Author

anyon17 commented Oct 14, 2024

Yeah that's perfectly put @JairusSW . Thanks for attaching the example to make the issue much clear.

@JairusSW
Copy link
Owner

Reopening this as there are a few bugs

@JairusSW JairusSW reopened this Oct 21, 2024
@JairusSW
Copy link
Owner

@anyon17, after taking a closer look, it seems like to properly implement this (and a few other things), I'm going to have to take multiple passes over the AST via the transform. I'm not sure if your familiar with transforms, but its a pretty big undertaking, so I'll pull some features out and release a non-feature-complete version that's stable. This means I'll add full type checking in a later release.
I'm still going to be working on this, just thought I'd let you know it may take a bit longer.

@anyon17
Copy link
Author

anyon17 commented Oct 24, 2024

hey @JairusSW thanks for letting me know this detail. Will wait for the feature to become stable and looking forward to use it. Thanks again for picking up the requested feature.

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

No branches or pull requests

2 participants