BuilderIO / BuilderIO/builder

Unavailability of Generic Type for builder.get() and builder.getAll() functions

Open
#4,015 0 comments 2 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
8.8k
Forks
1.2k
Avg merge
1d 6h
Merged PRs (30d)
17

Description

/* This is my first ever contribution to GH, so forgive my non-standard way of Starting a Discussion. Thanks in advance! */

Issue in `@builder/sdk`

TLDR: `builder.get()` and `builder.getAll()` should accept a Generic Type to determine the Object shape of Custom Component, content models, etc. to avoid type assertion hackaround downstream.

Background - In our organization, we use a lot of custom components, content models in NextJS Framework, and `.get()` and `.getAll()` functions are used all the time to get the data for these components. These methods return `BehaviorSubject` and `Promise` respectively.

The first 'real' rule of TS in production environments is - "No matter what happens, do not use '`any`'. We did not go through all the hardships of setting up Typescript and related protections, and for a developer to just ignore all of it by using '`any`'". (I know we can force a 'compilation' error, but that is not the point here).

Keeping that in mind, what bugs me a lot is that Builder for some reason does not provide any mechanism to determine the shape of the return value of those two functions mentioned earlier, and defaults it to '`any`'. PFB the typings -

@builder.io\sdk\dist\src\builder.class.d.ts -
1) `get(modelName: string, options?: GetContentOptions & {
req?: IncomingMessage;
res?: ServerResponse;
apiKey?: string;
authToken?: string;
}): BehaviorSubject;`

2) `getAll(modelName: string, options?: GetContentOptions & {
req?: IncomingMessage;
res?: ServerResponse;
apiKey?: string;
authToken?: string;
}): Promise`

Here, `BuilderContent['data']` has a placeholder `[key: string]: any;`

Now, my question is - why? Why does this defaults to any, and why have they not just given us the ability to provide custom types via a Generic Type to BuilderContent, because at the end of the day, both `.get()` and `.getAll()` do return either a `BuilderContent` or a `BuilderContent[]`.

If you dig deep in builder\packages\core\src\builder.class.ts, which is not that hard to do, you will see that they explicitly have used `any` in their logic. It does makes sense in the SDK context, as they have no idea what the return object is, and they should not care. But, for us, the end users of this SDK have to suffer through setting up custom types to resolve this `any` to something meaningful which can be used downstream.. well.. meaningfully.

BDW, this is what I had to create for a meaningful intellisense, and use it to define a variable holding the return value of .get() -

`type BuilderResponse = Omit & {
data?: Model & Omit;
}`

And it still does not work as it is supposed to.

This can easily be avoided if Builder simply provides ability to add Generic return types to these functions, and similarly to other functions which return '`any`', because as mentioned earlier, they return BuilderContent in one way or another.

Maybe I am totally out of the line for mentioning this issue, and maybe I am the only one who does not like hackarounds of inferred types and custom type assertions, but as a developer using Builder SDK, I would like to see this fixed on a fundamental level, and I am ready to contribute to this, if given the chance.

\<\\>

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with packages/core/src/builder.class.ts and compare its get() and getAll() entry points with the published builder.class.d.ts signatures mentioned in the issue. Trace the BuilderContent return types and confirm that callers can supply a generic shape for model data; done means the declarations expose typed results without relying on any or downstream assertions.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
api
Issue type
Feature
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.