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

wish: If no callback is supplied return a Promise #20

Open
markstos opened this issue May 30, 2018 · 11 comments
Open

wish: If no callback is supplied return a Promise #20

markstos opened this issue May 30, 2018 · 11 comments
Labels
feature-request New feature or enhancement. May require GitHub community feedback.

Comments

@markstos
Copy link

Node.js Now has some Promise methods in the core as well as async and await.

Having validate return a promise if no callback is provided would a welcome addition.

Bluebird's asCallback() method as one of example of supporting both callback and Promise APIs.

http://bluebirdjs.com/docs/api/ascallback.html

@srchase srchase added the feature-request New feature or enhancement. May require GitHub community feedback. label Dec 24, 2018
@ChrisTalman
Copy link

If this were to be written for the latest versions of Node, using native promises, it seems as though this would be a straightforward endeavour. It doesn't seem that much would need to be changed in the source code. Is there anything which complicates this, such as backwards compatibility? I notice that the source code doesn't use native classes - is that for style or backwards compatibility?

@markstos
Copy link
Author

A common policy for Node.js packages is to only support Node.js versions as far back as the oldest maintained LTS release, which is currently Node.js 6.

You can use "node.green" to confirm which features are supported on Node 6.

This is an AWS project and the oldest Node.js version that AWS supports is currently 6.10 (for AWS Lambda).

However, if you implement this with Bluebird, it would work on even older Node.js versions. This module doesn't have to use async / await internally for module consumers to use them externally.

References

@nathancahill
Copy link

Currently: Node 8 is in maintenance, Node 10 and 12 are active LTS and Node 14 is coming soon: https://nodejs.org/en/about/releases/

AWS Lambda runs Node 10 and 12: https://nodejs.org/en/about/releases/

@markstos if I send a PR to add support for promises only when no callback is passed, would you be interested?

@markstos
Copy link
Author

Yes.

@nathancahill
Copy link

Opps, I thought you were a maintainer. Should have asked @jeskew.

@nathancahill
Copy link

Published a fork as @nathancahill/sns-validator since the last commit to this project was 2 years ago. If this does get merged, I will be marking the fork as deprecated.

@robbiet480
Copy link

@trivikr @ajredniwja Very sorry to randomly tag both of you, but you are the two most recent committers to the aws-sdk-js repo that are AWS employees, so I'm hoping you can help. This PR (#28) is pretty important if AWS wants to continue supporting this module. The project hasn't seen any input from a AWS stakeholder for at least 2 years, I assume because the original team members have moved on. Can you help find someone will to review and merge the PR (#28) and then get a updated version published to NPM? I think Promises are a pretty standard level thing required of any good module these days. Thanks so much for your help!

@markstos
Copy link
Author

@robbiet480 Just use the @nathancahill fork. It's already published. https://www.npmjs.com/package/@nathancahill/sns-validator

@robbiet480
Copy link

@markstos Yeah I saw that and I am using it for now but would still be great to get this merged into upstream.

@JoshM1994
Copy link

I opted to just promisify the existing upstream version:

import { promisify } from 'util';

import * as lambda from 'aws-lambda';
import MessageValidator from 'sns-validator';

const snsValidator = new MessageValidator();
const validateProm = promisify(snsValidator.validate.bind(snsValidator));

export const handler = async (event: lambda.SNSEvent) => {
  const { Records } = event;
  if (Records.length !== 1) throw new Error('Expected exactly one record');
  const [{ Sns }] = Records;
  // Throws an error if the message is invalid
  await validateProm(Sns)
  ...
};

@devinstewart
Copy link

Hey folks. Ran across this thread and decided by the looks of the code, what I wanted was something a little more modern. I wrote a module that does the the same stuff, and wouldn't mind some eyes on it, even if you don't intend on using it.

https://github.com/devinstewart/sns-payload-validator

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request New feature or enhancement. May require GitHub community feedback.
Projects
None yet
Development

No branches or pull requests

7 participants