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 value transformer to de/serializer #572

Closed
edgesite opened this issue Jun 14, 2019 · 4 comments
Closed

[FEAT] Add value transformer to de/serializer #572

edgesite opened this issue Jun 14, 2019 · 4 comments

Comments

@edgesite
Copy link

edgesite commented Jun 14, 2019

Information

  • Type: Feature request

Description

Add value transformer to des/serializer can help apply Hashids like mechanism much easier.

Example

const SALT = 'SALT';

class Entity {
  @Property({encode: v => hashids.encode(SALT, v), decode: e => hashids.decode(SALT, e)})
  @PrimaryColumn()
  id: number;
}
@Romakita Romakita added this to the BACKLOG milestone Jun 15, 2019
@Romakita
Copy link
Collaborator

Romakita commented Jun 15, 2019

Good Idea :).
I suggest this implementation:

const SALT = 'SALT';

class Entity {
  @PropertySerialize(v =>  hashids.encode(SALT, v)) 
  @PropertyDeserialize(e =>  hashids.decode(SALT, e))
  @PrimaryColumn()
  id: number;
}

What do you think ?

@edgesite
Copy link
Author

Great. Maybe better also add it to JsonPropertyOptions? I just made an example
edgesite@5f6f439

Also, I noticed the #539 but it's seems stalled. Split ser/des to a standalone package is a great idea, the existing converter system is a bit tricky. It would be nice to redesign the API of the converter system to keep it clean and elegant.

@Romakita
Copy link
Collaborator

@edgesite I don't understand in your code why you change the IConverterOptions with decode and encode, because decode/encode are applied on a specific prop.

Yes #539 is a test. Problem: It's a breaking change for developers, because it require to add this dependency in package.json for each projects using Ts.ED (like @tsed/di and @tsed/common).
But in future the converters and jsonschema will be splited. Redesign jsonschema API is planed also. For the converter, isn't planed. The tricky code is over JsonSchema / PropertyMetada / PropertyRegistry. This part is really confusing and redundante.

Right know, I'll work on your feature ;). Refactoring jsonschema/converter will be done in another PR in future.

See you
Romain

@edgesite
Copy link
Author

edgesite commented Jun 16, 2019

@Romakita oh I just miss push -f when I amend the commit.
Check edgesite@5837227

Also, can we document *Registry to show users how to create custom filters? It's really a pain feature and needs looking into the code right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants