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

Can't compile typescript without adding @types/express #671

Closed
victorbadila opened this issue Apr 22, 2021 · 7 comments
Closed

Can't compile typescript without adding @types/express #671

victorbadila opened this issue Apr 22, 2021 · 7 comments
Labels
type: twilio enhancement feature request on Twilio's roadmap

Comments

@victorbadila
Copy link

Issue Summary

I'm not able to build a typescript project with this package just by adding the twilio and @types/twilio packages, when I build the solution. I am not using anything express related in terms of imports from twilio and my project is express free as well.

If I install @types/express as a dev dependency then it will build, but I don't think that's how it's supposed to work.

Steps to Reproduce

  1. Create node typescript app
  2. Install twilio as dependency and @types/twilio as dev dependency
  3. Import twilio in a ts file, use the method for sending a basic message
  4. Run npm run build

Exception/Log

> tsc -p tsconfig.json

node_modules/twilio/lib/webhooks/webhooks.d.ts:1:35 - error TS2307: Cannot find module 'express' or its corresponding type declarations.

1 import { Request, Response } from 'express';
                                    ~~~~~~~~~


Found 1 error.

Technical details:

  • twilio-node version: 3.19.3
  • node version: 14.16.0
@thinkingserious
Copy link
Contributor

Hello @victorbadila,

We are not including @types/express as a dependency based on this guidance.

Is it a requirement that your project be express free? I'm trying to understand how we could have reduced friction for you to get up and running quickly. Thank you!

With best regards,

Elmer

@thinkingserious thinkingserious added status: waiting for feedback waiting for feedback from the submitter type: question question directed at the library labels Apr 23, 2021
@victorbadila
Copy link
Author

Hi @thinkingserious

It is not a requirement that my project is express, however I wasn't able to get it running without installing @types/express. When I just instaled twilio and @types/twilio I would get the error mentioned above when building, that's how I figured I should install @types/express as well, though.

The only things I do twilio related are:

import * as twilio from "twilio";

// ...

const accountSid: string = process.env.TWILIO_ACCOUNT_SID;
const token: string = process.env.TWILIO_AUTH_TOKEN;

const client = twilio(accountSid, token);

// ...

await client.messages.create({ from, to, body });

Could it be that first import * that's the culprit? It's the only way it lets me import twilio in TS from what I've seen.

@thinkingserious
Copy link
Contributor

Please try: import { Twilio } from "twilio"; and then const client = new Twilio(accountSid, token);

What documentation did you use to get started?

Thanks!

@victorbadila
Copy link
Author

I tried it like this, it still gives me an error if I don't have @types/express installed as a dev dependency. For documentation I looked here https://github.com/twilio/twilio-node/blob/main/examples/typescript/example.ts
It doesn't state exactly how to import twilio in the example so I tried it in any ways I could.

@eshanholtz
Copy link
Contributor

There's a nice Twilio blog post on how to get started with Twilio and TypeScript.

Regarding whether or not the @types packages should be a dependency on a devDependency, there are various conflicting opinions out there. As linked earlier, the primary motivation in moving this to devDependencies was that many twilio-node users do not use TypeScript, and therefore do not need the @types packages. Adding it as a required dependency would cause unnecessary package bloat. However, given that all twilio-node Typescript users would have to manually install the @types packages in order to use twilio at all, I'm leaning towards adding this back as a required dependency.

This issue has been added to our internal backlog to be prioritized. Pull requests and +1s on the issue summary will help it move up the backlog.

@eshanholtz eshanholtz added status: help wanted requesting help from the community type: community enhancement feature request not on Twilio's roadmap and removed status: waiting for feedback waiting for feedback from the submitter type: question question directed at the library labels Apr 29, 2021
@shellscape
Copy link

@thinkingserious it's a bit of a bummer if a shop is all koa-based, or using something like apollo-server-lambda, to have to include types manually in our dependencies for a package that we have no interest in, nor use for. Kind of a wonky architecture decision to force users to install that. I get why you're piggy backing on express types, but that's making a grand assumption about the majority use-case.

the primary motivation in moving this to devDependencies was that many twilio-node users do not use TypeScript, and therefore do not need the @types packages. Adding it as a required dependency would cause unnecessary package bloat.

I'm a core team member on the Rollup project, and the lead for the official plugins repo. The rollup project ships types with everything we can (or have time to add to) because enough users these days use TypeScript that it makes sense to. It's no longer niche, so plain vanilla JS users (I'm one of them half the time) have come to expect to see TS accommodations within packages. It's no longer black magic. Catering to vanilla JS users over TS users is making what should be an equal class of developer a 2nd class citizen in the ecosystem. All together, the Rollup project and it's plugins have around 50x the weekly downloads that twilio-node enjoys, and we've seen no net negatives from including dependency types in devDeps that are required to deliver an effective and smooth developer experience.

@eshanholtz "unnecessary package bloat" only applies to dependencies as it's been best practice for a long time now to use a production flag for the package manager of choice, to prevent devDeps from being installed during any kind of deployments, and any bundler out there now will ignore devDeps. Quibbling over a few kb for a SaSS supporting core package seems a bit silly. Please do move those back to devDeps and out of peerDeps.

@childish-sambino
Copy link
Contributor

Fixed by #675

@childish-sambino childish-sambino added type: community enhancement feature request not on Twilio's roadmap type: twilio enhancement feature request on Twilio's roadmap and removed status: help wanted requesting help from the community type: community enhancement feature request not on Twilio's roadmap labels Nov 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: twilio enhancement feature request on Twilio's roadmap
Projects
None yet
Development

No branches or pull requests

5 participants