fix: prevent enumerable undefined properties in InputType instances (complete fix for #475)

[AviBueno]

Description

Fixes #475 (and completes the partial fix from commit 6f64ac4)

TypeGraphQL's @InputType() decorated classes were creating instances where ALL optional fields became enumerable properties set to undefined, even when not provided in the GraphQL query. This caused issues when using Object.keys(), for...in, or spread operators on input objects.

The Problem

Before This Fix

@InputType()
class FilterInput {
  @Field(() => String)
  tenantId: string;

  @Field(() => String, { nullable: true })
  optionalField?: string;
}

// GraphQL Query: { filter: { tenantId: "123" } }

// In resolver:
Object.keys(filter)  // ['tenantId', 'optionalField'] ❌
filter.optionalField // undefined (but enumerable!)

After This Fix

Object.keys(filter)  // ['tenantId'] ✅
filter.optionalField // undefined (not enumerable)

Root Cause

In src/helpers/types.ts, the convertToType() function was using:

return Object.assign(new Target(), data);

Calling new Target() invokes the constructor, which initializes ALL @Field() decorated properties as enumerable with undefined values. Object.assign() only overwrites properties present in data, leaving unprovided optional fields as enumerable undefined values.

The Solution

The fix now:

1. Calls the constructor to initialize instance fields (preserves existing behavior for non-GraphQL instance properties)
2. Removes undefined properties that weren't provided in the input data
3. Assigns the provided data via Object.assign()

// Create instance by calling constructor to initialize instance fields
const instance = new (Target as any)();

// Remove undefined properties that weren't provided in the input data
// This prevents optional @Field() decorated properties from being enumerable
for (const key of Object.keys(instance)) {
  if (instance[key] === undefined && !(key in data)) {
    delete instance[key];
  }
}

return Object.assign(instance, data);

This creates an object with only the properties from the GraphQL input as enumerable, while preserving proper instance field initialization.

Changes Made

Modified Files

• src/helpers/types.ts - Updated convertToType() function (added 9 lines)
• tests/functional/inputtype-enumerable-properties.ts - Added comprehensive tests

Code Changes

 export function convertToType(Target: Function, data: any) {
   if (data == null) {
     return data;
   }
   if (Target instanceof GraphQLScalarType) {
     return data;
   }
   if (simpleTypes.includes(data.constructor)) {
     return data;
   }
   if (data instanceof Target) {
     return data;
   }
   if (Array.isArray(data)) {
     return data.map(item => convertToType(Target, item));
   }
-  return Object.assign(new Target(), data);
+  // Create instance by calling constructor to initialize instance fields
+  const instance = new (Target as any)();
+
+  // Remove undefined properties that weren't provided in the input data
+  // This prevents optional @Field() decorated properties from being enumerable
+  for (const key of Object.keys(instance)) {
+    if (instance[key] === undefined && !(key in data)) {
+      delete instance[key];
+    }
+  }
+
+  return Object.assign(instance, data);
 }

Testing

New Test Cases

Added comprehensive test file tests/functional/inputtype-enumerable-properties.ts with 4 test cases:

1. Basic optional fields - Verifies Object.keys() only returns provided fields
2. Nested InputTypes - Ensures nested optional types aren't enumerable
3. Provided optional fields - Confirms provided fields ARE enumerable
4. Explicit null values - Verifies null (not undefined) is preserved

Existing Tests

All existing tests pass (478 passed, 1 todo), confirming backward compatibility including tests that rely on instance field initialization.

Real-World Impact

This bug affects production code that:

1. Iterates over input properties:

   const fields = Object.keys(filter);
   for (const field of fields) {
     // Unexpectedly iterates over undefined properties
   }

2. Uses property existence checks:

   if ('optionalField' in input) {
     // Incorrectly returns true even when not provided
   }

3. Spreads input objects:

   const combined = { ...filter };
   // Includes undefined properties

4. Deals with GraphQL client metadata:

   • Apollo and other clients add __typename
   • Combined with enumerable undefined properties, causes validation errors

Related Issues and History

• Original Report: InputType returns undefined properties #475 (November 2019) - "InputType returns undefined properties"
• Partial Fix: Commit 6f64ac4 (November 25, 2019) - Fixed argument conversion but not convertToType()
• Incomplete Fix Noted: March 23, 2022 - User @forsigner identified that convertToType() still had the issue
• This PR: Completes the fix by addressing the root cause

Breaking Changes

None. This change makes the behavior more correct without breaking existing functionality:

• ✅ Property access (e.g., input.optionalField) works identically
• ✅ Type checking unchanged
• ✅ Validation behavior unchanged
• ✅ All existing tests pass (including instance field initialization)
• ✅ Only affects enumeration (Object.keys, for...in, spread)

Verification

The fix has been tested with:

• ✅ Simple InputTypes with optional fields
• ✅ Nested InputTypes
• ✅ Array inputs
• ✅ GraphQL client metadata (__typename)
• ✅ Complex filter objects with multiple optional fields
• ✅ Instance field initialization (Math.random() fields in tests)
• ✅ All existing TypeGraphQL tests (478 passed)
• ✅ Production codebase via patch-package

[MichalLytek]

I can't imagine a better prepared PR ❤️

Will try to take a look at the changed and tests today 🙌

[AviBueno]

Thanks @MichalLytek! In full transparency I did have AI help with this. Still, I did dive deep to find the real root cause, and aimed to be as elaborate as possible.

This issue surfaced for me after upgrading to Prisma 6 and using the fix by @ahmedhosnypro (mentioned in MichalLytek/typegraphql-prisma#475).

Note on Implementation Approach

The suggested solution in the current PR is a second attempt.
Initially, I tried a simpler fix using Object.create() to bypass the constructor entirely:

- return Object.assign(new Target(), data);
+ // Create instance without calling constructor to avoid initializing
+ // all decorated properties as enumerable undefined values
+ const instance = Object.create(Target.prototype);
+ return Object.assign(instance, data);

However, this broke existing tests because it prevented all constructor initialization, including legitimate instance fields that aren't GraphQL fields (e.g., instanceField = Math.random() used in tests).

The suggested solution here calls the constructor but removes only the undefined properties that weren't provided.

[AviBueno]

Update: I noticed that properties with undefined values may still appear in nested objects, causing:
{"a": 1, "b": undefined, "c": { "d": 2, "e": undefined } }
to end up as:
{"a": 1, "c": { "d": 2, "e": undefined } }

I'm looking into integrating a fix for that, but it might incur an overhead.

[AviBueno]

I noticed that properties with undefined values may still appear in nested objects, causing: {"a": 1, "b": undefined, "c": { "d": 2, "e": undefined } } to end up as: {"a": 1, "c": { "d": 2, "e": undefined } }

I'm looking into integrating a fix for that, but it might incur an overhead.

TL;DR: False alarm - the fix works.

@MichalLytek after further investigation, I can confirm that my concern about nested objects retaining undefined properties was a false alarm (I mistakenly tested with the latest release code instead of testing with my fix).

The fix handles nested InputTypes correctly, and here's why:

The convertToInput function in convert-args.ts recursively processes nested InputTypes, but crucially, it calls convertToType at every nesting level (line 113). So for an object like {args: {orderBy: [{ name: "asc" }]}:

1. convertToInput processes the parent object
2. For the nested orderBy array elements, it recursively calls convertToInput
3. At each level, convertToType(Target, filteredData) is called
4. My fix cleans up constructor-initialized undefined properties at each level

I verified this by testing with a real-world scenario involving nested InputTypes - the fix works correctly throughout the object tree.

Edge case (not a practical concern)

The only scenario where nested undefined properties could persist is if someone explicitly pre-instantiates nested objects in their InputType class definition:

@InputType()
class Parent {
  child: ChildInput = new ChildInput(); // Anti-pattern
}

However, this is an anti-pattern for InputTypes since GraphQL inputs should come from the query, not defaults. This doesn't represent a real-world use case.

Regarding the GitHub Code Scanning security warning

I noticed GitHub flagged a "Remote property injection" warning on the PR. When trying to view details, the link leads to a 404 (https://github.com/MichalLytek/type-graphql/security/code-scanning/11), and the repo doesn't seem to have a SECURITY.md configured (https://github.com/MichalLytek/type-graphql/security).

I believe this is a false positive - the code iterates over Object.keys(instance) (properties from the class constructor) and checks against key in data (where data comes from GraphQL's already-validated input). The property names are controlled by the class definition, not user input.

Please advise on how to proceed with this.

Thanks.