Side effects when using the spread operator to merge objects

[serkodev]

Currently, if we use the spread operator (...) to merge objects as shown below, it causes side effects and cannot be tree-shaken:

const ParentSchema = v.object({
  foo: v.string(),
});

const LoginSchema = v.object({
  ...ParentSchema.entries, // <- side effect
  email: v.pipe(v.string(), v.email()),
  password: v.pipe(v.string(), v.minLength(8)),
});

Playground
https://stackblitz.com/edit/node-gq8gcqea?file=index.js

The reason is that bundlers like Rollup set propertyReadSideEffects to true by default, which means the spread operator is treated as a side effect.

We can set the treeshake.preset to "smallest" to disable propertyReadSideEffects and make it tree-shakeable but this may cause other issues.

Solution

I wonder if we could have a v.merge function with a @__NO_SIDE_EFFECTS__ annotation for merging objects, making it possible for bundlers to tree-shake it:

// @__NO_SIDE_EFFECTS__
function merge(a, b) {
  return v.object({
    ...a.entries,
    ...b.entries,
  });
}

Usage

const ParentSchema = v.object({
  foo: v.string(),
});

const LoginSchema = v.merge(
  ParentSchema,
  v.object({
    email: v.pipe(v.string(), v.email()),
    password: v.pipe(v.string(), v.minLength(8)),
  })
);

It would be even better if the v.merge function supported an unlimited number of arguments to merge multiple objects at once.

Alternative: Supports input array in v.object

Also, I am wondering if we can support input an array of entries in v.object, like:

// @__NO_SIDE_EFFECTS__
function object(entries, message) {
  if (Array.isArray(entries)) {
    return v.object(
      entries.reduce((acc, current) => ({ ...acc, ...current }), {}),
      message
    );
  }
  // ... original implementation
}

Usage

const ParentSchema = v.object({
  foo: v.string(),
});

const LoginSchema = v.object([
  ParentSchema.entries,
  {
    email: v.pipe(v.string(), v.email()),
    password: v.pipe(v.string(), v.minLength(8)),
  }
]);

It can also be tree-shaken as the spread operator is not used.

---

Thanks @antfu for helping clarify this issue.

[serkodev]

Warning

UPDATE: This solution uses .entries and it might cause tree-shaking to fail.

I made a mergedObject utility for temporary fix in my project. It may not perfect and not supports message parameter yet.

type Merge<A extends object, B extends object> = Omit<A, keyof B> & B

type MergeArray<T extends object[]> =
  T extends [infer First, ...infer Rest]
    ? First extends object
      ? Rest extends object[]
        ? Merge<First, MergeArray<Rest>>
        : First
      : Record<never, never>
    : Record<never, never>

// @__NO_SIDE_EFFECTS__
export function mergedObject<const T extends v.ObjectEntries[]>(
  entries: [...T],
): v.ObjectSchema<MergeArray<T>, undefined> {
  return v.object(
    entries.reduce((acc, entry) => {
      return {
        ...acc,
        ...entry,
      }
    }, {} as MergeArray<T>),
  )
}

// Test
it('merge objects', () => {
  const parentSchema = mergedObject([
    { foo: v.string() },
    { bar: v.number() },
  ])

  const schema = mergedObject([
    parentSchema.entries, // extend parentSchema
    { foo: v.optional(v.string()) }, // override foo type
    { baz: v.number() }, // add baz key
  ])

  const t = v.parse(schema, { bar: 123, baz: 234 })
  expect(t).toEqual({ bar: 123, baz: 234 })
})

[fabian-hiller]

Thanks for reaching out in this regard! The problem with a merge function is that Valibot offers 3 other object schemas besides object. These are looseObject, strictObject and objectWithRest. The challenge is to figure out how to make merge work for object, looseObject and strictObject. Maybe something like merge(schemaFunction, schemaList) will work. Here is an example:

const Schema1 = v.object({ key1: v.string() });
const Schema1 = v.strictObject({ key2: v.number() });

const Schema3 = v.merge(v.strictObject, [Schema1, Schema2]);

Under the hood, merge merges the entries from Schema1 and Schema2 into a new strictObject schema.

[antfu]

I wonder if

const Schema1 = v.object({ key1: v.string() });
const Schema1 = v.strictObject({ key2: v.number() });

const Schema3 = v.strictObject(v.merge(
	Schema1,
    Schema2
))

makes more sense?

[fabian-hiller]

Great idea! That would be similar to our entriesFromList util: https://valibot.dev/api/entriesFromList/

const ObjectSchema = v.object(
  v.entriesFromList(['foo', 'bar', 'baz'], v.string())
);

// { foo: string; bar: string; baz: string; }

Maybe we should call it mergeEntries to be more explicit.

[fabian-hiller]

Are any of you interested in creating a PR? If you don't have time for it, that's fine. We will probably add it after v1 RC is released.

[fabian-hiller]

There are a few design decisions to make:

Should the entire object schema be passed or just the entries? The first option reduces boilerplate when only passing existing schemas and the latter one would reduce boilerplate when extending the entries of existing schemas with additional entries.

const Schema1 = v.object({ key1: v.string() });
const Schema2 = v.strictObject({ key2: v.number() });

// Pass schemas directly
const SchemaX = v.object(
  v.mergeEntries(
    Schema1, // reduces boilerplate here
    Schema2, // reduces boilerplate here
    v.object({ key3: v.boolean() })
  )
);

// Pass entries of schemas
const SchemaX = v.object(
  v.mergeEntries(
    Schema1.entries,
    Schema2.entries, 
    { key3: v.boolean() } // reduces boilerplate here
  )
);

Should the schemas or entries be passed as individual arguments or wrapped in an array?

const Schema1 = v.object({ key1: v.string() });
const Schema2 = v.strictObject({ key2: v.number() });

// With individual arguments
const SchemaX = v.object(v.mergeEntries(Schema1, Schema2));

// With an array of schemas
const SchemaX = v.object(v.mergeEntries([Schema1, Schema2]));

[antfu]

Just adding a note,

// Pass entries of schemas
const SchemaX = v.object(
  v.mergeEntries(
    Schema1.entries,
    Schema2.entries, 
    { key3: v.boolean() } // reduces boilerplate here
  )
);

This approach does not help the tree-shaking, as Schema1.entries is still be treated as accessing properties.

[serkodev]

Thanks @antfu for pointing out that *.entries property access behavior might also cause tree-shaking to fail.

Then I'll vote for "Pass schemas directly" and "With individual arguments".

If use an array as parameter, I would prefer direct pass it to v.object instead of creating v.mergeEntries.

---

Are any of you interested in creating a PR? If you don't have time for it, that's fine. We will probably add it after v1 RC is released.

I’m happy to help, need some time to study the codebase and figure out how I can contribute.

[fabian-hiller]

This is not the final implementation, but it will help you get started. I think the main thing that is missing is the return type that merges the entries and the unit and type tests.

Path: /library/src/utils/mergeEntries/mergeEntries.ts

import type {
  LooseObjectIssue,
  LooseObjectSchema,
  LooseObjectSchemaAsync,
  ObjectIssue,
  ObjectSchema,
  ObjectSchemaAsync,
  ObjectWithRestIssue,
  ObjectWithRestSchema,
  ObjectWithRestSchemaAsync,
  StrictObjectIssue,
  StrictObjectSchema,
  StrictObjectSchemaAsync,
} from '../../schemas/index.ts';
import type {
  BaseIssue,
  BaseSchema,
  BaseSchemaAsync,
  ErrorMessage,
  ObjectEntries,
  ObjectEntriesAsync,
} from '../../types/index.ts';

/**
 * Schema type.
 */
type Schema =
  | LooseObjectSchema<ObjectEntries, ErrorMessage<LooseObjectIssue> | undefined>
  | LooseObjectSchemaAsync<
      ObjectEntriesAsync,
      ErrorMessage<LooseObjectIssue> | undefined
    >
  | ObjectSchema<ObjectEntries, ErrorMessage<ObjectIssue> | undefined>
  | ObjectSchemaAsync<ObjectEntriesAsync, ErrorMessage<ObjectIssue> | undefined>
  | ObjectWithRestSchema<
      ObjectEntries,
      BaseSchema<unknown, unknown, BaseIssue<unknown>>,
      ErrorMessage<ObjectWithRestIssue> | undefined
    >
  | ObjectWithRestSchemaAsync<
      ObjectEntriesAsync,
      | BaseSchema<unknown, unknown, BaseIssue<unknown>>
      | BaseSchemaAsync<unknown, unknown, BaseIssue<unknown>>,
      ErrorMessage<ObjectWithRestIssue> | undefined
    >
  | StrictObjectSchema<
      ObjectEntries,
      ErrorMessage<StrictObjectIssue> | undefined
    >
  | StrictObjectSchemaAsync<
      ObjectEntriesAsync,
      ErrorMessage<StrictObjectIssue> | undefined
    >;

/**
 * Merges the entries of existing object schemas into a new object entries definition.
 *
 * @param schemas The schemas to merge.
 *
 * @returns The object entries.
 */
// @__NO_SIDE_EFFECTS__
export function mergeEntries<const TSchemas extends Schema[]>(
  ...schemas: TSchemas
) {
  const entries = {};
  for (const schema of schemas) {
    Object.assign(entries, schema.entries);
  }
  return entries;
}

This is an old type we used in our previous merge function:

/**
 * Merge entries types.
 */
type MergeEntries<TSchemas extends Schema[]> = TSchemas extends [
  infer TFirstSchema,
]
  ? TFirstSchema extends Schema
    ? TFirstSchema['entries']
    : never
  : TSchemas extends [infer TFirstSchema, ...infer TRestSchemas]
    ? TFirstSchema extends Schema
      ? TRestSchemas extends Schema[]
        ? {
            [TKey in Exclude<
              keyof TFirstSchema['entries'],
              keyof MergeEntries<TRestSchemas>
            >]: TFirstSchema['entries'][TKey];
          } & MergeEntries<TRestSchemas>
        : never
      : never
    : never;

[serkodev]

PR #1023 summited.

I’m considering name this to entriesFromObjects as the parameter is an object instead of entries, and it creates a new entries object from the given objects.

Any better suggestions?