edgedb-js icon indicating copy to clipboard operation
edgedb-js copied to clipboard

Cardinality issue using filter_single without {"strict": true} in tsconfig

Open bavellone opened this issue 2 years ago • 1 comments

Describe the bug When selecting a node using filter_single, I was seeing a Many cardinality type being returned instead of AtMostOne. I'm using a monorepo setup with nested tsconfigs and I wasn't able to reproduce the issue on a fresh project. Finally tracked the problem down and found it went away when "strict": true was specified in compilerOptions in tsconfig.json.

Reproduction

default.esdl

module default {
  type User  {
    required email: str
  }
}

tsconfig.json

{ 
  "compilerOptions": {
    "strict": false
  }
}

test.ts

import { createClient } from 'edgedb'
import e from './types'

const client = createClient('...')
const result = await e.select(e.User, $ => ({
    filter_single: { id: '...' },
})).run(client)

// Hovering over the result above in VSCode shows the below type signature when "strict": false
const result: {
    id: string;
}[]

// Hovering over just the select query will show the query cardinality as Many when "strict": false
$expr_Select<{
  ...
  __cardinality__: Cardinality.Many;
}>

// Adding "strict": true makes it show the correct type
const result: {
    id: string;
} | null

Expected behavior The type signature in VSCode for:

const result = await e.select(e.User, $ => ({
    filter_single: { id: '...' },
})).run(client)

should show:

const result: {
    id: string;
} | null

Versions (please complete the following information):

  • edgedb-js version (e.g. 0.20.10;): 1.3.4
  • Node/Deno version: 20.4.0

bavellone avatar Aug 05 '23 04:08 bavellone

The fact that TS doesn't make strict default to true is absurd. Given the complexity of the types here it's basically impossible to support types with it disabled. This requirement should just be added to docs & readme. This is the same thing zod does: https://github.com/colinhacks/zod#requirements

CarsonF avatar Jun 07 '24 14:06 CarsonF