mockingbird icon indicating copy to clipboard operation
mockingbird copied to clipboard

bug: using `null` or `undefined` as the `@Mock(() => result)` throws error

Open omermorad opened this issue 3 years ago • 9 comments

If i use null or undefined as the @Mock(() => result), it throws:

    TypeError: Cannot convert undefined or null to object
        at Function.getOwnPropertyNames (<anonymous>)

      at getClassMembers (../../../node_modules/@plumier/reflect/lib/parser.js:87:28)
      at parseMethods (../../../node_modules/@plumier/reflect/lib/parser.js:134:21)
      at parseClassNoCache (../../../node_modules/@plumier/reflect/lib/parser.js:173:18)
      at ../../../node_modules/@plumier/reflect/lib/helpers.js:18:31
      at walkTypeMembers (../../../node_modules/@plumier/reflect/lib/walker.js:35:45)
      at walkTypeMembersRecursive (../../../node_modules/@plumier/reflect/lib/walker.js:54:23)
      at reflectClass (../../../node_modules/@plumier/reflect/lib/reflect.js:15:55)
      at reflectModuleOrClass (../../../node_modules/@plumier/reflect/lib/reflect.js:66:16)
      at ../../../node_modules/@plumier/reflect/lib/helpers.js:18:31

Originally posted by @arthurfiorette in https://github.com/omermorad/mockingbird/issues/135#issuecomment-1017600274

omermorad avatar Jan 22 '22 21:01 omermorad

Sorry for the delay, I was traveling. Did you manage to reproduce this bug?

arthurfiorette avatar Jan 23 '22 14:01 arthurfiorette

Keep on doing that, we need some fresh air sometimes :) I didn't get to it yet, but until I will - if you have a chance to add a snippet it might be helpful!

omermorad avatar Jan 25 '22 20:01 omermorad

Hey @omermorad, It's me again.

This bug is occurring when i use the T | null or T | undefined type.

class Test {
 @Mock(...)
 fine: string;

 @Mock(...)
 alsoFine?: string;

 @Mock(...)
 notFine: string | null;
 
 @Mock(...)
 alsoNotFine: string | undefined;
}

And that's because with a strict tsconfig, the decorator metadata emitted for these lines are different:

// With fine?: string;
tslib_1.__metadata("design:type", String)

// With notFine: string | null;
tslib_1.__metadata("design:type", Object)

Note that this only occurs with strict: true on the tsconfig.json.

This could be alright if it was trying automatically to find a mock value, but as it was manually specified (not this), I don't think that the metadata should be read and interpreted.

And when it needs to be interpreted, but the generated metadata is Object, it should emit a warning (not an error) or adapt itself like other decorators libraries do.

// type-graphql does manually the type

@Field(() => String) // <--
a: string;

I don't have time to create a 100% reproductible steps in the moment, but this should be sufficient...

arthurfiorette avatar Apr 16 '22 17:04 arthurfiorette

@arthurfiorette what version are you using? Can you try it with 3.0.0-rc.3?

omermorad avatar Apr 16 '22 17:04 omermorad

Yep, 3.0.0-rc.3 works just fine!

arthurfiorette avatar Apr 16 '22 17:04 arthurfiorette

But, is it safe to upgrade? If it is, you should create a stable release (3.0.0)...

arthurfiorette avatar Apr 16 '22 17:04 arthurfiorette

Yes, it's been tested with a new integration test, look here: https://github.com/omermorad/mockingbird/blob/mockingbird%403.0.0-rc.3/packages/parser/test/integration/common/test-classes.ts, those are all actual classes that are being tested (simulates a real situation).

I belive I will release this in the next week, I almost have no time to breathe now because of the new startup I work for :)

omermorad avatar Apr 16 '22 17:04 omermorad

Hello, Plumier developer here. I come here because I see that the error was related to the @plumier/reflect error.

We currently have some bugs fixed on our newest release v1.1.0, let me know if you having any further issues.

ktutnik avatar Dec 28 '22 12:12 ktutnik

@omermorad ?

arthurfiorette avatar Jan 30 '23 22:01 arthurfiorette