search

kontent-ai / delivery-sdk-js

Kontent Delivery SDK for Javascript
https://kontent.ai
MIT License
50 stars 34 forks source link

Indexer in IContentItem breaks TypeScript Intellisense #320

Closed ChristopherJennings closed 3 years ago

ChristopherJennings commented 3 years ago

I've noticed that when I'm working with Content Items in TypeScript, I don't get errors on typos or changes to my models because of the Content Item Indexer:

https://github.com/Kentico/kontent-delivery-sdk-js/blob/ff39e1f9c2f5a1e681ef21cb7abca0347c92f1e1/lib/models/item/item-models.ts#L74

The [key: string] is basically swallowing up any property name typos or changes and indicating that properties that don't exist are fine and return any. What's the intent of this and can it be removed?

Enngage commented 3 years ago

@ChristopherJennings this is because elements are placed directly on content item and you would get typescript errors if you tried to access some element because it would not exist on the type. New version should have elements placed under the "elements" property, but the indexer will most probably stay there, just with better typing narrowing it to base element.

ChristopherJennings commented 3 years ago

Ok, so this is for the situation where you are using a generic ContentItem rather than an explicitly created type for a given content item (e.g. using ContentItem rather than the Actor class for an actor content item)?

In that situation, I'd rather have an error that I can fix by casting to the proper type than to have the SDK swallow my typos silently.

I don't like the idea of the SDK making it possible for my to introduce accidental errors that Typescript is meant to solve. If I'm using Typescript, I should be using proper types.

Unless I'm missing or misunderstanding something...

I like that the plan is to keep the elements under the elements sub-object. This keeps it closer to the original payload.

Enngage commented 3 years ago

@ChristopherJennings , this shouldn't be a problem with the new models definition (https://github.com/Kentico/kontent-delivery-sdk-js/blob/vnext/test/browser/setup/models.ts#L3) and the fact that elements are separated (https://github.com/Kentico/kontent-delivery-sdk-js/blob/vnext/lib/models/item/item-models.ts#L77].

There is still an indexer on the elements (https://github.com/Kentico/kontent-delivery-sdk-js/blob/vnext/lib/models/item/item-models.ts#L70), but it's used only when you don't provide your own model.