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

SDK became worse #335

Closed
devlato opened this issue Nov 7, 2016 · 14 comments
Closed

SDK became worse #335

devlato opened this issue Nov 7, 2016 · 14 comments
Labels
type: question question directed at the library

Comments

@devlato
Copy link

devlato commented Nov 7, 2016

Hey guys,

why do you changed the SDK so much? Previously developer had no need to care about lots of internal things, and now it is required to know even your URLs, and it makes all your SDK and helper stuff pretty useless comparing to the previous SDK (because sometimes it's easier to implement it manually).

@happyruss
Copy link

and the build appears to be broken.
and since nodemailer-sendgrid-transport is no longer supported, so I can no longer use deitch/activator and must create my own activation and password-reset algorithms now.

@thinkingserious thinkingserious added the type: question question directed at the library label Nov 7, 2016
@thinkingserious
Copy link
Contributor

Hello @devlato,

Thanks for reaching out and sharing your feedback, we appreciate it!

This version is step one in a three step upgrade of the previous library.

The first step is to provide baseline support for all v3 Web API endpoints across all of our 7 SDKs. This is complete.

Step two is in progress, and when complete, I believe, will satisfy your requirements. You can follow along here [see the Project "Mail Helper Enhancement (v3 mail/send)"] and of course we would love to hear any feedback you may have.

Step three are workflows which will provide working examples for various common use cases.

If you have further questions, feel free to continue the conversation here as well.

Hello @happyruss,

This should not be true, can you please open a new ticket with the details and I'll do my best to help.

@happyruss
Copy link

Build Failing:
https://github.com/sendgrid/sendgrid-nodejs - it says 'build-failing' am i missing something?

nodemailer-sendgrid-transport
https://github.com/sendgrid/nodemailer-sendgrid-transport - says no longer supported

but it's ok, i have a plan already to do it without deitch/activator

@thinkingserious
Copy link
Contributor

@happyruss,

Thanks for taking the time to reply.

I'm not sure what is going on with the badge, but the tests are passing: https://travis-ci.org/sendgrid/sendgrid-nodejs/builds/170623381 :(

The failed test, according to the badge, is this pull request: https://travis-ci.org/sendgrid/sendgrid-nodejs/builds/172291784, but it has not been merged yet.

Yes, nodemailer-sendgrid-transport is no longer supported. Instead, we are recommending people use this library.

@thinkingserious
Copy link
Contributor

@happyruss,

The issue turned out to be that the mock server happened to be down during the last test run :(

Thanks for the heads up, all is good now.

@thebigredgeek
Copy link

thebigredgeek commented Nov 18, 2016

Is there any way we could revert to the old API? It requires an intricate knowledge of sendgrid internals rather than providing abstraction. At the same time, the paradigm seems to be functional but without chaining. I have to create new objects, then call 4 or 5 functions one at a time (zero chaining) to configure the object before passing it into yet another function call. It literally takes more code to use the helpers than it does to construct the request payloads manually!

@thinkingserious
Copy link
Contributor

Hi @thebigredgeek,

Yes you can, please see this: https://github.com/sendgrid/sendgrid-nodejs/blob/master/TROUBLESHOOTING.md#continue-using-v2

With regards to your concerns, they are definitely valid. You have arrived at the new SDK in Phase 1 of 3.

Phase 1 was complete basic coverage of the v3 endpoints and decoupling the SDK from the v2 mail/send endpoint. Phase 2 is the abstraction in the form of helpers (you can track the progress here -- see the Mail Helper enhancement project). Phase 3 is creating abstractions for common workflows.

Thanks for reaching out and supporting SendGrid!

@victorhqc
Copy link

Its amazing how using the helpers is more complicated than using the API Object directly. There seem to have a lot of problems with the helpers. I end up with so many lines of codes, and so many new objects that this is not usable at all. Looks like some weird mix of OOP with Composite pattern that ends up being a half bake code with some serious problems.

Also, I noted the following problem:

import Sendgrid from 'sendgrid';

const apiKey = 'some-key-just-for-example';
class MyClass {
    getMailer() {
      return Sendgrid(apiKey);
    }

   someFunction() {
     const mailer = this.getMailer();
   }
}

When I try to use the mailer object its failing, similar than here:
http://stackoverflow.com/questions/10107198/javascript-not-a-constructor-exception-while-creating-objects

Not even using the require in the getMailer

const apiKey = 'some-key-just-for-example';
class MyClass {
    getMailer() {
      return require('sengrid')(apiKey);
    }

I had to change it like the following to have it working

const apiKey = 'some-key-just-for-example';
class MyClass {
   // This is not working
    getMailer() {
     return require('sengrid')(apiKey);
    }

   someFunction() {
     const mailer = require('sengrid')(apiKey);
     ... // mailer used later works
   }
}

@thebigredgeek
Copy link

@victorhqc I am just gonna roll my own functional wrapper around the REST api. https://github.com/thebigredgeek/sendgridjs

@thinkingserious
Copy link
Contributor

Hello @victorhqc,

I'm having trouble reproducing your error. Here is what I have:

"use strict";

class MyClass {
  getMailer() {
    return require('sendgrid')(process.env.SENDGRID_API_KEY);
  }

  sendEmail() {
    const mailer = this.getMailer();
    var helper = require('sendgrid').mail;
    var from_email = new helper.Email('[email protected]');
    var to_email = new helper.Email('[email protected]');
    var subject = 'Hello World from the SendGrid Node.js Library!';
    var content = new helper.Content('text/plain', 'Hello, Email!');
    var mail = new helper.Mail(from_email, subject, to_email, content);

    var request = mailer.emptyRequest({
      method: 'POST',
      path: '/v3/mail/send',
      body: mail.toJSON(),
    });

    mailer.API(request, function(error, response) {
      console.log(response.statusCode);
      console.log(response.body);
      console.log(response.headers);
    });
  }
}

var sg = new MyClass();
sg.sendEmail();

With regards to the usability of the helpers, we are working on a new version, as mentioned here, please do let us know if you have any feedback. We appreciate your support!

@transitive-bullshit
Copy link

when is the phase v3 planned? tbh this seems more like either an internal client lib or a power user lib, not something which is meant to be consumed by your 95% userbase who just wanna sign in and send an email exactly like nodemailer allows.

@thinkingserious
Copy link
Contributor

@fisch0920,

You should see progress in the coming weeks.

For a preview, you can check out what we did with the C# SDK recently. We will have something similar for Node.js, but adapted to this community's needs.

For background on the current status on this SDK, please see this comment.

We hope you will join us as we redesign to help ensure we create an experience that serves you well.

With Best Regards,

Elmer

@transitive-bullshit
Copy link

Hey @thinkingserious -- thanks for the prompt reply && looking forward to seeing the updates :)

@SendGridDX
Copy link
Collaborator

Hello Everyone,

I thought you might be interested in the re-design on the SendGrid Node.js Mail Helper: #378

Your feedback would be greatly appreciated!

With Best Regards,

Elmer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: question question directed at the library
Projects
None yet
Development

No branches or pull requests

7 participants