Bug: coercion on a modelcore subclass field always throws TypeValidationError

[DavidMANZI-093]

coerce: true on a modelcore subclass field always throws TypeValidationError

modelcore on v1.6.0, base.ts around line 611 in validateType.

Description

The bug was discovered while working with modelcore tand prisma-modelcore. Its generator (prisma-modelcore's) emits relation fields as { type: RelatedModel, coerce: true, optional: true }, so when a query hydrates a row that includes a relation, modelcore is asked to coerce a plain object into a nested modelcore instance, and it throws.

The behavior reproduces with modelcore alone, with no Prisma involved:

import Base from '@bufferpunk/modelcore';

class Inner extends Base {
  static schema = { x: { type: String, min: 1 } } as const;
}

class Outer extends Base {
  static schema = {
    items: { type: Array, coerce: true, values: { type: Inner, coerce: true } },
  } as const;
}

new Outer({ items: [{ x: 'hi' }] });
// throws: TypeValidationError: Invalid type at 'items[0]', expected Inner, got Inner

The error message reads expected Inner, got Inner, which indicates the value is already the correct type and the type check itself passes. The throw originates from a different clause.

In validateType, after coercion the code performs:

if (!isOfType() || isNaN(value))
  throw buildError(TypeValidationError, `Invalid type at '${path}', expected ${conf.type.name}, got ${value.constructor.name}`, ...);

The isNaN(value) clause is the cause. isNaN() coerces its argument to a Number before testing, and Number(anyObjectInstance) evaluates to NaN. Therefore, for any object-typed field (a modelcore subclass, a plain Object, and so on), isNaN(value) is always true, and the guard rejects an otherwise valid, correctly typed instance.

The intent is visible in the surrounding code and history. The coercion comment block directly above the throw already acknowledges that JavaScript constructors are "permissive in dangerous ways":

// Attempt to coerce the value to the correct type if possible. Valuable for date strings from a json for example
if (conf.coerce) value = !unionTypes ? new conf.type(value) : (() => {
  // dangerous but necessary! Javascript will most probably coerce in an invalid way
  // like `new Array({}) = [{}]` instead of throwing so we can check the next type.
  // We have to accept the language's downsides here.
  ...

The isNaN check was a safety net for precisely those dangerous coercions that succeed at the constructor level but yield garbage: new Number(undefined) produces a real Number instance (constructor === Number passes) but a NaN payload; new Date("not-a-date") produces an Invalid Date whose getTime() is NaN. For Number/Date, that NaN is the meaningful "this coercion is invalid" signal, so the check was simple and, at the time, sufficient for the scalar types it was written against.

The gap is that the guard was never scoped to those types. It is applied unconditionally to every conf.type, so it also fires for object instances where NaN carries no meaning. It covers the numeric/date case but not the full range of possible conf.type occurrences (custom classes, nested objects, and others).

Suggested fixes

Fix A (my take...) - scope the guard to the types it was meant for (minimal, preserves intent):

const isNumericCoercion = conf.type === Number || conf.type === Date;
if (!isOfType() || (isNumericCoercion && isNaN(value as any)))
  throw buildError(TypeValidationError, ...);

This keeps the NaN safety net for Number/Date while allowing object, string, boolean, and custom-class coercion to pass.

Fix B - remove the bare isNaN and rely solely on isOfType() after coercion:

if (!isOfType())
  throw buildError(TypeValidationError, ...);

This is cleaner but loses the NaN protection (for example, new Number("abc") would then pass the type check).

[Scion-Kin]

Thanks for this valuable observation and input.

I have fixed the issue, and released a version 1.6.1 with the improvements.