Vonage / vonage-node-sdk

Vonage API client for Node.js. API support for SMS, Voice, Text-to-Speech, Numbers, Verify (2FA) and more.
Apache License 2.0
382 stars 181 forks source link

Not working in Typescript Build #533

Closed mrfambo closed 3 years ago

mrfambo commented 3 years ago

@vonage/server-sdk is not working in typescript build

Expected Behavior

import VonageLib from '@vonage/server-sdk'
const Vonage = new VonageLib({
    apiKey: process.env.VONAGE_API_KEY,
    apiSecret: process.env.VONAGE_API_SECRET
 })

Needs to work in typescript build too. Not just in TS environment.

Current Behavior

The code is fine in typescript, but when build is created it makes the following JS code:

const server_sdk_1 = require("@vonage/server-sdk");
const Vonage = new server_sdk_1.default({
    apiKey: process.env.VONAGE_API_KEY,
    apiSecret: process.env.VONAGE_API_SECRET
});

Then the build gives the following error in production(running js code):

Cloud Function TypeError: server_sdk_1.default is not a constructor

If I change the import to import * as VonageLib from '@vonage/server-sdk', typescript gives the error This expression is not constructable.

bbmouka commented 3 years ago

The same with js EsLint

https://github.com/Vonage/vonage-node-sdk/issues/534

the temporary solution is to fix your package to 2.10.8 in your pacage.json

kellyjandrews commented 3 years ago

This should be resolved in 2.10.10

shihabuddin commented 3 years ago

I am using 2.10.10, but still getting this error. Which commit fixes this issue? I would like to check if the commit exists in my local version.

kellyjandrews commented 3 years ago

@shihabuddin sorry to hear that - it seems to compile ok for me - but could you share your tsconfig as well? This version is still sort of a mess with typescript, and I'm focused more on replacing, but can continue to update the types here to hopefully keep you rolling.

shihabuddin commented 3 years ago

@kellyjandrews I am using nx. Here are the tsconfig files generated by nx:

tsconfig.base.json

{
  "compileOnSave": false,
  "compilerOptions": {
    "rootDir": ".",
    "sourceMap": true,
    "declaration": false,
    "moduleResolution": "node",
    "emitDecoratorMetadata": true,
    "experimentalDecorators": true,
    "importHelpers": true,
    "target": "es2015",
    "module": "esnext",
    "lib": ["es2017", "dom"],
    "skipLibCheck": true,
    "skipDefaultLibCheck": true,
    "baseUrl": ".",
    "paths": {
      "@mycompany/data-access-client": [
        "libs/data-access-client/src/index.ts"
      ],
      "@mycompany/util-logging": ["libs/util-logging/src/index.ts"]
    }
  },
  "exclude": ["node_modules", "tmp"]
}

tsconfig.json

{
  "extends": "../../tsconfig.base.json",
  "files": [],
  "include": [],
  "references": [
    {
      "path": "./tsconfig.lib.json"
    },
    {
      "path": "./tsconfig.spec.json"
    }
  ]
}

tsconfig.lib.json

{
  "extends": "./tsconfig.json",
  "compilerOptions": {
    "module": "commonjs",
    "outDir": "../../dist/out-tsc",
    "declaration": true,
    "types": ["node"]
  },
  "exclude": ["**/*.spec.ts"],
  "include": ["**/*.ts"]
}

tsconfig.spec.json

{
  "extends": "./tsconfig.json",
  "compilerOptions": {
    "outDir": "../../dist/out-tsc",
    "module": "commonjs",
    "types": ["jest", "node"]
  },
  "include": [
    "**/*.spec.ts",
    "**/*.spec.tsx",
    "**/*.spec.js",
    "**/*.spec.jsx",
    "**/*.d.ts"
  ]
}

For now I am using this workaround:

import * as Vonage from '@vonage/server-sdk';

// workaround for https://github.com/Vonage/vonage-node-sdk/issues/533
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const vonage = new (Vonage as any)(
  {
    apiKey: env.VONAGE_API_KEY,
    apiSecret: env.VONAGE_API_SECRET,
  }
);

By the way, I don't see 2.10.10 in https://github.com/Vonage/vonage-node-sdk/releases. Why is that?

kellyjandrews commented 3 years ago

@shihabuddin Thanks - I'll take a look and see if I can figure it out. My typescript is still a bit shaky. PRs with solutions also welcome - sorry for the troubles :(

shihabuddin commented 3 years ago

@kellyjandrews Thanks. I have submitted a PR with the changes that works for me.

kellyjandrews commented 3 years ago

@shihabuddin I will take a look today/tomorrow - my main concern is that we had it that way before, and had similar issues with imports. I want to check a bit on my side and see if there is a setting that fixes this, or if the changes you added require something specific in the config to prevent issues. Thanks for your help.

kellyjandrews commented 3 years ago

@shihabuddin I've used the following and had no issues with the current version (2.10.9 is the same as 2.10.10 - I need to go tag that properly).

    "compilerOptions": {
        "declaration": true,
        "outDir": "dist",
        "strict": false,
        "lib": ["es2017", "dom"],
        "module": "commonjs",
        "target": "es2017",
        "moduleResolution": "node",
        "esModuleInterop": true,
        "skipDefaultLibCheck": true
    },
    "include": ["src/index.ts"]
}

At this time, I'm going to keep things as they are so there isn't any potential regression on this.

shihabuddin commented 3 years ago

Thanks @kellyjandrews. Using esModuleInterop solved the problem for me.