Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
omermorad opened this issue Jan 22, 2022 · 9 comments
Open
Labels
bug 🪲 Something isn't working

Comments

@omermorad
Copy link
Owner

omermorad commented Jan 22, 2022

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 #135 (comment)

@omermorad omermorad changed the title Using null or undefined as the @Mock(() => result) throws error bug: using null or undefined as the @Mock(() => result) throws error Jan 22, 2022
@omermorad omermorad added the bug 🪲 Something isn't working label Jan 22, 2022
@arthurfiorette
Copy link

arthurfiorette commented Jan 23, 2022

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

@omermorad
Copy link
Owner Author

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!

@arthurfiorette
Copy link

arthurfiorette commented Apr 16, 2022

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...

@omermorad
Copy link
Owner Author

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

@arthurfiorette
Copy link

Yep, 3.0.0-rc.3 works just fine!

@arthurfiorette
Copy link

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

@omermorad
Copy link
Owner Author

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 :)

@ktutnik
Copy link

ktutnik commented Dec 28, 2022

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.

@arthurfiorette
Copy link

@omermorad ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🪲 Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants