support for `undefined` in JSON serialization for .patch()

[mmailaender]

Problem Currently, I can pass undefined to a mutation, as the JSON serialization strips it away. At the same time undefined is the way to go to strip values from a document in .patch(). This makes it necessary to write verbose and inconsistent code:

await ctx.runMutation(internal.user.updateUser, {
      userId,
      data: {
        imageId: storageId,
        image: null,
      },
    });

export const updateUser = internalMutation({
  args: {
    userId: v.id("users"),
    data: v.record(v.string(), v.any()),
  },
  handler: async (ctx, args) => {
    const { userId, data } = args;

    // Convert null values to undefined
    const patchData = Object.fromEntries(
      Object.entries(data).map(([key, value]) => [
        key,
        value === null ? undefined : value,
      ])
    );

    return await ctx.db.patch(userId, patchData);
  },
});

Expected Extend the serialization to support undefined, so removing fields from the database becomes consistent clear without manual verbose extra logic.

await ctx.runMutation(internal.user.updateUser, {
      userId,
      data: {
        imageId: storageId,
        image: undefined,
      },
    });

export const updateUser = internalMutation({
  args: {
    userId: v.id("users"),
    data: v.record(v.string(), v.any()),
  },
  handler: async (ctx, args) => {
    const { userId, data } = args;

    return await ctx.db.patch(userId, patchData);
  },

An option could be having a custom serialization with the sentinel value "_undefined". If I pass image: undefined it will be converted behind the scene to image: "_undefined", serialized, parsed and then converted back to image: undefined

[nipunn1313]

Hi! Thanks for the message.

From here, you can see that null is a valid convex value, while undefined is not. https://docs.convex.dev/database/types

Serializing undefined isn't so simple, as we've chosen (for simplicity) to have args, and database documents have the same requirements of being convex documents.

patch specifically uses undefined to represent removals, because null is an actual valid Convex value, and would instead be a replacement.

[franckdsf]

@nipunn1313 I understand the reasoning, but it makes patching entries in the database really painful. Many users are coming from ORMs that support this behavior. Is there a way to handle optional fields at runtime in the patch object ?

Dropping a key, etc..

[franckdsf]

For now I created a helper

export function removeUndefinedKeys<T extends object>(obj: T): Partial<T> {
  return Object.fromEntries(Object.entries(obj).filter(([_, value]) => value !== undefined)) as Partial<T>;
}

[nipunn1313]

I think that sort of helper seems like a good way to go if you want the ergonomics for patching and care a bit less about null values.

If you want to get fancy, you could even write a wrapper around the entire handler function that converts the args at runtime.

[franckdsf]

@nipunn1313 I didn't dig that far into the customFunctions package yet ahah.

But I feel like there’s a conflict between "convex/values" and how undefined is handled in the database. From what I understand, setting a field to undefined during an update effectively removes that field, right?

Let’s take this example:

// I only want to update the email
const userUpdate = mutation({
  args: {
    userId: v.id('users'),
    email: v.optional(v.string()),
    name: v.optional(v.string()),
  },
  handler: async (ctx, { userId, email, name }) => {
    await ctx.db.patch(userId, { email, name })
  }
})

If I call userUpdate({ userId: "XXX", email: "[email protected]" }), it ends up erasing the name field — which feels a bit unexpected.

That’s kind of unwanted from a design perspective. If I wanted to explicitly remove a value, I’d expect to set it to null in the mutation call. But then null just sets the field to null in the DB (which also wouldn't work since the field isn't nullable). So you're left with undefined deleting it and null doing something different 🤔.

[nipunn1313]

Yep that's right. Honestly - from design perspective, db.patch is a bit confusing. It only patches at the top level (not nested fields) and the behavior around undefined being the deletor is tricky.

Using db.replace is more obvious what the behavior is.

Do you have ideas on how to improve the design of db.patch within the constraint of null being a valid Convex value? It's unlikely we'd be able to implement any kind of change until 2.0 (which is not on our radar for a while) for compatibility reasons, but if you want to put out proposals, we'd hear them.

[franckdsf]

I created the helper prismaToConvex

/**
 * Removes undefined values from an object recursively
 * @param obj The object to process
 * @returns A new object without properties that have undefined values
 */
export function removeUndefinedKeys<T extends object>(obj: T): Partial<T> {
  if (Array.isArray(obj)) {
    return obj
      .filter(item => item !== undefined)
      .map(item =>
        item !== null && typeof item === 'object'
          ? removeUndefinedKeys(item as object)
          : item
      ) as unknown as Partial<T>;
  }

  return Object.fromEntries(
    Object.entries(obj)
      .filter(([, value]) => value !== undefined)
      .map(([key, value]) => [
        key,
        value !== null && typeof value === 'object'
          ? removeUndefinedKeys(value as object)
          : value
      ])
  ) as Partial<T>;
}

/**
 * Converts all null values in an object to undefined recursively
 * @param obj The object to process
 * @returns A new object with null values converted to undefined
 */
export function turnNullKeysToUndefined<T extends object>(obj: T): {
  [K in keyof T]: T[K] extends null ? undefined :
  T[K] extends (infer U | null) ? U | undefined :
  T[K] extends object ? ReturnType<typeof turnNullKeysToUndefined<T[K]>> :
  T[K]
} {
  if (Array.isArray(obj)) {
    return obj.map(item =>
      item === null
        ? undefined
        : item !== null && typeof item === 'object'
          ? turnNullKeysToUndefined(item as object)
          : item
      // eslint-disable-next-line @typescript-eslint/no-explicit-any
    ) as any;
  }

  return Object.fromEntries(
    Object.entries(obj).map(([key, value]) => [
      key,
      value === null
        ? undefined
        : value !== null && typeof value === 'object'
          ? turnNullKeysToUndefined(value as object)
          : value
    ])
    // eslint-disable-next-line @typescript-eslint/no-explicit-any
  ) as any;
}

export const prismaToConvex = <T extends object>(obj: T) => turnNullKeysToUndefined(removeUndefinedKeys(obj));

Basically it removes the undefined keys (the ones that we don't want to update), and turn the nullable keys to undefined so that they are removed in the patch operation

const elements = prismaToConvex(args);
await ctx.db.patch(project._id, { ...elements });

[franckdsf]

Yep that's right. Honestly - from design perspective, db.patch is a bit confusing. It only patches at the top level (not nested fields) and the behavior around undefined being the deletor is tricky.

Using db.replace is more obvious what the behavior is.

Do you have ideas on how to improve the design of db.patch within the constraint of null being a valid Convex value? It's unlikely we'd be able to implement any kind of change until 2.0 (which is not on our radar for a while) for compatibility reasons, but if you want to put out proposals, we'd hear them.

Regarding the db.path update, it seems a bit complicated to handle both null and make null behave like undefinedat the same time. We could check whether the table supports null values: if it doesn't, we simply remove the value; otherwise, we treat it as null . However, this might lead to some inconsistency during development. For now, letting the dev handle the format himself seems better 🤔.

[nipunn1313]

one of our go-to strategies is to put helper methods into this library https://github.com/get-convex/convex-helpers - which is pretty popular. This library lets you layer things on top of the base convex package.

A smart-patch method could be a good candidate for something in there - if you want to clean it up + document it well. @ianmacartney would be the person to ask to see if it seems like a good idea.

I don't think (based on this conversation) it feels like a good candidate for the base convex package.

[ianmacartney]

I agree with Nipunn's assessment. undefined isn't technically a valid Convex value (under the hood it translates to an object with that key not present), though for ergonomics it has behavior like you describe - not being present "across the wire", used for deletions on patch, etc. That's the opinionated syntax we've taken and changing it would be a breaking change, so changing the built-in behavior won't happen any time soon.

A couple approaches I'd take:

1. Have an explicit argument to clear values.
2. Use null explicitly in your schema for values where you intend to clear it. This can also catch type errors where you forget to pass the parameter when doing an insert. Downside is you need to migrate undefined(missing) to null.
3. Don't use null except as the sentinel for deleting. You could have a helper like yours above that does undefinedToNull on the client and nullToUndefined on the server, preserving types. This would be explicit for endpoints where you want this behavior, without accidentally deleting fields in others.

A sketch of (1) using convex-helpers liberally:

const fieldsAllowedToUpdate = ["imageId", "image"];

export const updateUser = mutation({
  args: {
    patch: partial(pick(v.doc("users"), fieldsToUpdate)),
    clear: v.array(literals(fieldsToUpdate)),
  },
  handler: async (ctx, args) => {
    const userId = await getAuthUserId(ctx);
    await ctx.db.patch(userId, args.patch);
    for (const field of args.clear) {
      await ctx.db.patch(userId, { [field]: undefined });
    }
  }
});

One aside: I would highly highly discourage having that mutation be public as-is.

1. If anyone knows anyone else's userId (typically not a secret value) they could update any other user's data. I'd have the userId inferred from auth if you do
2. If you have a field like isAdmin in the future, they'd be able to change themselves to an admin.

I'd instead infer the userId from ctx.auth and only specify the fields you allow changing, as I do above.