From 5091fe85af6279faf458b6b94373ba8eb751ebd3 Mon Sep 17 00:00:00 2001 From: Gulfaraz Rahman Date: Wed, 31 Jan 2024 17:56:22 +0100 Subject: [PATCH 1/4] fix: array types should not unknown fixes #581 --- src/server/routes/generators/typescript.ts | 3 +- src/server/server.ts | 3 +- src/server/templates/typescript.ts | 74 +++++----------------- test/db/00-init.sql | 7 ++ test/server/typegen.ts | 22 ++++++- 5 files changed, 46 insertions(+), 63 deletions(-) diff --git a/src/server/routes/generators/typescript.ts b/src/server/routes/generators/typescript.ts index e5c69b26..0e2f6b33 100644 --- a/src/server/routes/generators/typescript.ts +++ b/src/server/routes/generators/typescript.ts @@ -111,8 +111,7 @@ export default async (fastify: FastifyInstance) => { functions: functions.filter( ({ return_type }) => !['trigger', 'event_trigger'].includes(return_type) ), - types: types.filter(({ name }) => name[0] !== '_'), - arrayTypes: types.filter(({ name }) => name[0] === '_'), + types, detectOneToOneRelationships, }) }) diff --git a/src/server/server.ts b/src/server/server.ts index 88f2fa50..7137d5fe 100644 --- a/src/server/server.ts +++ b/src/server/server.ts @@ -120,8 +120,7 @@ if (EXPORT_DOCS) { functions: functions!.filter( ({ return_type }) => !['trigger', 'event_trigger'].includes(return_type) ), - types: types!.filter(({ name }) => name[0] !== '_'), - arrayTypes: types!.filter(({ name }) => name[0] === '_'), + types: types!, detectOneToOneRelationships: GENERATE_TYPES_DETECT_ONE_TO_ONE_RELATIONSHIPS, }) ) diff --git a/src/server/templates/typescript.ts b/src/server/templates/typescript.ts index 64793605..aa297f86 100644 --- a/src/server/templates/typescript.ts +++ b/src/server/templates/typescript.ts @@ -19,7 +19,6 @@ export const apply = async ({ relationships, functions, types, - arrayTypes, detectOneToOneRelationships, }: { schemas: PostgresSchema[] @@ -30,7 +29,6 @@ export const apply = async ({ relationships: PostgresRelationship[] functions: PostgresFunction[] types: PostgresType[] - arrayTypes: PostgresType[] detectOneToOneRelationships: boolean }): Promise => { const columnsByTableId = Object.fromEntries( @@ -102,12 +100,9 @@ export type Database = { ...schemaFunctions .filter((fn) => fn.argument_types === table.name) .map((fn) => { - const type = types.find(({ id }) => id === fn.return_type_id) - let tsType = 'unknown' - if (type) { - tsType = pgTypeToTsType(type.name, types, schemas) - } - return `${JSON.stringify(fn.name)}: ${tsType} | null` + const pgType = types.find(({ id }) => id === fn.return_type_id) + const type = pgTypeToTsType(pgType?.name, types, schemas) + return `${JSON.stringify(fn.name)}: ${type} | null` }), ]} } @@ -289,25 +284,9 @@ export type Database = { } const argsNameAndType = inArgs.map(({ name, type_id, has_default }) => { - let type = arrayTypes.find(({ id }) => id === type_id) - if (type) { - // If it's an array type, the name looks like `_int8`. - const elementTypeName = type.name.substring(1) - return { - name, - type: `(${pgTypeToTsType(elementTypeName, types, schemas)})[]`, - has_default, - } - } - type = types.find(({ id }) => id === type_id) - if (type) { - return { - name, - type: pgTypeToTsType(type.name, types, schemas), - has_default, - } - } - return { name, type: 'unknown', has_default } + const pgType = types.find(({ id }) => id === type_id) + const type = pgTypeToTsType(pgType?.name, types, schemas) + return { name, type, has_default } }) return `{ @@ -322,20 +301,9 @@ export type Database = { const tableArgs = args.filter(({ mode }) => mode === 'table') if (tableArgs.length > 0) { const argsNameAndType = tableArgs.map(({ name, type_id }) => { - let type = arrayTypes.find(({ id }) => id === type_id) - if (type) { - // If it's an array type, the name looks like `_int8`. - const elementTypeName = type.name.substring(1) - return { - name, - type: `(${pgTypeToTsType(elementTypeName, types, schemas)})[]`, - } - } - type = types.find(({ id }) => id === type_id) - if (type) { - return { name, type: pgTypeToTsType(type.name, types, schemas) } - } - return { name, type: 'unknown' } + const pgType = types.find(({ id }) => id === type_id) + const type = pgTypeToTsType(pgType?.name, types, schemas) + return { name, type } }) return `{ @@ -364,11 +332,7 @@ export type Database = { // Case 3: returns base/composite/enum type. const type = types.find(({ id }) => id === return_type_id) - if (type) { - return pgTypeToTsType(type.name, types, schemas) - } - - return 'unknown' + return pgTypeToTsType(type?.name, types, schemas) })()})${is_set_returning_function ? '[]' : ''} }` ) @@ -398,15 +362,9 @@ export type Database = { ({ name, attributes }) => `${JSON.stringify(name)}: { ${attributes.map(({ name, type_id }) => { - const type = types.find(({ id }) => id === type_id) - if (type) { - return `${JSON.stringify(name)}: ${pgTypeToTsType( - type.name, - types, - schemas - )}` - } - return `${JSON.stringify(name)}: unknown` + const pgType = types.find(({ id }) => id === type_id) + const type = pgTypeToTsType(pgType?.name, types, schemas) + return `${JSON.stringify(name)}: ${type}` })} }` ) @@ -506,11 +464,13 @@ export type Enums< // TODO: Make this more robust. Currently doesn't handle range types - returns them as unknown. const pgTypeToTsType = ( - pgType: string, + pgType: string | undefined, types: PostgresType[], schemas: PostgresSchema[] ): string => { - if (pgType === 'bool') { + if (pgType === undefined) { + return 'unknown' + } else if (pgType === 'bool') { return 'boolean' } else if (['int2', 'int4', 'int8', 'float4', 'float8', 'numeric'].includes(pgType)) { return 'number' diff --git a/test/db/00-init.sql b/test/db/00-init.sql index b625ae03..e0e7d3d5 100644 --- a/test/db/00-init.sql +++ b/test/db/00-init.sql @@ -3,6 +3,8 @@ -- Tables for testing CREATE TYPE public.user_status AS ENUM ('ACTIVE', 'INACTIVE'); +CREATE TYPE array_type AS (my_text_array text[]); + CREATE TABLE public.users ( id bigint GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY, name text, @@ -76,6 +78,11 @@ $$ select $1.details_length > 20; $$ language sql stable; +create function public.details_words(public.todos) returns text[] as +$$ +select string_to_array($1.details, ' '); +$$ language sql stable; + create extension postgres_fdw; create server foreign_server foreign data wrapper postgres_fdw options (host 'localhost', port '5432', dbname 'postgres'); create user mapping for postgres server foreign_server options (user 'postgres', password 'postgres'); diff --git a/test/server/typegen.ts b/test/server/typegen.ts index 853764f1..b2d6459c 100644 --- a/test/server/typegen.ts +++ b/test/server/typegen.ts @@ -79,6 +79,7 @@ test('typegen', async () => { blurb_varchar: string | null details_is_long: boolean | null details_length: number | null + details_words: string[] | null } Insert: { details?: string | null @@ -306,6 +307,12 @@ test('typegen', async () => { } Returns: number } + details_words: { + Args: { + "": unknown + } + Returns: string[] + } function_returning_row: { Args: Record Returns: { @@ -366,7 +373,9 @@ test('typegen', async () => { user_status: "ACTIVE" | "INACTIVE" } CompositeTypes: { - [_ in never]: never + array_type: { + my_text_array: string[] + } } } } @@ -539,6 +548,7 @@ test('typegen w/ one-to-one relationships', async () => { blurb_varchar: string | null details_is_long: boolean | null details_length: number | null + details_words: string[] | null } Insert: { details?: string | null @@ -778,6 +788,12 @@ test('typegen w/ one-to-one relationships', async () => { } Returns: number } + details_words: { + Args: { + "": unknown + } + Returns: string[] + } function_returning_row: { Args: Record Returns: { @@ -838,7 +854,9 @@ test('typegen w/ one-to-one relationships', async () => { user_status: "ACTIVE" | "INACTIVE" } CompositeTypes: { - [_ in never]: never + array_type: { + my_text_array: string[] + } } } } From 53f209d49986d61e841d7a39af4aa263575b478c Mon Sep 17 00:00:00 2001 From: Gulfaraz Rahman Date: Wed, 31 Jan 2024 17:56:35 +0100 Subject: [PATCH 2/4] chore: update test random ids --- test/lib/views.ts | 24 ++++++++++++------------ test/server/indexes.ts | 10 +++++----- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/test/lib/views.ts b/test/lib/views.ts index 1d421c73..275eb4a3 100644 --- a/test/lib/views.ts +++ b/test/lib/views.ts @@ -15,7 +15,7 @@ test('list', async () => { "default_value": null, "enums": [], "format": "int8", - "id": "16420.1", + "id": "16423.1", "identity_generation": null, "is_generated": false, "is_identity": false, @@ -26,7 +26,7 @@ test('list', async () => { "ordinal_position": 1, "schema": "public", "table": "todos_view", - "table_id": 16420, + "table_id": 16423, }, { "check": null, @@ -35,7 +35,7 @@ test('list', async () => { "default_value": null, "enums": [], "format": "text", - "id": "16420.2", + "id": "16423.2", "identity_generation": null, "is_generated": false, "is_identity": false, @@ -46,7 +46,7 @@ test('list', async () => { "ordinal_position": 2, "schema": "public", "table": "todos_view", - "table_id": 16420, + "table_id": 16423, }, { "check": null, @@ -55,7 +55,7 @@ test('list', async () => { "default_value": null, "enums": [], "format": "int8", - "id": "16420.3", + "id": "16423.3", "identity_generation": null, "is_generated": false, "is_identity": false, @@ -66,7 +66,7 @@ test('list', async () => { "ordinal_position": 3, "schema": "public", "table": "todos_view", - "table_id": 16420, + "table_id": 16423, }, ], "comment": null, @@ -112,7 +112,7 @@ test('retrieve', async () => { "default_value": null, "enums": [], "format": "int8", - "id": "16420.1", + "id": "16423.1", "identity_generation": null, "is_generated": false, "is_identity": false, @@ -123,7 +123,7 @@ test('retrieve', async () => { "ordinal_position": 1, "schema": "public", "table": "todos_view", - "table_id": 16420, + "table_id": 16423, }, { "check": null, @@ -132,7 +132,7 @@ test('retrieve', async () => { "default_value": null, "enums": [], "format": "text", - "id": "16420.2", + "id": "16423.2", "identity_generation": null, "is_generated": false, "is_identity": false, @@ -143,7 +143,7 @@ test('retrieve', async () => { "ordinal_position": 2, "schema": "public", "table": "todos_view", - "table_id": 16420, + "table_id": 16423, }, { "check": null, @@ -152,7 +152,7 @@ test('retrieve', async () => { "default_value": null, "enums": [], "format": "int8", - "id": "16420.3", + "id": "16423.3", "identity_generation": null, "is_generated": false, "is_identity": false, @@ -163,7 +163,7 @@ test('retrieve', async () => { "ordinal_position": 3, "schema": "public", "table": "todos_view", - "table_id": 16420, + "table_id": 16423, }, ], "comment": null, diff --git a/test/server/indexes.ts b/test/server/indexes.ts index 94937046..9de3aef1 100644 --- a/test/server/indexes.ts +++ b/test/server/indexes.ts @@ -18,7 +18,7 @@ test('list indexes', async () => { "class": "3124", "collation": "0", "comment": null, - "id": 16396, + "id": 16399, "index_attributes": [ { "attribute_name": "id", @@ -42,14 +42,14 @@ test('list indexes', async () => { "number_of_key_attributes": 1, "options": "0", "schema": "public", - "table_id": 16390, + "table_id": 16393, } ` ) }) test('retrieve index', async () => { - const res = await app.inject({ method: 'GET', path: '/indexes/16396' }) + const res = await app.inject({ method: 'GET', path: '/indexes/16399' }) const index = res.json() expect(index).toMatchInlineSnapshot( ` @@ -59,7 +59,7 @@ test('retrieve index', async () => { "class": "3124", "collation": "0", "comment": null, - "id": 16396, + "id": 16399, "index_attributes": [ { "attribute_name": "id", @@ -83,7 +83,7 @@ test('retrieve index', async () => { "number_of_key_attributes": 1, "options": "0", "schema": "public", - "table_id": 16390, + "table_id": 16393, } ` ) From 1a76c967f724f1140ba95fd9185dd0013a5dae66 Mon Sep 17 00:00:00 2001 From: Bobbie Soedirgo Date: Wed, 6 Mar 2024 17:00:48 +0800 Subject: [PATCH 3/4] chore: keep pgType arg non-undefined It should be the resposibility of the caller to make sure the argument is defined --- src/server/templates/typescript.ts | 50 +++++++++++++++++++----------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/src/server/templates/typescript.ts b/src/server/templates/typescript.ts index aa297f86..56b118ed 100644 --- a/src/server/templates/typescript.ts +++ b/src/server/templates/typescript.ts @@ -100,9 +100,12 @@ export type Database = { ...schemaFunctions .filter((fn) => fn.argument_types === table.name) .map((fn) => { - const pgType = types.find(({ id }) => id === fn.return_type_id) - const type = pgTypeToTsType(pgType?.name, types, schemas) - return `${JSON.stringify(fn.name)}: ${type} | null` + const type = types.find(({ id }) => id === fn.return_type_id) + let tsType = 'unknown' + if (type) { + tsType = pgTypeToTsType(type.name, types, schemas) + } + return `${JSON.stringify(fn.name)}: ${tsType} | null` }), ]} } @@ -284,9 +287,12 @@ export type Database = { } const argsNameAndType = inArgs.map(({ name, type_id, has_default }) => { - const pgType = types.find(({ id }) => id === type_id) - const type = pgTypeToTsType(pgType?.name, types, schemas) - return { name, type, has_default } + const type = types.find(({ id }) => id === type_id) + let tsType = 'unknown' + if (type) { + tsType = pgTypeToTsType(type.name, types, schemas) + } + return { name, type: tsType, has_default } }) return `{ @@ -301,9 +307,12 @@ export type Database = { const tableArgs = args.filter(({ mode }) => mode === 'table') if (tableArgs.length > 0) { const argsNameAndType = tableArgs.map(({ name, type_id }) => { - const pgType = types.find(({ id }) => id === type_id) - const type = pgTypeToTsType(pgType?.name, types, schemas) - return { name, type } + const type = types.find(({ id }) => id === type_id) + let tsType = 'unknown' + if (type) { + tsType = pgTypeToTsType(type.name, types, schemas) + } + return { name, type: tsType } }) return `{ @@ -330,9 +339,13 @@ export type Database = { }` } - // Case 3: returns base/composite/enum type. + // Case 3: returns base/array/composite/enum type. const type = types.find(({ id }) => id === return_type_id) - return pgTypeToTsType(type?.name, types, schemas) + if (type) { + return pgTypeToTsType(type.name, types, schemas) + } + + return 'unknown' })()})${is_set_returning_function ? '[]' : ''} }` ) @@ -362,9 +375,12 @@ export type Database = { ({ name, attributes }) => `${JSON.stringify(name)}: { ${attributes.map(({ name, type_id }) => { - const pgType = types.find(({ id }) => id === type_id) - const type = pgTypeToTsType(pgType?.name, types, schemas) - return `${JSON.stringify(name)}: ${type}` + const type = types.find(({ id }) => id === type_id) + let tsType = 'unknown' + if (type) { + tsType = pgTypeToTsType(type.name, types, schemas) + } + return `${JSON.stringify(name)}: ${tsType}` })} }` ) @@ -464,13 +480,11 @@ export type Enums< // TODO: Make this more robust. Currently doesn't handle range types - returns them as unknown. const pgTypeToTsType = ( - pgType: string | undefined, + pgType: string, types: PostgresType[], schemas: PostgresSchema[] ): string => { - if (pgType === undefined) { - return 'unknown' - } else if (pgType === 'bool') { + if (pgType === 'bool') { return 'boolean' } else if (['int2', 'int4', 'int8', 'float4', 'float8', 'numeric'].includes(pgType)) { return 'number' From a6f534b02037252d38f9309dcca03bff8e955177 Mon Sep 17 00:00:00 2001 From: Bobbie Soedirgo Date: Wed, 6 Mar 2024 17:11:20 +0800 Subject: [PATCH 4/4] chore: clarify type name --- package.json | 2 +- test/db/00-init.sql | 2 +- test/server/typegen.ts | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index bfb71eb5..cee9928b 100644 --- a/package.json +++ b/package.json @@ -29,7 +29,7 @@ "db:clean": "cd test/db && docker-compose down", "db:run": "cd test/db && docker-compose up --detach && sleep 5", "test:run": "vitest run", - "test:update": "run-s db:clean db:run && vitest run && run-s db:clean" + "test:update": "run-s db:clean db:run && vitest run --update && run-s db:clean" }, "engines": { "node": ">=20", diff --git a/test/db/00-init.sql b/test/db/00-init.sql index e0e7d3d5..02aa912b 100644 --- a/test/db/00-init.sql +++ b/test/db/00-init.sql @@ -3,7 +3,7 @@ -- Tables for testing CREATE TYPE public.user_status AS ENUM ('ACTIVE', 'INACTIVE'); -CREATE TYPE array_type AS (my_text_array text[]); +CREATE TYPE composite_type_with_array_attribute AS (my_text_array text[]); CREATE TABLE public.users ( id bigint GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY, diff --git a/test/server/typegen.ts b/test/server/typegen.ts index b2d6459c..05565038 100644 --- a/test/server/typegen.ts +++ b/test/server/typegen.ts @@ -373,7 +373,7 @@ test('typegen', async () => { user_status: "ACTIVE" | "INACTIVE" } CompositeTypes: { - array_type: { + composite_type_with_array_attribute: { my_text_array: string[] } } @@ -854,7 +854,7 @@ test('typegen w/ one-to-one relationships', async () => { user_status: "ACTIVE" | "INACTIVE" } CompositeTypes: { - array_type: { + composite_type_with_array_attribute: { my_text_array: string[] } }