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

Make "deserialize" promisifiable #77

Open
razetdinov opened this issue Feb 24, 2018 · 2 comments · May be fixed by #123
Open

Make "deserialize" promisifiable #77

razetdinov opened this issue Feb 24, 2018 · 2 comments · May be fixed by #123

Comments

@razetdinov
Copy link
Contributor

deserialize uses node-style callbacks, but fails to follow the rule "callback is the last argument". This prevents making use of util.promisify, which

Takes a function following the common error-first callback style, i.e. taking a(err, value) => ... callback as the last argument, and returns a version that returns promises.

I suggest moving callback to the end of argument list. This would also be more convenient: now you have to pass null for the callback in cases when you don't need async deserialization, but have to pass custom args.

@alexggordon
Copy link
Member

You are correct, this is a change that needs to be made. Unfortunately, this would be a breaking change which would be tough to implement in version 1x of serializer.

I'm thinking version 2 should return a promise on serialization/deserialization instead of a callback. Thoughts?

@waliarubal
Copy link

Hello @razetdinov and @alexggordon, I have created below code for my typescript project and using it successfully without any issues. Its a generic await-able method.

static async Deserialize<T>(modelSchema: ClazzOrModelSchema<T>, json: any): Promise<T> { return new Promise<T>((resolve, reject) => { deserialize<T>(modelSchema, json, (error: any, result: T) => { if (error) reject(error); else resolve(result); }); }); }

This was referenced Feb 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants