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 deser class by schema collection userfriendlyapi #377

Conversation

fospring
Copy link
Contributor

Pre-flight checklist

  • I have read the Contributing Guidelines on pull requests.
  • Commit messages follow the conventional commits spec
  • If this is a code change: I have written unit tests.
  • If this is a new API or substantial change: the PR has an accompanying issue (closes #0000) and the maintainers have approved on my working plan.

Motivation

add suguar to decode class, create more friendly API, resolve comment of: #370 (comment)

Test Plan

Related issues/PRs

@fospring fospring force-pushed the feat-deser-class-by-schema-collection-userfriendlyapi branch from fcc2b24 to c0ab23d Compare January 26, 2024 02:35
is_inited: "boolean",
records: {map: { key: 'string', value: 'string' }},
truck: Truck,
messages: {array: {value: 'string'}},
Copy link
Member

Choose a reason for hiding this comment

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

And this messages: "array"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can remove it since no reconstruct is needed. I will update it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

@@ -28,7 +28,8 @@ class Truck {
static schema = {
name: "string",
speed: "number",
loads: {collection: {reconstructor: UnorderedMap.reconstruct, value: 'string'}}
// loads: {collection: {reconstructor: UnorderedMap.reconstruct, value: 'string'}}
loads: {class: UnorderedMap }
Copy link
Member

Choose a reason for hiding this comment

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

Nice this case still works. I think this is worth mention as an example in readme, as one case in:

export class StatusDeserializeClass {
  static schema = {

Copy link
Member

@ailisp ailisp left a comment

Choose a reason for hiding this comment

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

Looks great! I have a few minor concerns on the documentation above, other than that this api is quite easy to use and well implemented!

@gagdiez gagdiez mentioned this pull request Jan 29, 2024
4 tasks
@gagdiez
Copy link
Collaborator

gagdiez commented Jan 29, 2024

fixed the documentation on the doc

@gagdiez
Copy link
Collaborator

gagdiez commented Jan 30, 2024

@fospring @ailisp I think it would be much clearer if we follow this idea: https://github.com/nameskyteam/borsher

Basically, it would look like this:

import {Schema} from 'near-sdk-js'

class Car {
  schema = { brand: Schema.string, velocity: Schema.number }
}

class Contract {
  schema = {
    field1: Schema.string,
    field2: Schema.HashMap(Schema.string, Schema.number),
    field3: Schema.Array(Schema.bigInt),
    field4: Schema.Class(Car)
    field5: Schema.Option(number)
  }
}

This has several advantages:

  • The Schema object makes the schema error-proof.
  • All fields are always well defined, making it easy to (de)serialize them
  • It can be used across JSON and Borsh serialization.

We could even just grab the https://github.com/nameskyteam/borsher and import it directly into here.

@fospring
Copy link
Contributor Author

fospring commented Feb 2, 2024

@fospring @ailisp I think it would be much clearer if we follow this idea: https://github.com/nameskyteam/borsher

Basically, it would look like this:

import {Schema} from 'near-sdk-js'

class Car {
  schema = { brand: Schema.string, velocity: Schema.number }
}

class Contract {
  schema = {
    field1: Schema.string,
    field2: Schema.HashMap(Schema.string, Schema.number),
    field3: Schema.Array(Schema.bigInt),
    field4: Schema.Class(Car)
    field5: Schema.Option(number)
  }
}

This has several advantages:

  • The Schema object makes the schema error-proof.
  • All fields are always well defined, making it easy to (de)serialize them
  • It can be used across JSON and Borsh serialization.

We could even just grab the https://github.com/nameskyteam/borsher and import it directly into here.

great suggestion! I will update it

@ailisp
Copy link
Member

ailisp commented Feb 5, 2024

@gagdiez great suggestion! If it's adding a lot work, let's keep the behavior for:

  • collections, maybe look like: Schema.Class(LookupMap, <schema of the value>)
  • for primitive types like field1: Schema.string, it can be omitted from schema

@fospring
Copy link
Contributor Author

@gagdiez great suggestion! If it's adding a lot work, let's keep the behavior for:

  • collections, maybe look like: Schema.Class(LookupMap, <schema of the value>)
  • for primitive types like field1: Schema.string, it can be omitted from schema

It looks a little difficult, I will try to resolve it in a new MR

@gagdiez
Copy link
Collaborator

gagdiez commented Feb 25, 2024

@fospring sounds good to me, ping me and I'll see to help as well

@fospring
Copy link
Contributor Author

@fospring sounds good to me, ping me and I'll see to help as well

Great!

@fospring fospring force-pushed the feat-deser-class-by-schema-collection-userfriendlyapi branch from c3830f2 to ed0bda2 Compare February 25, 2024 09:01
@ailisp ailisp merged commit 61fe54e into near:develop Feb 26, 2024
2 checks passed
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.

3 participants