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

[RFC] - Annotation for nested properties #46

Closed
ms1111 opened this issue Jun 13, 2020 · 1 comment · Fixed by #49
Closed

[RFC] - Annotation for nested properties #46

ms1111 opened this issue Jun 13, 2020 · 1 comment · Fixed by #49
Assignees
Labels
documentation Improvements or additions to documentation enhancement New feature or request question Further information is requested

Comments

@ms1111
Copy link
Collaborator

ms1111 commented Jun 13, 2020

Annotation for nested properties

Here is the current syntax for deserializing a nested object.

    class Test1 extends Serializable<Test1> {
      @SerializeProperty("serialize_me_1")
      serializeMe = "nice1";
    }
    class Test2 extends Serializable<Test2> {
      @SerializeProperty({
        serializedKey: "serialize_me_2",
        fromJsonStrategy: (json) => new Test1().fromJson(json),
      })
      nested!: Test1;
    }
    const test = new Test2();

    test.fromJson(`{"serialize_me_2":{"serialize_me_1":"pass"}}`);

That line, fromJsonStrategy: (json) => new Test1().fromJson(json),: I bet this is going to be one of those "you don't know typescript" things, but it'd be cool to have a shortcut for that. Something like:

    class Test2 extends Serializable<Test2> {
      @SerializeProperty({
        serializedKey: "serialize_me_2",
        fromJsonStrategy: deserializeNestedProperty(Test1),
      })
      nested!: Test1;
    }

or even better:

    class Test2 extends Serializable<Test2> {
      @SerializeNestedProperty({ serializedKey: "serialize_me_2", type: Test1 })
      nested!: Test1;
    }

Motivation

Make model classes more readable when doing ser/deser of nested properties.

Design Detail

magnets

Drawbacks

Rationale and Alternatives

Prior Art

Unresolved questions

The fact that decorators don't automatically have access to the TS type is a big buzzkill.

@hardy925
Copy link
Contributor

hardy925 commented Jun 13, 2020

I can't believe you took the time to fill out most of this template! I put this template in to try it myself, @ChrisDufourMB was like "Dude, chill on the RFC template..." So we are going to revisit that too, but this is really appreciated.

I agree that the line fromJsonStrategy: (json) => new Test1().fromJson(json), is out of place, considering we have createDateStrategy() for dates. I am not really a fan of @SerializeNestedProperty({ serializedKey: "serialize_me_2", type: Test1 }) because it introduces a new decorator, one of our goals is to keep this as lite as possible with exported decorators, using strategies in place of them.

One of our earlier iterations had a type field in our serialize property options, we replaced it for the strategies. Strategies allow us to offload work or logic on the author, however, I think we can provide an exported function as you have in your first example.

   class Test2 extends Serializable<Test2> {
      @SerializeProperty({
        serializedKey: "serialize_me_2",
        fromJsonStrategy: deserializeNestedProperty(Test1),
      })
      nested!: Test1;
    }

What are your thoughts on calling it reviveAs<User>(User) I believe the exported function would be quite simple?

export function reviveAs<T>(type: Serializable<T>): Serializable<T> {
  return (value: T) => new type().fromJson(value);
}

All that said. Maybe all we need is to update the documentation because there is nothing stopping external authors from having their own strategies folder in their project.

How to build custom strategies

details on strategy factories


At a minimum, we should be updating our docs to be more clear.

@hardy925 hardy925 added documentation Improvements or additions to documentation enhancement New feature or request question Further information is requested labels Jun 13, 2020
@hardy925 hardy925 mentioned this issue Jun 24, 2020
3 tasks
@hardy925 hardy925 self-assigned this Jun 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants