Make schema fields emit `prop?: T` (not `prop?: T | undefined`) to align with runtime + exactOptionalPropertyTypes

[AtAFork]

Context

I use the TS compiler option exactOptionalPropertyTypes: true in my codebase (for general TS logic as well as Convex logic) to prevent a lot of footguns with missing attributes vs attributes with a value of undefined, but it's incompatible with the dynamic types that Convex emits for schema with optional fields:

Repro

const schema = defineSchema({
  users: defineTable({
    stripeSubStatus: v.optional(v.string()),
  }),
});

Convex emits stripeSubStatus?: string | undefined, but at runtime a Convex ctx.db.query would never return undefined for a field, it would just omit it (conflicting with exactOptionalPropertyTypes).

Convex patch operation type safety

With the exactOptionalPropertyTypes flag on, TS properly prevents { foo: undefined } unless I explicitly type it that way, which e.g. reduces accidental field drops for ctx.db.patch operations.

My proposal

As a customer I can't force others to use exactOptionalPropertyTypes, and other customers probably already rely on the current prop?: T | undefined.

To maintain backward compatibility, Convex could add an opt-in type parameter on defineSchema (similar to the existing StrictTableNameTypes extends boolean = true), maybe StrictOptionalTypes extends boolean = false, which would mean Convex emits the stricter prop?: T instead.

Cc @thomasballinger since @ianmacartney mentioned Tom has been internally advocating for this for years.

[thomasballinger]

Thanks @AtAFork! I want to understand how much this will rock the ecosystem, i.e. does this break a lot of things in convex-helpers? Actually, does this actually break anything for users who aren't using exactOptionalPropertyTypes, shouldn't be be able to just make this change and tell anyone it does break to disable this flag? I'd be in favor of fixing the types, adding a type test to make sure we don't regress it (probably a demo or private-demos project that uses exactOptionalPropertyTypes).

Let's start by fixing the types, add we can add a flag if we learn about cases this this breaks types for poeple or get complaints.

[AtAFork]

I'd be in favor of fixing the types, adding a type test to make sure we don't regress it

This would be ideal for me, although the opt-in type parameter proposal I mentioned as a short-term fix would also work for me (even if deprecated later once actual types are fixed).

Also want to clarify I'm just a customer and not a Convex employee, and I'm not familiar enough with either convex-backend or convex-helpers to submit a PR for this

[thomasballinger]

@AtAFork do you know of any other places where there are issues with exactOptionalPropertyTypes? We've made fixes before for this. Understood this is a bit involved, but if you try cloning https://github.com/get-convex/convex-js, npm i, making a change, running a build with npm run build, and then in your project running npm i ../convex-js you'll be able to test out a change, would love to know what it takes to fix this. I'll worry about who this breaks, but would be great to know what we'd need to change.

[AtAFork]

Went down a bit of a rabbit hole but couldn't figure it out (and afraid I can't commit more time to digging deeper). I did see | undefined quite a bit in validators.ts and thought removing those would solve it:

• in the asOptional() in abstract class BaseValidator
• in each of the asOptional() in the classes export class VString, export class VBytes< etc
• in export type VOptional

(and also tried removing the | undefined in validator.test.ts which let it build, but still getting the same results)

I also saw mention of exactOptionalPropertyTypes in export type ObjectType in validator.ts

[thomasballinger]

Thanks for looking into it! This is something we want but if we need to duplicate a lot of code to write it two different ways it's a harder sell. I'm adding it to me weekend low pri queue 😄

[AtAFork]

@thomasballinger did you get a chance to look at this?

[thomasballinger]

I didn't, last weekend I was looking at moving pagination out of usePaginatedQuery and into another class which turned out to take a while.

Could you give an example of what you want? I tried this and it seems to work for me, but maybe I'm looking for the wrong thing:

[thomasballinger]

⬆️ this is TypeScript 5.9.2 with a tsconfig.json like

{
  /* This TypeScript project config describes the environment that
   * Convex functions run in and is used to typecheck them.
   * You can modify it, but some settings are required to use Convex.
   */
  "compilerOptions": {
    /* These settings are not required by Convex and can be modified. */
    "allowJs": true,
    "strict": true,
    "moduleResolution": "Bundler",
    "jsx": "react-jsx",
    "skipLibCheck": true,
    "allowSyntheticDefaultImports": true,

    // Stricter Typechecking Options
    "noUncheckedIndexedAccess": true,
    "exactOptionalPropertyTypes": true,

    /* These compiler options are required by Convex */
    "target": "ESNext",
    "lib": ["ES2021", "dom"],
    "forceConsistentCasingInFileNames": true,
    "module": "ESNext",
    "isolatedModules": true,
    "noEmit": true
  },
  "include": ["./**/*"],
  "exclude": ["./_generated"]
}

and a schema like

import { defineSchema, defineTable } from "convex/server";
import { v } from "convex/values";

export default defineSchema({
  messages: defineTable({
    author: v.string(),
    body: v.string(),
    optionalString: v.optional(v.string()),
  }),
});

[thomasballinger]

I'll land this in this repo, once it's in a PR demonstrating what's broken would be helpful

[AtAFork]

Found something interesting that I suspect is a bug: in your example the obj you passed to defineTable was a regular object, and that works for me. But if I pass a v.object directly to it (or a v.union(v.object, v.object)), I get the undesirable prop?: T | undefined

I'd expect for all 3 of these examples below, for ctx.db.query to return a Doc with prop?: T instead of the undesirable prop?: T | undefined):

• yours which works and ctx.db.query returns a Doc with prop?: T

const schema = defineSchema({
  users: defineTable({ stripeSubStatus: v.optional(v.string()) })
})

• my current one which does not work as expected (i.e. ctx.db.query returns a Doc with prop?: T | undefined)

const schema = defineSchema({
  users: defineTable(
    v.object({
      stripeSubStatus: v.optional(v.string())
    })
  )
})

• 3rd one that also doesn't work as expected (i.e. ctx.db.query returns a Doc with prop?: T | undefined)

const schema = defineSchema({
  users: defineTable(
    v.union(
      v.object({
        stripeSubStatus: v.optional(v.string())
      })
    )
  )
})

Hope this helps!

[thomasballinger]

Ah thanks! Great, I'll start by start by documenting this

[AtAFork]

Also looks like an issue with nesting too even when I do example 1 with v.object:

const schema = defineSchema({
  users: defineTable({ stripeSubStatus: v.object({ foo: v.optional(v.string()) }) })
})

Here ctx.db.query returns a Doc with the nested foo as foo?: string | undefined instead of foo?: string

[thomasballinger]

After the expected changes to get things working, just compiling the convex package with exactOptionalPropertyTypes: true fixes this, maybe it's this change to the compiled types

diff -r --exclude=*.map dist-false/esm-types/values/validator.d.ts dist-true/esm-types/values/validator.d.ts
89c89
<     object: <T_2 extends PropertyValidators>(fields: T_2) => VObject<Expand<{ [Property in OptionalKeys<T_2>]?: Exclude<Infer<T_2[Property]>, undefined> | undefined; } & { [Property_1 in Exclude<keyof T_2, OptionalKeys<T_2>>]: Infer<T_2[Property_1]>; }>, T_2, "required", { [Property_2 in keyof T_2]: Property_2 | `${Property_2 & string}.${T_2[Property_2]["fieldPaths"]}`; }[keyof T_2] & string>;
---
>     object: <T_2 extends PropertyValidators>(fields: T_2) => VObject<Expand<{ [Property in OptionalKeys<T_2>]?: Exclude<Infer<T_2[Property]>, undefined>; } & { [Property_1 in Exclude<keyof T_2, OptionalKeys<T_2>>]: Infer<T_2[Property_1]>; }>, T_2, "required", { [Property_2 in keyof T_2]: Property_2 | `${Property_2 & string}.${T_2[Property_2]["fieldPaths"]}`; }[keyof T_2] & string>;

I'm not sure we can flip this setting yet but I'll give it a try!