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!

[thomasballinger]

This seems fine? Here's an alpha with it, curious what this fixes [email protected]

[AtAFork]

I upgraded to [email protected] and looks perfect on my end! I tested it with all the examples above and ctx.db.query properly returns a Doc with prop?: T

[thomasballinger]

Might take a few days to get this out, need to make sure it doesn't mess up convex-helpers or similar but this may be able to be in the next client release: no optimg in ither than whether you use the tsconfig.json flag.

[thomasballinger]

This is in! https://github.com/get-convex/convex-backend/commit/3d89c6ebae5a5ab70700245609f286d0b5475325

Would love to know if this causes any problems for anyone who doesn't use exactOptionalPropertyTypes, try [email protected] and let me know!

[AtAFork]

Much appreciated! Any ballpark estimate on a 1.27.2 release that includes this? Using this with legacy peer deps so convex migrations package doesn't complain (but I want to avoid that flag as soon as I can)

[thomasballinger]

Ballpark ~~this~~ next week? But no guarantees. If you were to try using various convex-helpers that could move this up, the work left to do is to understand how compatible this is (what type errors we see) so we know how to instruct people to upgrade.

[AtAFork]

I use a good amount of functionality from convex-helpers in my codebase and haven't seen any compatibility or type issues with these:

import type { RLSConfig } from 'convex-helpers/server/rowLevelSecurity'
import { customCtx, customCtxAndArgs } from 'convex-helpers/server/customFunctions'
import { wrapDatabaseReader, wrapDatabaseWriter } from 'convex-helpers/server/rowLevelSecurity'
import { zCustomQuery, zCustomMutation } from 'convex-helpers/server/zod'

import type { WithoutSystemFields, UserIdentity } from 'convex/server'
import type { Rules } from 'convex-helpers/server/rowLevelSecurity'
import { zodOutputToConvex } from 'convex-helpers/server/zod'

import { Triggers } from 'convex-helpers/server/triggers'

import { makeUseQueryWithStatus } from 'convex-helpers/react'

One thing I did notice was that when I registered a migration with @convex-dev/migrations via migrations.define(), I had to specify the return type of it:

export const myMigration: RegisteredMutation<
  'internal',
  {
    fn?: string
    cursor?: string | null
    batchSize?: number
    dryRun?: boolean
    next?: string[]
  },
  Promise<any>
> = migrations.define({

in order to prevent the optionals of fn, cursor etc from being | undefined, which without it would otherwise cause a type error when trying to pass to migrations.runner([internal.migrations._backfillAggregatesMigration]:

is not assignable to type 'MigrationFunctionReference'.
  The types of '_args.fn' are incompatible between these types.
    Type 'string | undefined' is not assignable to type 'string'.

[thomasballinger]

That's something we should fix but probably fine for us to release, my main concern is not breaking users who don't use exactOptionalPropertyTypes. Probably hard for you to test since your codebase is written with it

[AtAFork]

Yea fair point, for what it's worth I tried flipping the flag to false just now and didn't see any TS issues pop in the convex folder

[ianmacartney]

I suspect for other packages, they're installing a non-alpha version due to peer dependencies - e.g. npm ls convex would likely have the migrations component using the version of convex without these changes

[AtAFork]

For my previous comment I can confirm I'm using [email protected] across @convex-dev/migrations, @convex-dev/aggregate, and convex-helpers, verified via npm ls convex and module resolution checks and they all resolve to the alpha build (since I've got an .npmrc with legacy-peer-deps=true until this ships in a stable release)