twilio-node icon indicating copy to clipboard operation
twilio-node copied to clipboard

[Bug] Invalid types in last update

Open codygordon opened this issue 9 months ago • 9 comments

Some of the changes in the following commit define types don't match the actual structure of the data causing type errors when attempting to access values as documented.

For example, lineTypeIntelligence: Record<string, object> suggests that the shape would be:

lineTypeIntelligence: {
   key: { ... }
}

It should probably instead be Record<string, any>.

Or, even better, it should contain the actual available keys and value types as documented.

https://github.com/twilio/twilio-node/commit/49f3f2b147818bf03d0067ad7cc13fbcbaf72c7d#diff-55a60b51b684465835a0448f6efceb0ef9b0cb5ada598728e1594ebf2507deddR292

codygordon avatar Mar 11 '25 15:03 codygordon

I also see an issue in src/rest/bulkexports/v1/export/job.ts:

https://github.com/twilio/twilio-node/commit/49f3f2b147818bf03d0067ad7cc13fbcbaf72c7d#diff-5a4d9d347825dc80283179a0537726bed59c467114fdbefac69d0b4d73329a0bR409

This type should be Record<string, any>[] because the API returns the following structure for job.details:

[
  {
    status: 'Submitted',
    count: 3,
    days: [ '2025-03-07', '2025-03-08', '2025-03-09' ]
  }
]

msheakoski avatar Mar 11 '25 22:03 msheakoski

The same invalid types remain as of v5.5.2

msheakoski avatar Apr 10 '25 17:04 msheakoski

I'm also seeing an issue with broken types as a result of this commit: https://github.com/twilio/twilio-node/commit/49f3f2b147818bf03d0067ad7cc13fbcbaf72c7d

The issue I'm encountering is similar to the one @codygordon posted but on ParticipantInstance.

Old type prior to v.5.5.0: messaging_binding: any; Broken type introduced in v5.50: messaging_binding: Record<string, object>; Expected type: messaging_binding: Record<string, string>; or messaging_binding: Record<string, any>;

danny-fairly avatar Apr 11 '25 18:04 danny-fairly

Same here. A few erroneous types were introduced including:

  • DocumentInstance.data: Record<string, object>;
  • FlexFlowInstance.integration: Record<string, object>;

dmitriy-kudelko avatar Apr 17 '25 08:04 dmitriy-kudelko

Hello! Thanks for raising the issue. We will look at it on priority basis.

manisha1997 avatar May 03 '25 10:05 manisha1997

Hi, we are experiencing a similar issue with the types field on the SubscriptionListInstanceCreateOptions interface, which was updated from Array<any> to Array<object> type. Now when trying to create a subscription, we get the following error Error: Input list of Types cannot be empty.

We have reverted our twilio client version back to 5.2.2, before this change, and things are working as expected, but it would be great to get this fixed, assuming this is indeed the root issue here.

jswirbs avatar May 05 '25 14:05 jswirbs

We experienced the same issue: Input list of Types cannot be empty above. We reverted to version 5.4.5 and I noticed the same change noted above, appearing to be the culprit.

It looks like the provided objects are no longer stringified using (e: any) => serialize.object(e) as they were before. (See serialize.object() functionality)

mindiweik avatar May 09 '25 18:05 mindiweik

Hi! So as these helpers are autogenerated from the public open api specs, due to a recent update these changes have been introduced. I see the best way in most of the above cases would be to use Record<string, any>. In case of encryption_details, where the expected datatype should be Record<string, any>[], I'll check with the product team to update the spec accordingly so that we can generate them correctly. I'll raise a PR to preview these changes and once finalized, they should be available in the next release. Thanks!

tiwarishubham635 avatar May 20 '25 07:05 tiwarishubham635

Please review #1090 and let me know if there are any issues. Thanks!

tiwarishubham635 avatar May 20 '25 07:05 tiwarishubham635

This has been fixed in 5.7.3 thanks!

tiwarishubham635 avatar Jul 14 '25 08:07 tiwarishubham635

@tiwarishubham635 after updating to 5.7.3 I am still seeing this issue. There are lots of Record<string, object> type definitions still in the main branch.

Specifically looking at the lookups.v2.phoneNumbers function, they are still present for the nested object payload values.

codygordon avatar Jul 23 '25 20:07 codygordon

Hi @codygordon! The places that you are pointing to, they are coming as expected. As per my understanding, these attributes except map type entries and thus Record<string, object> can represent the data correctly. Let me know if you are facing any issue while using the library.

tiwarishubham635 avatar Jul 24 '25 06:07 tiwarishubham635

Sure, technically strings, numbers, etc are all objects in JS, but object in the TS context refers specifically to typeof x === 'object', so string, number, boolean, etc are actually not valid types for this, and these fields are mostly those types so delivering this type causes TS errors out of the box if you are mapping them to variables that are actually the known types of the API output without some sort of parsing or type-casting, which shouldn't necessary when the API specification is known.

For example: https://github.com/twilio/twilio-node/blob/34fd66f6f6040d44e3c21eed2a5bcc2908975d20/src/rest/lookups/v2/query.ts#L71

Can you explain why the generator is creating these types? I did a bit of investigation and can't seem to find the source of the specification it's using to generate the type defs or even what version of openapi-generator is being used.

Here is a TS playground that illustrates the issues, hopefully it's enough to help you understand why this is bothersome and that the SDK should provide the correct shape and typing for nested objects and the other types users in this thread identified as they are known and specified, somewhere.

codygordon avatar Jul 24 '25 17:07 codygordon

Thanks for this explanation @codygordon! I have understood the issue. This is coming because the return type of lineTypeIntelligence property inside lookupResult is a Record<string, object> but when you are trying to map it with the interface it expects a string. Hmmm, I can check why this is happening. Meanwhile for you reference, this is the OAI specs that we use for auto-generation of the SDK and this is our generator.

tiwarishubham635 avatar Jul 25 '25 07:07 tiwarishubham635

thanks for the links and investigation @tiwarishubham635

I think I found where the generator has this defined as the type to use: https://github.com/twilio/twilio-oai-generator/commit/ebf96345c0f074f429e910b9544c55490166d678

This is the incorrect type to use for object shapes with unknown value types in TypeScript (the 1st param defines the key type, the 2nd the value). Most correctly it should be Record<string, unknown>, but the easier, albeit less type-safe, user-experience would be to use any instead of unknown as we've discussed earlier in this thread and you used for the Array typing.

As for the spec, it seems that this "unknown object" type is generated for these nested $ref specs – but they don't actually seem to link to any documented specifications in that repo. I wonder if they actually linked to a specification, that the generator could pull them in?

https://github.com/twilio/twilio-oai/blob/bf8a616ddaa83a6a273fadd5f257667c4c425817/spec/json/twilio_lookups_v2.json#L2371-L2394

codygordon avatar Jul 25 '25 14:07 codygordon