maltejur / directus-extension-generate-types

Create types for your directus project in your favourite language.
Other
199 stars 24 forks source link

Update generated Typescript schema for Directus SDK v11 #31

Closed stuikomma closed 1 year ago

stuikomma commented 1 year ago

The latest release of the directus SDK version 11 requires the collections to be of array types. See the example code taken from https://docs.directus.io/guides/sdk/getting-started.html#creating-a-composable-client

import { createDirectus } from '@directus/sdk';
import { rest } from '@directus/sdk/rest';
import { graphql } from '@directus/sdk/graphql';

interface Article {
    id: number;
    title: string;
    content: string;
}

interface Schema {
    articles: Article[]; // <-- Note the array
}

// The extension currently generates types like this
// interface Schema {
//  articles: Article; // <-- Not an array
//}

// Client with REST support
const client = createDirectus<Schema>('http://directus.example.com').with(rest());

// Client with GraphQL support
const client = createDirectus<Schema>('http://directus.example.com').with(graphql());

If the array is missing, you will get type errors when making a call like this:

client.request(readItems('articles'));

I propose adding a checkbox [ ] Generate types for SDK v11.

Thanks for making this extension!

stuikomma commented 1 year ago

I found another issue: Optional fields should be be typed optionalField: ValueType | null instead of optionalField?: ValueType.

When using the current version, optional fields become required and vice-versa.

maltejur commented 1 year ago

Are you talking about the export type CustomDirectusTypes being generated? If that is required, adding a checkbox (probably enabled by default) sounds like a sensible change.

I found another issue: Optional fields should be be typed optionalField: ValueType | null instead of optionalField?: ValueType.

I would argue that is pretty much up to personal preference. Or are there any objective reasons it should be done this way and not with the ?? Of couse we can also add a checkbox to configure this behavior.

stuikomma commented 1 year ago

Are you talking about the export type CustomDirectusTypes being generated? If that is required, adding a checkbox (probably enabled by default) sounds like a sensible change.

Yes, that's what I meant.

I would argue that is pretty much up to personal preference. Or are there any objective reasons it should be done this way and not with the ?? Of couse we can also add a checkbox to configure this behavior.

The types in the new directus SDK do not resolve properly with ?. They can be resolved by using | null instead of ?. Here is an example use case I have in my projects:

import { createDirectus, readItems, rest } from '@directus/sdk';

interface Article {
    id: number;
    title: string;
    content: string;
    author: Author | null; // <-- this works
    // author?: Author; // <-- this doesn't work
}

interface Author {
    first_name: string;
}

interface Schema {
    articles: Article[]; // <-- Note the array
    authors: Author[];
}

// Client with REST support
const client = createDirectus<Schema>('http://directus.example.com').with(
    rest(),
);

function fetchArticles() {
    return client.request(
        readItems('articles', {
            fields: ['id', 'title', { author: ['first_name'] }],
        }),
    );
}

const mockArticles = [
    { id: 1, title: 'Mock Title', author: null },
    //                              ^ errors if you choose `?`
    // { id: 2, title: 'Some Title', author: undefined },
    //                                ^ this errors in both cases, so this is not a workaround
    {
        id: 3,
        title: 'Another Title',
        author: {
            first_name: 'Edgar',
        },
    },
] satisfies Awaited<ReturnType<typeof fetchArticles>>;
maxbec commented 1 year ago

Is there any progress regarding the array types implementation?

maltejur commented 1 year ago

Implemented in https://github.com/maltejur/directus-extension-generate-types/commit/84afcd2f2d041a12e6dee17f69334b361f18bade

Can anybody try it out and check if I changed the right thing? I currently don't actively use Directus anymore, so it isn't that easy for me.

index.js

JeremieLeblanc commented 1 year ago

Implemented in 84afcd2

Can anybody try it out and check if I changed the right thing? I currently don't actively use Directus anymore, so it isn't that easy for me.

index.js

I have tested it quickly and it does seem to work except for the fields that add "any[]"

An example of this is translations fields like the following:

export type Articles = {
  date_created?: string | null;
  date_updated?: string | null;
  id: string;
  status: string;
  translations: any[] | ArticlesTranslations[];
  user_created?: string | DirectusUsers | null;
  user_updated?: string | DirectusUsers | null;
};

This seems to break the deep filtering.

As soon as I removed all of the "any" in the types, the errors went away.

JeremieLeblanc commented 1 year ago

One more thing.

Singletons should not me marked as arrays in the CustomDirectusTypes.

maltejur commented 1 year ago

Singletons should not me marked as arrays in the CustomDirectusTypes.

Ah yes, fixed.

I have tested it quickly and it does seem to work except for the fields that add "any[]"

Is that related to the original issue or something else? I originally wrote it like that because, depending on the query, the API either returned a list of primary keys (of which we do not know the type), or complete objects of the type we know.

marquetd commented 1 year ago

Sorry another thing, don't forget to update the example when " Generate Types for Directus SDK >= v11" is checked :

Capture d’écran du 2023-09-05 15-18-33

maltejur commented 1 year ago

@marquetd what exactly needs to change there?

marquetd commented 1 year ago

When the checkbox is checked :

image

unchecked :

image

maltejur commented 1 year ago

Thanks, I've updated the example code in https://github.com/maltejur/directus-extension-generate-types/commit/95c53f649c60f161089a09b7357e1bc2d7df3e6c

maltejur commented 1 year ago

https://github.com/maltejur/directus-extension-generate-types/releases/tag/0.5.0

D3VL-Jack commented 1 year ago

@maltejur

Ah yes, fixed.

In 0.5.0, Singletons are still returning as arrays.

export type CustomDirectusTypes = {
    // ...
    global_variables: GlobalVariables[];
    // ...
};

image