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

Bug: incorrect type for mediaUrl in MessageListInstanceCreateOptions #459

Closed
okhomenko opened this issue Jun 14, 2019 · 3 comments · Fixed by #476
Closed

Bug: incorrect type for mediaUrl in MessageListInstanceCreateOptions #459

okhomenko opened this issue Jun 14, 2019 · 3 comments · Fixed by #476
Labels
status: work in progress Twilio or the community is in the process of implementing type: bug bug in the library type: twilio enhancement feature request on Twilio's roadmap

Comments

@okhomenko
Copy link

Version:
"twilio@3.23.2"

Code Snippet

/**
 * @param {object} opts
 * @param {object} opts.body
 * @param {string} [opts.mediaUrl]
 * @param {string} opts.recipientNumber
 */
const sendSMS = ({ body, mediaUrl, recipientNumber }) => {
  /** @type {{ twilioClient: import('twilio').Twilio, twilioNumber: string; }} */
  const { twilioClient, twilioNumber } = container.cradle;

  const toNumberE164Format = `+1${recipientNumber}`;

  /**
   * @type {import('twilio/lib/rest/api/v2010/account/message').MessageListInstanceCreateOptions}
   */
  const smsContent = {
    from: twilioNumber,
    to: toNumberE164Format,
    body,
    mediaUrl,
  };

  return twilioClient.messages.create(smsContent);
};

Steps to Reproduce

  1. run this snippet against tsc
  2. notice it's a javascript code with types in jsdoc annotations

Bug

Type for mediaUrl in import('twilio/lib/rest/api/v2010/account/message').MessageListInstanceCreateOptions is

  mediaUrl?: string[];

I think it should be

  mediaUrl?: string | string[];
@childish-sambino
Copy link
Contributor

A string array is the intended type. See example usage here: https://www.twilio.com/docs/sms/send-messages?code-sample=code-send-an-mms-message&code-language=Node.js&code-sdk-version=3.x}

Are you suggesting the lib should accept either?

@childish-sambino childish-sambino added status: waiting for feedback waiting for feedback from the submitter type: question question directed at the library labels Aug 27, 2019
@okhomenko
Copy link
Author

I think it accepts either string|string[]

* @param {string|list} [opts.mediaUrl] -

@childish-sambino
Copy link
Contributor

Ah, I see. The JS accepts either and so should the TS. I'll look into a fix.

@childish-sambino childish-sambino added status: work in progress Twilio or the community is in the process of implementing type: bug bug in the library type: twilio enhancement feature request on Twilio's roadmap and removed status: waiting for feedback waiting for feedback from the submitter type: question question directed at the library labels Aug 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: work in progress Twilio or the community is in the process of implementing type: bug bug in the library type: twilio enhancement feature request on Twilio's roadmap
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants