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
375 stars 178 forks source link

Importing @vonage/server-sdk causes TypeScript errors because of incorrect imports in @vonage/voice #847

Closed duckdotapk closed 5 months ago

duckdotapk commented 11 months ago

Expected Behavior

This should not cause TypeScript errors.

Current Behavior

Importing the Vonage class as follows:

import { Vonage } from "@vonage/server-sdk";

Causes the following TypeScript errors:

node_modules/@vonage/voice/dist/classes/NCCO/Conversation.d.ts:15:46 - error TS2694: Namespace '"<TRUNCATED_FOR_PRIVACY>/node_modules/@vonage/voice/dist/types"' has no exported member 'ConversationAction'.

15     serializeToNCCO(): import("../../types").ConversationAction;
                                                ~~~~~~~~~~~~~~~~~~

node_modules/@vonage/voice/dist/classes/NCCO/Stream.d.ts:11:46 - error TS2694: Namespace '"<TRUNCATED_FOR_PRIVACY>/node_modules/@vonage/voice/dist/types"' has no exported member 'StreamAction'.

11     serializeToNCCO(): import("../../types").StreamAction;
                                                ~~~~~~~~~~~~

Possible Solution

This seems to be because the packages/voice/lib/types.ts file does not actually export these types.

I can see they are defined in the types/NCCO folder so maybe it should? I'm not sure of the exact solution because I'm not that familiar with the inner design decisions of this SDK.

Steps to Reproduce (for bugs)

As far as I can tell, just import the Vonage class as described above in a TypeScript project.

Context

I periodically audit my packages and update out-of-date ones but these errors cropping up when upgrading @vonage/server-sdk from 3.4.0 to 3.7.0, which indirectly updates @vonage/voice from 1.4.0 to 1.6.0 in my package-lock.json, has made me revert the upgrade for now.

Today, I had some time to write up this issue so it can hopefully get acknowledged and fixed.

Your Environment

manchuck commented 10 months ago

@duckdotapk I cannot reproduce with version of server SDK (3.7.1) or with the version you are reporting. Can you share your tsconfig.json so I can take a look to see if there is something wonky with it?

duckdotapk commented 10 months ago

@manchuck It seems to be related to me having skipLibCheck explicitly set to false in my tsconfig.json.

If I set it to true, the errors go away. I don't quite remember why I set it like this, though, and I think there might still be a problem in the SDK regardless? I'm not sure.

duckdotapk commented 10 months ago

Btw, this is with 3.7.2 installed now.

manchuck commented 10 months ago

@duckdotapk Ahh, that would do it. For now, skipLibCheck is set to true due to some conflicts when bucket files were added. We had code samples that suggested importing like @vonage/voice/dist/classes/NCCO/Conversation. Since that is not ideal (as you would need to know the whole structure of each package), bucket files were created. I had some concerns that developers were still using that older method of importing and wanted to deprecate the older way of importing. There were many cases where we had types, interfaces and classes all named the same when they should have been made unique. We will be going through and resolving these conflicts which will allow us to turn off skipLibCheck

manchuck commented 7 months ago

Quick update for you @duckdotapk There is a current initiative to add documentation to the code. This is requiring fixes in Typescript which will no longer require us to need to have skipLibCheck enabled.

duckdotapk commented 6 months ago

Fantastic, thanks for the update. :)

manchuck commented 6 months ago

@duckdotapk I am wrapping up this work with #872 As it is the end of the year, wont be released until early Jan. I apologize that we were not able to get this out sooner for you. I hope you have a happy holiday and I will provide an update once I get back from the holidays

duckdotapk commented 6 months ago

That's awesome, I appreciate it a lot! No worries about the time frame and I hope you have a happy holiday, too!

manchuck commented 5 months ago

This should be resolved now that v3.12.0 has been released

duckdotapk commented 5 months ago

@manchuck Awesome! Thank you for your hard work, Hot Dog Hat Man.

manchuck commented 5 months ago

🌭

manchuck commented 5 months ago

Side question: How are the doc blocks looking in your IDE of choice?

duckdotapk commented 5 months ago

@manchuck A little bit broken looking in WebStorm 2023.3.1 though I'm not sure if that's your fault or theirs.

example

manchuck commented 5 months ago

hmm maybe it does not recognize ts as a valid render. Or I'm building out the @example tag incorrectly. I'll have to play around with it. Thanks for the screen shot